Re: [XEN PATCH 01/10] xen/include: address violations of MISRA C Rule 20.7

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-02-29 17:47, Andrew Cooper wrote:
> On 29/02/2024 4:21 pm, Nicola Vetrini wrote:
> > On 2024-02-29 17:10, Andrew Cooper wrote:
> > > On 29/02/2024 3:27 pm, Nicola Vetrini wrote:
> > > > diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
> > > > index c25dc0f6c2a9..b7e70289737b 100644
> > > > --- a/xen/include/xen/kconfig.h
> > > > +++ b/xen/include/xen/kconfig.h
> > > > @@ -25,7 +25,7 @@
> > > >  #define __ARG_PLACEHOLDER_1 0,
> > > >  #define config_enabled(cfg) _config_enabled(cfg)
> > > >  #define _config_enabled(value)
> > > > __config_enabled(__ARG_PLACEHOLDER_##value)
> > > > -#define __config_enabled(arg1_or_junk)
> > > > ___config_enabled(arg1_or_junk 1, 0)
> > > > +#define __config_enabled(arg1_or_junk)
> > > > ___config_enabled(arg1_or_junk (1), (0))
> > > >  #define ___config_enabled(__ignored, val, ...) val
> > > This one hunk I suggest we deviate rather than adjust.  You've subtly
> > > broken it, and it's extreme preprocessor magic in the first place to
> > > turn an absent symbol into a 0.
> > How so? I did test this because I was very wary of it, but it seemed
> > to expand fine (either if ((0)) or if ((1)) ). I may of course be
> > wrong, and it could be deviated regardless.
> arg1_or_junk(1) can now be a function or macro expansion depending on
> context, where previously it couldn't be.
I see, that would be a latent bug. I do agree on deviating then.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)