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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.10.2021 22:07, Andrew Cooper wrote:
> GCC master (nearly version 12) complains:
> 
>   hvm.c: In function 'hvm_gsi_eoi':
>   hvm.c:905:10: error: the comparison will always evaluate as 'true' for the
>   address of 'dpci' will never be NULL [-Werror=address]
>     905 |     if ( !pirq_dpci(pirq) )
>         |          ^
>   In file included from /local/xen.git/xen/include/xen/irq.h:73,
>                    from /local/xen.git/xen/include/xen/pci.h:13,
>                    from /local/xen.git/xen/include/asm/hvm/io.h:22,
>                    from /local/xen.git/xen/include/asm/hvm/domain.h:27,
>                    from /local/xen.git/xen/include/asm/domain.h:7,
>                    from /local/xen.git/xen/include/xen/domain.h:8,
>                    from /local/xen.git/xen/include/xen/sched.h:11,
>                    from /local/xen.git/xen/include/xen/event.h:12,
>                    from hvm.c:20:
>   /local/xen.git/xen/include/asm/irq.h:140:34: note: 'dpci' declared here
>     140 |             struct hvm_pirq_dpci dpci;
>         |                                  ^~~~
> 
> The location marker is unhelpfully positioned and upstream may get around to
> fixing it.  The complaint is intended to be:
> 
>   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.

Jan