Re: [RFC/PATCH] ignore memcmp() overreading in bsearch() callback

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Jan 15, 2013, 15:55 UTC
Message-ID: <50F57BDF.1050400@lsrfire.ath.cx>
In-Reply-To: <7v38y38hhm.fsf@alter.siamese.dyndns.org>

Am 15.01.2013 00:36, schrieb Junio C Hamano:

Show 5 quoted lines

> It appears that memcmp() uses the usual "one word at a time"
> comparison and triggers valgrind in a callback of bsearch() used in
> the refname search.  I can easily trigger problems in any script
> with test_commit (e.g. "sh t0101-at-syntax.sh --valgrind -i -v")
> without this suppression.

I can't reproduce it on Debian, but can we perhaps do without the suppression with a patch like this instead? I would expect it to be slightly faster because we lose the strlen() call, but didn't check. It's also simpler, perhaps with the exception of the last line. Does it help in your case?

René

---
 refs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 541fec2..1a0e049 100644
--- a/refs.c
+++ b/refs.c
@@ -335,12 +335,10 @@ static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
 {
 	struct string_slice *key = (struct string_slice *)key_;
 	struct ref_entry *ent = *(struct ref_entry **)ent_;
-	int entlen = strlen(ent->name);
-	int cmplen = key->len < entlen ? key->len : entlen;
-	int cmp = memcmp(key->str, ent->name, cmplen);
+	int cmp = strncmp(key->str, ent->name, key->len);
 	if (cmp)
 		return cmp;
-	return key->len - entlen;
+	return '\0' - ent->name[key->len];
 }
 
 /*