Re: [REGRESSION] git-wrapper to run-commands codepath regression

From: Jeff King <peff@peff.net>
Date: Apr 18, 2011, 21:11 UTC
Message-ID: <20110418211102.GA13566@sigill.intra.peff.net>
In-Reply-To: <7v8vv78eld.fsf@alter.siamese.dyndns.org>

On Mon, Apr 18, 2011 at 01:54:54PM -0700, Junio C Hamano wrote:

Show 29 quoted lines

> There appears to be a regression in the codepath between git wrapper and
> run_commands API.
> 
> 	$ T=/var/tmp/test-commands
> 	$ mkdir $T
> 	$ cat >$T/git-hello <<\-EOF
> 	#!/bin/sh
> 	echo hello
> 	EOF
> 	$ chmod +x $T/git-hello
> 	$ oPATH=$PATH
> 	$ PATH=$T:$PATH
> 	$ export PATH
> 	$ git hello
> 	hello
> 
> So far, I added a "hello" subcommand to "git", and it runs correctly.
> 
> Now, when I make the script non-executable, this is what I get from
> 'maint':
> 
> 	$ chmod a-x $T/git-hello
> 	$ git hello
> 	fatal: cannot exec 'git-hello': Permission denied
> 
> But with 'master', we get a disturbing output:
> 
> 	$ git hello
>         fatal: $

The good news is that the bug is trivial. It bisects Jonathan's ebec842 (run-command: prettify -D_FORTIFY_SOURCE workaround, 2011-03-16), which introduces:

-       unused = write(child_err, "fatal: ", 7);
-       unused = write(child_err, msg, len);
-       unused = write(child_err, "\n", 1);
+       if (write(child_err, "fatal: ", 7) ||
+           write(child_err, msg, len) ||
+           write(child_err, "\n", 1))
+               ; /* yes, gcc -D_FORTIFY_SOURCE, we know there was an error. */

Stare at that for a minute and see if you can guess what's wrong. :)

-Peff