From: Jeff King <peff@peff.net>
Date: Wed, 01 Oct 2014 12:33:44 GMT
Subject: Re: [PATCH 2/2] sha1-lookup: fix handling of duplicates in sha1_pos()
Message-ID: <20141001123343.GA10772@peff.net>
In-Reply-To: <542BE114.9070300@web.de>

On Wed, Oct 01, 2014 at 01:10:12PM +0200, René Scharfe wrote:

> >I wonder if it is worth adding a test (you test only that "not found"
> >produces a negative index, but not which index). Like:
> 
> api-sha1-array.txt says about sha1_array_lookup: "If not found, returns a
> negative integer", and that's what the test checks.

Hmm. I do not recall intentionally leaving the value unspecified; I
think it is more that I was simply not thorough when writing the
documentation. That being said...

> I actually like that the value is not specified for that case because no
> existing caller actually uses it and it leaves room to implement the
> function e.g. using bsearch(3).

Yeah, if no callers actually care right now, that is a reasonable
argument for leaving the exact return value unspecified (and testing
only what the documentation claims).

> I agree that adding a "lookup non-existing entry with duplicates" test would
> make t0064 more complete, though.

Agreed.

-Peff