Re: [PATCH] run-command: prettify -D_FORTIFY_SOURCE workaround

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> Instead, use an "if" statement with an empty body to make the intent
> clear.
>
> 	if (write(...))
> 		; /* yes, yes, there was an error. */

Yuck --- and that is not meant against your workaround, but against the
compiler bogosity.  The above is reasonable (for some definition of the
word) and the comment makes the yuckiness tolerable by being somewhat
amusing.

But your comment in the actual patch is not amusing at all.

It certainly is _not_ "ok" to see errors from write(2); we are _ignoring_
the error because at that point in the codepath there isn't any better
alternative.  The unusual "if ()" whose condition is solely for its side
effect, with an empty body, is a strong enough sign to any reader that
there is something fishy going on, and it would be helpful to the reader
to hint _why_ such an unusual construct is there.  It would be much better
for the longer term maintainability to say at least "gcc" in the comment,
i.e.

	if (write(...))
        	; /* we know we are ignoring the error, mr gcc! */

or something.

Thanks for another amusing patch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]