On 11/07/14 09:46, Jeff King wrote:
> The "struct object" type implements basic object
> polymorphism. Individual instances are allocated as
> concrete types (or as a union type that can store any
> object), and a "struct object *" can be cast into its real
> type after examining its "type" enum. This means it is
> dangerous to have a type field that does not match the
> allocation (e.g., setting the type field of a "struct blob"
> to "OBJ_COMMIT" would mean that a reader might read past the
> allocated memory).
>
> In most of the current code this is not a problem; the first
> thing we do after allocating an object is usually to set its
> type field by passing it to create_object. However, the
> virtual commits we create in merge-recursive.c do not ever
> get their type set. This does not seem to have caused
> problems in practice, though (presumably because we always
> pass around a "struct commit" pointer and never even look at
> the type).
>
> We can fix this oversight and also make it harder for future
> code to get it wrong by setting the type directly in the
> object allocation functions.
>
> This will also make it easier to fix problems with commit
> index allocation, as we know that any object allocated by
> alloc_commit_node will meet the invariant that an object
> with an OBJ_COMMIT type field will have a unique index
> number.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> alloc.c | 18 ++++++++++--------
> blob.c | 2 +-
> builtin/blame.c | 1 -
> commit.c | 2 +-
> object.c | 5 ++---
> object.h | 2 +-
> tag.c | 2 +-
> tree.c | 2 +-
> 8 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/alloc.c b/alloc.c
> index d7c3605..fd5fcb7 100644
> --- a/alloc.c
> +++ b/alloc.c
> @@ -18,11 +18,11 @@
>
> #define BLOCKING 1024
>
> -#define DEFINE_ALLOCATOR(name, type) \
> +#define DEFINE_ALLOCATOR(name, flag, type) \
> static struct alloc_state name##_state; \
> void *alloc_##name##_node(void) \
> { \
> - return alloc_node(&name##_state, sizeof(type)); \
> + return alloc_node(&name##_state, flag, sizeof(type)); \
> }
I don't particularly like 'flag' here. (not a massive dislike, mind you:)
Perhaps: flag->object_type, type->node_type?
Or, if that's too verbose, maybe just: flag->type, type->node?
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html