On Wed, Oct 1, 2014 at 5:43 AM, René Scharfe <l.s.r@xxxxxx> wrote:
> 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@xxxxxx>
> ---
> 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
> +'
An alternative would be to introduce these two tests in patch 1/2 as
test_expect_failure and flip them to test_expect_success in this patch
which fixes the problem.
> +
> test_done
> --
> 2.1.2
--
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