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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.08.2025 18:56, Dmytro Prokopchuk1 wrote:
> On 8/1/25 01:32, Nicola Vetrini wrote:
>> On 2025-07-31 22:43, Dmytro Prokopchuk1 wrote:
>>> MISRA C Rule 5.5 states: "Identifiers shall be distinct from macro 
>>> names".
>>>
>>> Update ECLAIR configuration to deviate clashes: specify the macros that
>>> should be ignored. Update deviations.rst and rules.rst accordingly.
>>>
>>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
>>
>> Reviewed-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> # ECLAIR
>>
>>> ---
>>> Changes in v4:
>>> - fixed formatting (aligned length chars per line)
>>> - set 'ignored_macros' as a regex expression
>>> - set a deviation restriction on xen/common/grant_table.c
>>> - s/ensures/to ensure/
>>> - fixed grammar errors
>>>
>>> Link to v3:
>>> https://eur01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Fpatchew.org%2FXen%2Fe681e0c083d945f48e6d0add1aee32af16be224e.1753911247.git.dmytro._5Fprokopchuk1%40epam.com%2F&data=05%7C02%7Cdmytro_prokopchuk1%40epam.com%7Ce9d60ddeef764dfa381208ddd0823178%7Cb41b72d04e9f4c268a69f949f367c91d%7C1%7C0%7C638895979778846477%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=n0QPvqr8O8gos%2BYMpZ%2BMOG0spC46A7T56Vpz44wutlw%3D&reserved=0
>>>
>>> Test CI pipeline:
>>> https://eur01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Fgitlab.com%2Fxen- 
>>> project%2Fpeople%2Fdimaprkp4k%2Fxen%2F- 
>>> %2Fpipelines%2F1960066579&data=05%7C02%7Cdmytro_prokopchuk1%40epam.com%7Ce9d60ddeef764dfa381208ddd0823178%7Cb41b72d04e9f4c268a69f949f367c91d%7C1%7C0%7C638895979778869002%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=z8DtrlxxN8nyrMNRXXW8IktFaru4f3f8N99iirhnbw0%3D&reserved=0
>>> ---
>>>  .../eclair_analysis/ECLAIR/deviations.ecl     | 10 +++++++++
>>>  docs/misra/deviations.rst                     | 22 +++++++++++++++++++
>>>  docs/misra/rules.rst                          | 17 ++++++++++++++
>>>  3 files changed, 49 insertions(+)
>>>
>>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/ 
>>> automation/eclair_analysis/ECLAIR/deviations.ecl
>>> index 483507e7b9..ab3400fc89 100644
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -117,6 +117,16 @@ 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 bitops functions and macro names are 
>>> deliberate.
>>> +These macros are needed for input validation and error handling."
>>> +-config=MC3A2.R5.5,ignored_macros+="^(__)?(test|set|clear|change| 
>>> test_and_(set|clear|change))_bit$"
>>> +-doc_end
>>> +
>>> +-doc_begin="Clashes between grant table functions and macro names in 
>>> 'xen/common/grant_table.c' are deliberate.
>>> +These macros address differences in argument count during compile- 
>>> time, effectively discarding unused parameters to avoid warnings or 
>>> errors related to them."
>>> +-config=MC3A2.R5.5,ignored_macros+="name(update_gnttab_par|| 
>>> parse_gnttab_limit)&&loc(file(^xen/common/grant_table\\.c$))"
>>> +-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 e78179fcb8..4c64a8be62 100644
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -142,6 +142,28 @@ Deviations related to MISRA C:2012 Rules:
>>>         memmove.
>>>       - Tagged as `deliberate` for ECLAIR.
>>>
>>> +   * - R5.5
>>> +     - Clashes between bitops ('__test_and_set_bit', 
>>> '__test_and_clear_bit',
>>> +       '__test_and_change_bit', 'test_bit', 'set_bit', 'clear_bit', 
>>> 'change_bit',
>>> +       'test_and_set_bit', 'test_and_clear_bit', 'test_and_change_bit')
>>> +       functions and macro names are intentional. These are necessary 
>>> for error
>>> +       handling and input validation to ensure that the size of the 
>>> object being
>>> +       referenced by the memory address (passed as an argument to the 
>>> macro)
>>> +       meets the minimum requirements for the bit operation. This 
>>> prevents unsafe
>>> +       operations on improperly sized data types that could lead to 
>>> undefined
>>> +       behavior or memory corruption. The macros encapsulate this 
>>> conditional
>>> +       logic into a single, reusable form, simplifying the code and 
>>> avoiding
>>> +       function call overhead. Also this bit operations API was 
>>> inherited from
>>> +       Linux and should be kept for familiarity.
>>> +     - ECLAIR has been configured to ignore these macros.
>>> +
>>> +   * - R5.5
>>> +     - Clashes between grant table ('update_gnttab_par', 
>>> 'parse_gnttab_limit')
>>> +       functions and macro names are intentional. These macros address
>>> +       differences in argument count during compile-time, effectively 
>>> discarding
>>> +       unused 2nd and 3rd parameters to avoid warnings or errors 
>>> related to them.
>>> +     - ECLAIR has been configured to ignore these macros.
>>> +
>>>     * - R5.6
>>>       - The type ret_t is deliberately defined multiple times 
>>> depending on the
>>>         type of guest to service.
>>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>>> index 3e014a6298..cba15933fe 100644
>>> --- a/docs/misra/rules.rst
>>> +++ b/docs/misra/rules.rst
>>> @@ -196,6 +196,23 @@ maintainers if you want to suggest a change.
>>>             #define f(x, y) f(x, y)
>>>             void f(int x, int y);
>>>
>>> +       Clashes between bitops functions and macro names are allowed
>>> +       because they are used for input validation and error handling.
>>> +       Example::
>>> +
>>> +           static inline void set_bit(int nr, volatile void *addr)
>>> +           {
>>> +               asm volatile ( "lock btsl %1,%0"
>>> +                              : "+m" (ADDR) : "Ir" (nr) : "memory");
>>> +           }
>>> +           #define set_bit(nr, addr) ({                            \
>>> +               if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
>>> +               set_bit(nr, addr);                                  \
>>> +           })
>>> +
>>> +       Clashes between grant table functions and macro names are allowed
>>> +       because they are used for discarding unused parameters.
>>> +
>>>     * - `Rule 5.6 <https://eur01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Fgitlab.com%2FMISRA%2FMISRA-C%2FMISRA- 
>>> C-2012%2FExample-Suite%2F- 
>>> %2Fblob%2Fmaster%2FR_05_06.c&data=05%7C02%7Cdmytro_prokopchuk1%40epam.com%7Ce9d60ddeef764dfa381208ddd0823178%7Cb41b72d04e9f4c268a69f949f367c91d%7C1%7C0%7C638895979778883822%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=crVYwr4A0vyXcUXfQ2%2FJ5EObWfr0XGLgaQ0%2FLH9pMnM%3D&reserved=0>`_
>>>       - Required
>>>       - A typedef name shall be a unique identifier
> 
> Hello Jan, Nicola.
> 
> Do you have any comments regarding this patch?
> Does it require updates/fixes?

Hmm, you have Nicola's R-b, so it's not clear to me what else you need from
him. In its present shape I'm okay for the patch to go in with somebody
else's ack (after all it's not just me who can legitimately ack it); I'm not
happy enough with the patch to give an ack myself, even if I can't offer a
good suggestion towards improvements.

Sorry, Jan