Re: [PATCH v3 2/2] read-cache: plug a few leaks

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 9, 2013, 18:49 UTC
Message-ID: <7v8v2jqfuf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <51B32FFD.5070302@lsrfire.ath.cx>

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

Show 5 quoted lines

> A comment before read_index_from says "remember to discard_cache()
> before reading a different cache!".  That is probably a reminder that
> read_index_from does nothing if ->initialized is set.  Entries added
> before calling read_index_from make up a different cache, however, so
> I think this comment applies for the call sequence above as well.

Yes, you can lose the probably from there. Back when he comment was added at 8fd2cb406917 (Extract helper bits from c-merge-recursive work, 2006-07-25), there was only one in-core index, and checking active_cache or cache_mmap is not NULL was a way to say "Have we loaded from $GIT_DIR/index already? If so, there is nothing more to do".

Later unpack_trees() added a way to populate the index not by reading from $GIT_DIR/index and the original "Has file, hence is loaded, so ignore read_cache()" caused issues. A discussion was in

    http://thread.gmane.org/gmane.comp.version-control.git/93260/focus=93469

which brought the "initialized" bit in, which lead to 913e0e99b6a6 (unpack_trees(): protect the handcrafted in-core index from read_cache(), 2008-08-23).

> Side note: I wonder why we need to guard against multiple
> read_index_from calls in a row with ->initialized.  Wouldn't it be
> easier to avoid the duplicate calls in the first place?  Finding them
> now might be not so easy, though.