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

On Mon, Apr 18, 2011 at 02:40:31PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

And here's the fix.

I was tempted to suggest the change in your patch.

With ebec842 (run-command: prettify -D_FORTIFY_SOURCE workaround,
2011-03-16) reverted, I still don't get complaints from -D_FORTIFY_SOURCE
for run-command.c (but I do get "ignoring return value of 'fwrite' from
many places).  Perhaps the kinds of checks done by versions of gcc you,
Jonathan and I use are different.

I don't use _FORTIFY_SOURCE at all, so I have no clue. I just saw that
the code in ebec842 is obviously wrong, and the fix looked equally
obvious.

I'd rather revert it for now; I don't want to see patch ping-pong at this
late in the pre-release cycle.

That's your call, but the fix seems dead simple to me. _FORTIFY_SOURCE
likes the conditional, according to Jonathan's patch. We don't remove
the conditional, just the wrong "non-zero is an error" assumption. So I
wouldn't expect any ping-pong on it, but then again, it looked like a
pretty innocuous patch in the first place, and held a pretty nasty and
surprising bug. :)

-Peff