From: "René Scharfe" <l.s.r@web.de>
To: Git Mailing List <git@vger.kernel.org>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
Christian Couder <chriscool@tuxfamily.org>
Subject: [PATCH 2/2] sha1-lookup: fix handling of duplicates in sha1_pos()
Date: Wed, 01 Oct 2014 11:43:21 +0200 [thread overview]
Message-ID: <542BCCB9.4050908@web.de> (raw)
In-Reply-To: <542BCBFC.5000509@web.de>
If the first 18 bytes of the SHA1's of all entries are the same then
sha1_pos() dies and reports that the lower and upper limits of the
binary search were the same that this wasn't supposed to happen. This
is wrong because the remaining two bytes could still differ.
Furthermore: It wouldn't be a problem if they actually were the same,
i.e. if all entries have the same SHA1. The code already handles
duplicates just fine otherwise. Simply remove the erroneous check.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
sha1-lookup.c | 2 --
t/t0064-sha1-array.sh | 20 ++++++++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/sha1-lookup.c b/sha1-lookup.c
index 2dd8515..5f06921 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -84,8 +84,6 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
die("BUG: assertion failed in binary search");
}
}
- if (18 <= ofs)
- die("cannot happen -- lo and hi are identical");
}
do {
diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh
index bd68789..3fcb8d8 100755
--- a/t/t0064-sha1-array.sh
+++ b/t/t0064-sha1-array.sh
@@ -61,4 +61,24 @@ test_expect_success 'lookup with duplicates' '
test "$n" -le 3
'
+test_expect_success 'lookup with almost duplicate values' '
+ {
+ echo "append 5555555555555555555555555555555555555555" &&
+ echo "append 555555555555555555555555555555555555555f" &&
+ echo20 "lookup " 55
+ } | test-sha1-array >actual &&
+ n=$(cat actual) &&
+ test "$n" -eq 0
+'
+
+test_expect_success 'lookup with single duplicate value' '
+ {
+ echo20 "append " 55 55 &&
+ echo20 "lookup " 55
+ } | test-sha1-array >actual &&
+ n=$(cat actual) &&
+ test "$n" -ge 0 &&
+ test "$n" -le 1
+'
+
test_done
--
2.1.2
next prev parent reply other threads:[~2014-10-01 9:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-01 9:40 [PATCH 1/2] sha1-array: add test-sha1-array and basic tests René Scharfe
2014-10-01 9:43 ` René Scharfe [this message]
2014-10-01 10:50 ` [PATCH 2/2] sha1-lookup: fix handling of duplicates in sha1_pos() Jeff King
2014-10-01 11:10 ` René Scharfe
2014-10-01 12:33 ` Jeff King
2014-10-01 14:12 ` Eric Sunshine
2014-10-01 14:11 ` [PATCH 1/2] sha1-array: add test-sha1-array and basic tests Eric Sunshine
2014-10-01 14:21 ` René Scharfe
2014-10-01 14:23 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=542BCCB9.4050908@web.de \
--to=l.s.r@web.de \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).