Jeremiah Mahler <jmmahler@xxxxxxxxx> 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 <jmmahler@xxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html