From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Fri, 11 Jul 2014 00:59:53 GMT
Subject: Re: [PATCH v3 2/2] alloc.c: remove the redundant commit_count variable
Message-ID: <53BF3709.6000307@ramsay1.demon.co.uk>
In-Reply-To: <20140711003053.GB11360@sigill.intra.peff.net>

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