Re: [PATCH] misra: add deviations of MISRA C Rule 5.5

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.07.2025 18:12, Dmytro Prokopchuk1 wrote:
> 
> 
> On 7/21/25 13:34, Jan Beulich wrote:
>> On 21.07.2025 12:27, Dmytro Prokopchuk1 wrote:
>>> On 7/16/25 21:08, Dmytro Prokopchuk wrote:
>>>> MISRA C Rule 5.5 states that: "Identifiers shall
>>>> be distinct from macro names".
>>>>
>>>> Update ECLAIR configuration to deviate:
>>>> - clashes in 'xen/include/xen/bitops.h';
>>>> - clashes in 'xen/include/xen/irq.h';
>>>> - clashes in 'xen/common/grant_table.c'.
>>>>
>>>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
>>>> ---
>>>> This patch makes MISRA Rule 5.5 "clean" for ARM only.
>>>>
>>>> Was agreed to narrow deviation:
>>>> https://patchew.org/Xen/cover.1752096263.git.dmytro._5Fprokopchuk1@xxxxxxxx/e47d08e4465f913f03348830954e800f420c652d.1752096263.git.dmytro._5Fprokopchuk1@xxxxxxxx/
>>>>
>>>> CI:
>>>> https://eclair-analysis-logs.xenproject.org/fs/var/local/eclair/xen-project.ecdf/xen-project/people/dimaprkp4k/xen/ECLAIR_normal/rule_5.5_deviation_final/ARM64/10706457595/PROJECT.ecd;/by_service.html#service&kind
>>>> ---
>>>>    automation/eclair_analysis/ECLAIR/deviations.ecl | 7 +++++++
>>>>    docs/misra/deviations.rst                        | 7 +++++++
>>>>    2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>>>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> index 8504e850c1..3895148460 100644
>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> @@ -117,6 +117,13 @@ it defines would (in the common case) be already 
>>>> defined. Peer reviewed by the c
>>>>    -config=MC3A2.R5.5,reports+={deliberate, 
>>>> "any_area(decl(kind(function))||any_loc(macro(name(memcpy||memset||memmove))))&&any_area(any_loc(file(^xen/common/libelf/libelf-private\\.h$)))"}
>>>>    -doc_end
>>>>
>>>> +-doc_begin="Clashes between function names and macros are deliberate and 
>>>> needed to have a function-like macro that acts as a wrapper for the 
>>>> function to be called.
>>>> +Before calling the function, the macro adds additional checks or adjusts 
>>>> the number of parameters depending on the configuration."
>>>> +-config=MC3A2.R5.5,reports+={deliberate, 
>>>> "any_area(all_loc(file(^xen/include/xen/bitops\\.h$)) && 
>>>> macro(name(__test_and_set_bit||__test_and_clear_bit||__test_and_change_bit||test_bit)))"}
>>>> +-config=MC3A2.R5.5,reports+={deliberate, 
>>>> "any_area(all_loc(file(^xen/common/grant_table\\.c$))&&macro(name(update_gnttab_par||parse_gnttab_limit)))"}
>>>> +-config=MC3A2.R5.5,reports+={deliberate, 
>>>> "any_area(all_loc(file(^xen/include/xen/irq\\.h$))&&macro(name(pirq_cleanup_check)))"}
>>>> +-doc_end
>>>> +
>>>>    -doc_begin="The type \"ret_t\" is deliberately defined multiple times,
>>>>    depending on the guest."
>>>>    
>>>> -config=MC3A2.R5.6,reports+={deliberate,"any_area(any_loc(text(^.*ret_t.*$)))"}
>>>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>>>> index 620e97f0bd..84bc933cbf 100644
>>>> --- a/docs/misra/deviations.rst
>>>> +++ b/docs/misra/deviations.rst
>>>> @@ -142,6 +142,13 @@ Deviations related to MISRA C:2012 Rules:
>>>>           memmove.
>>>>         - Tagged as `deliberate` for ECLAIR.
>>>>
>>>> +   * - R5.5
>>>> +     - Clashes between function names and macros are deliberate and needed
>>>> +       to have a function-like macro that acts as a wrapper for the 
>>>> function to be
>>>> +       called. Before calling the function, the macro adds additional 
>>>> checks or
>>>> +       adjusts the number of parameters depending on the configuration.
>>>> +     - Tagged as `deliberate` for ECLAIR.
>>>> +
>>>>       * - R5.6
>>>>         - The type ret_t is deliberately defined multiple times depending 
>>>> on the
>>>>           type of guest to service.
>>>
>>> Could you review this patch?
>>
>> I don't understand. I'm not feeling capable of (fully) reviewing changes to
>> automation/eclair_analysis/ECLAIR/deviations.ecl. Nor am I the only 
>> maintainer
>> of that file. And the text being added to docs/misra/deviations.rst is too
>> unspecific for my taste, yet I thought I'd better not repeat that same 
>> concern
>> over and over again. If others feel like ack-ing in this shape, so be it.
> 
> Hello all!
> 
> Since there have been no new comments so far, I would like to propose a
> straightforward solution: rename the macros (that violate Rule 5.5)
> using capital letters (using best practices for naming macros in C).

Well, in your ping as well as here, despite the "Hello all" you address
just me (as per the To: list). Therefore I think you will want to send a
wider ping first. Also note that it's (sadly) entirely usual to not get
any reply on a patch for extended periods of time. I have no good
suggestion what you could do to help that in this case. Pinging, in my
experience, only rarely helps. (However, if you ping, it may be a good
idea to identify the mail being a ping already in the subject. What I
often do is replace the Re: with Ping:.)

> I can prepare such a patch.
> 
> The main question is: "Is it possible to upstream such changes?"
> Or is this idea not worthwhile?

I'm opposed to it.

Jan