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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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)