Re: [PATCH v5 1/2] name-hash.c: replace cache_name_compare() with memcmp()

Jeremiah Mahler [off-list ref] writes:

When cache_name_compare() is used on counted strings of the same
length, it is equivalent to a memcmp().  Since the one use of
cache_name_compare() in name-hash.c requires that the lengths are
equal, just replace it with memcmp().

I do not think it is not "requires that the lengths are equal"; it
merely is a premature optimization, as it wants to catch only the
case where the names are the same.

Your patch is not wrong per-se, but with the above justification of
yours, I would actually have expected to see it updated to use
!cache_name_compare() and then later !name_compare().  That way, if
it ever turns out that giving name_compare() semantics specific to
"name" (as opposed to just byte-for-byte comparison given by
memcmp(3)) is a good idea, we will use that comparison with
semantics specific to "name"s here, without having to change it from
memcmp().

Having said all that, I think we see a more correct justification
for this change in the pre-context of the patch.  We want the exact
comparison, without any funky "name"-specific semantics in the
quick-and-exact case.

I've queued it like this (no need to reroll).
Thanks.

    name-hash.c: replace cache_name_compare() with memcmp(3)
    
    The same_name() private function wants a quick-and-exact check to
    see if they two names are byte-for-byte identical first and then
    fall back to the slow path.  Use memcmp(3) for the former to make it
    clear that we do not want any "name" specific comparison.
    
    Signed-off-by: Jeremiah Mahler [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

diff --git a/name-hash.c b/name-hash.c
index 97444d0..49fd508 100644
--- a/name-hash.c
+++ b/name-hash.c

@@ -179,7 +179,7 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
 	 * Always do exact compare, even if we want a case-ignoring comparison;
 	 * we do the quick exact one first, because it will be the common case.
 	 */
-	if (len == namelen && !cache_name_compare(name, namelen, ce->name, len))
+	if (len == namelen && !memcmp(name, ce->name, len))
 		return 1;
 
 	if (!icase)