Re: [PATCH for-4.16] x86/passthrough: Fix hvm_gsi_eoi() build with GCC 12

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.11.2021 17:13, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] x86/passthrough: Fix hvm_gsi_eoi() build 
> with GCC 12"):
>> On 27.10.2021 22:07, Andrew Cooper wrote:
>>>   if ( !((pirq) ? &(pirq)->arch.hvm.dpci : NULL) )
>>
>> I disagree with the compiler's analysis: While &(pirq)->arch.hvm.dpci
>> indeed can't be NULL, that's not the operand of !. The operand of !
>> can very well be NULL, when pirq is.
>>
>>> which is a hint that the code is should be simplified to just:
>>>
>>>   if ( !pirq )
>>>
>>> Do so.
>>
>> And I further agree with Roger's original reply (despite you
>> apparently having managed to convince him): You shouldn't be open-
>> coding pirq_dpci(). Your observation that the construct's result
>> isn't otherwise used in the function is only one half of it. The
>> other half is that hvm_pirq_eoi() gets called from here, and that
>> one does require the result to be non-NULL.
> 
> Can you (collectively) please come to some agreement here ?
> I think this is mostly a question of taste or style.

Personally I don't think open-coding of constructs is merely a style /
taste issue. The supposed construct changing and the open-coded
instance then being forgotten (likely not even noticed) can lead to
actual bugs. I guess the issue should at least be raised as one against
gcc12, such that the compiler folks can provide their view on whether
the warning is actually appropriate in that case (and if so, what their
take is on a general approach towards silencing such warnings when
they're identified as false positives).

Jan