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
