Re: t3404 static check of bad SHA-1 failure

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2016, 20:03 UTC
Message-ID: <xmqqshxl9142.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160513195911.GE9890@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

> On Fri, May 13, 2016 at 12:52:44PM -0700, Junio C Hamano wrote:
>
>> I _think_ "test -z" should succeed according to POSIX, because
>> 
>>  (1) it is not "test -z string" because it lacks string,
>> 
>>  (2) it is not any of the other "test -<option> thing" because -z,
>>     and
>> 
>>  (3) the only thing it matches in the supported form of "test" is
>>      "test <string>" that tests if the <string> is not the null
>>      string, and "-z" indeed is not the null string.
>> 
>> For the same reason, "test -n" succeeds.
>
> Yeah, I think you're right; POSIX is pretty clear that this falls under
> case 3. So that means "test -z" quietly does what we want. But it means
> that "test -n" does the _opposite_ of what we want.

;-)

Show 5 quoted lines

> And sadly,
>
>   git grep 'test -n [^"]'
>
> is not empty.

Are you doing an audit? Otherwise I'm interested in taking a brief look.

Show 8 quoted lines

>> But working around older/broken shells is easy and the resulting
>> script it more readable, so let's take this.  It makes the resulting
>> code easier to understand even when we know we run it under POSIX
>> shell.
>
> Yep. The POSIX-explanation of what is going on might be worth putting in
> the commit message for the "-z" case (i.e., it should work, but the
> "why" is subtle).

Perhaps.

A small consolation for ksh fans around is that ksh93 seems to get this case right ;-)