Jeremiah Mahler <jmmah...@gmail.com> 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 <jmmah...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html