Subject: Re: [PATCH v3 2/2] alloc.c: remove the redundant commit_count variable

From: Ramsay Jones <[email-masked]>
Date: Fri, 11 Jul 2014 01:59:53 +0100

On 11/07/14 01:30, Jeff King wrote:
> On Fri, Jul 11, 2014 at 12:59:48AM +0100, Ramsay Jones wrote:
> 
>> The 'commit_count' static variable is used in alloc_commit_node()
>> to set the 'index' field of the commit structure to a unique value.
>> This variable assumes the same value as the 'count' field of the
>> 'commit_state' allocator state structure, which may be used in its
>> place.
> 
> I don't think we want to do this, because there is a bug in the current
> code that I have not reported yet. :)

:P OK, I will simply drop this one then.

> 
> The code you're touching here was trying to make sure that each commit
> gets a unique index, under the assumption that commits only get
> allocated via alloc_commit_node. But I think that assumption is wrong.
> We can also get commit objects by allocating an OBJ_NONE (e.g., via
> lookup_unknown_object) and then converting it into an OBJ_COMMIT when we
> find out what it is.

Hmm, I don't know how the object is converted, but the object allocator
is actually allocating an 'union any_object', so it's allocating more
space than for a struct object anyway. If you add an 'index' field to
struct object, (and remove it from struct commit) it could be set in
alloc_object_node(). ie _all_ node types get an index field.

Hmm, that was just off the top of my head, so take with a pinch of salt.

ATB,
Ramsay Jones