Hi,
Jeff King wrote:
>> On Tue, 2018-08-21 at 23:03 -0400, Jeff King wrote:
>>> static inline int hashcmp(const unsigned char *sha1, const unsigned
>>> char *sha2)
>>> {
>>> + assert(the_hash_algo->rawsz == 20);
>>> return memcmp(sha1, sha2, the_hash_algo->rawsz);
>>> }
[...]
> The bigger questions are:
>
> - are we OK with such an assertion; and
>
> - does the assertion still give us the desired behavior when we add in
> a branch for rawsz==32?
>
> And I think the answers for those are both "probably not".
At this point in the release process, I think the answer to the first
question is a pretty clear "yes".
A ~10% increase in latency of some operations is quite significant, in
exchange for no user benefit yet. We can continue to try to figure
out how to convince compilers to generate good code for this (and
that's useful), but in the meantime we should also do the simple thing
to avoid the regression for users.
Thanks,
Jonathan
