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

From: Jonathan Nieder <[email-masked]>
Date: Mon, 18 Apr 2011 17:17:20 -0500

Junio C Hamano wrote:

>     	if (write(...))
>     		; /* yes, yes, there was an error. */
>
> No, a non-zero return is not an error from the write(2) system call.
> I cannot believe both of us didn't spot it.  What were we smoking?

Yagh.

	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. */

There are two unusual conditions in which this could fail:

 - it doesn't write anything at all, in which case the return value
   is -1.
 - a partial write, for example if writing to an almost-full pipe.

I suppose in a calmer time, a better fix will look like

	if (write_in_full(child_err, "fatal: ", 7) != 7 ||
	    write_in_full(child_err, msg, len) != len ||
	    write_in_full(child_err, "\n", 1) != 1)
		/* yes, yes, ...

and gcc will have told us something potentially useful.

Thanks for catching it.