On 10.06.2022 09:20, Roger Pau Monné wrote: > On Fri, May 27, 2022 at 12:37:19PM +0200, Jan Beulich wrote: >> As per [1] the expansion of the pirq_dpci() macro causes a -Waddress >> controlled warning (enabled implicitly in our builds, if not by default) >> tying the middle part of the involved conditional expression to the >> surrounding boolean context. Work around this by introducing a local >> inline function in the affected source file. >> >> Reported-by: Andrew Cooper <andrew.coop...@citrix.com> >> Signed-off-by: Jan Beulich <jbeul...@suse.com> >> >> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967 >> --- >> This is intended to replace an earlier patch by Andrew [2], open-coding >> and then simplifying the macro in the one problematic place. >> >> Note that, with pirq_dpci() presently used solely in the one file being >> changed here, we could in principle also remove the #define and use just >> an inline(?) function in this file. But then the macro would need >> reinstating as soon as a use elsewhere would become necessary.
Did you read this before ... >> As to the inline - I think it's warranted here, but it goes against our >> general policy of using inline only in header files. Hence I'd be okay >> to drop it to avoid controversy. >> >> [2] https://lists.xen.org/archives/html/xen-devel/2021-10/msg01635.html >> >> --- a/xen/drivers/passthrough/x86/hvm.c >> +++ b/xen/drivers/passthrough/x86/hvm.c >> @@ -25,6 +25,18 @@ >> #include <asm/hvm/support.h> >> #include <asm/io_apic.h> >> >> +/* >> + * Gcc12 takes issue with pirq_dpci() being used in boolean context (see gcc >> + * bug 102967). While we can't replace the macro definition in the header >> by an >> + * inline function, we can do so here. >> + */ >> +static inline struct hvm_pirq_dpci *_pirq_dpci(struct pirq *pirq) >> +{ >> + return pirq_dpci(pirq); >> +} >> +#undef pirq_dpci >> +#define pirq_dpci(pirq) _pirq_dpci(pirq) > > That's fairly ugly. Seeing as pirq_dpci is only used in hvm.c, would > it make sense to just convert the macro to be a static inline in that > file? (and remove pirq_dpci() from irq.h). ... saying so? IOW I'm not entirely opposed, but I'm a little afraid we might be setting us up for later trouble. Jan
