On Fri, Jun 10, 2022 at 09:29:44AM +0200, Jan Beulich wrote: > 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.
Sorry, started replying yesterday but had to leave and left the reply open. Then when I came back this morning I just read the code and not the commit message. Hm, so ideally we would also then move dpci_pirq() to hvm.c in order to match the move of pirq_dpci(), but that's not possible due to that helper having other callers outside of hvm.c. We could always export the function from hvm.c if we gained outside callers. In any case, I don't want to block you further on this, so: Acked-by: Roger Pau Monné <roger....@citrix.com> Thanks, Roger.
