Re: [Xen-devel] [PATCH for-4.8] ipxe: update to newer commit

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/10/2016 14:32, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [Xen-devel] [PATCH for-4.8] ipxe: update to 
> newer commit"):
>> On 10/11/2016 05:52 AM, Ian Jackson wrote:
>>> Maybe we could consider these as backports to earlier releases.
>>> However, I looked at the patch and it mostly seems to be removing null
>>> pointer checks.  I find this ... surprising.
>> That's because routines have __nonnull attribute (which tells compiler
>> that arguments are never NULL) and new gcc warns on non-NULL checks for
>> these arguments.
> *sigh*  Why don't we just disable that warning ?

*sigh* This shouldn't be warning at all.  It falls into the same
category as
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=26ea2cce0d3d25974eea3c643ce2081adfa2a69c
which is "Will cause you to actively write insecure code if you believe
the compiler".

The useful part of using __nonnull is to get the compiler to complain at
you if it does find a case where you end up passing NULL.  Forcing the
coder to remove the supposedly-redundant NULL check makes the code less
resistant to exceptional circumstances.

>
>> Another interesting new warning that is fatal with -Werror is
>>     if(a)
>>         foo();
>>         bar();
>>
>> gcc warns that bar() is indented and maybe braces are needed.
> Do we actually have cases like this ?

Yes

>   Are they real bugs ?

Very much yes.

goto fail;

This is one of the most useful warnings to have been introduced into C
compilers for a very long time.  c/s
ebdba150bff1d914805d60efa576337bbef0c305 was a real bug in our tree
caught by it.

>
>> BTW, another option for backporting may be removing -Werror. If we know
>> we are not changing sources then we might consider this.
> Perhaps we could disable warnings more selectively.

This code hasn't changed in ages.  Irrespective of what newer compilers
think, it won't start working differently,

For 4.8, if we are not going to change the iPXE version, then disabling
-Werror is probably the best move.  We have no idea which new warnings
will appear in GCC7.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel