Re: [RFH] unpack-trees: cache_entry lifetime issue?

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 2, 2012, 19:17 UTC
Message-ID: <7vk432dd89.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4F5102A2.70303@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

Show 11 quoted lines

> which shows some parts of unpack-trees.c that I use as context to
> ask: Should we check for o->merge in line 775, before using src[0]?
>
> If o->merge is 0, the src[0] will be NULL right up to the call of
> unpack_nondirectories() in line 772.  There it may be set (in line
> 582).  In that case we'll end up at line 779, where mark_ce_used()
> is applied to it.
>
> I suspect that this is unintended and that line 775 should rather
> read "if (o->merge && src[0]) {".  Can someone with a better
> understanding of unpack-trees confirm or refute that suspicion?

Yeah, src[0] is meant to hold the entry from the current index to take it as well as our tree into account during o->merge, and I do not think it should affect when we are only reading tree(s) into the index.

I think da165f4 (unpack-trees.c: prepare for looking ahead in the index, 2010-01-07) simply forgot that the codepath also has to work when it is not merging.

Having said that, I do not know offhand if we just should nothing in no-merge case, or we should be doing something else instead, without thinking a bit more.