Re: [PATCH 2/2] read-cache.c: fix timestamp comparison

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jan 22, 2008, 22:00 UTC
Message-ID: <alpine.LFD.1.00.0801221355110.1741@woody.linux-foundation.org>
In-Reply-To: <alpine.LFD.1.00.0801211242330.2957@woody.linux-foundation.org>

On Mon, 21 Jan 2008, Linus Torvalds wrote:

Show 14 quoted lines

> On Mon, 21 Jan 2008, Junio C Hamano wrote:
> > 
> > Very good catch.
> 
> The stupid thing is that I literally _grepped_ for the code testing 
> "ce_mode".
> 
> I had missed that one because I had looked for things like
> 
> 	if ([!].*ce_mode)
> 
> but that switch statement meant that the comparison to zero was 
> non-local and my grep didn't see it. And while the test-suite did have a 
> test for this, it was hidden by the racy index logic.. Unlucky.

Ok, having looked a bit more, I found another two cases that my grep had missed. They were regular if-statements, just more complex than my stupid grep pattern had taken into account.

Both are in read-cache.c:

read-cache.c: if (stage || istate->cache[pos]->ce_mode) { read-cache.c: if (ce_stage(p) == stage && (stage || p->ce_mode))

and I'd send a patch, except my tree right now is in pretty bad shape because I'm also trying to see if I can add a name hash to the index.

Anyway, the "xyzzy->ce_mode" check should in both cases become a "!(xyzzy->ce_flags & CE_REMOVE)" instead.

And we obviously don't seem to have any tests for this situation (I think they are both D/F conflicts with the file having been marked removed by an earlier phase, so they'd be some really odd kind of merge or something).

		Linus