Re: [PATCH] sha1_file: avoid comparison if no packed hash matches the first byte

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 9, 2017, 05:36 UTC
Message-ID: <xmqqpoc58fq6.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170808225851.kgbyrlqrtlurzc73@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 14 quoted lines

> On Tue, Aug 08, 2017 at 06:52:31PM -0400, Jeff King wrote:
>
>> > Interesting.  I see that we still have the conditional code to call
>> > out to sha1-lookup.c::sha1_entry_pos().  Do we need a similar change
>> > over there, I wonder?  Alternatively, as we have had the experimental
>> > sha1-lookup.c::sha1_entry_pos() long enough without anybody using it,
>> > perhaps we should write it off as a failed experiment and retire it?
>> 
>> There is also sha1_pos(), which seems to have the same problem (and is
>> used in several places).
>
> Actually, I take it back. The problem happens when we enter the loop
> with no entries to look at. But both sha1_pos() and sha1_entry_pos()
> return early before hitting their do-while loops in that case.

Ah, I was not looking at that part of the code.  Thanks.

I still wonder if we want to retire that conditional invocation of sha1_entry_pos(), though.