Re: [XEN PATCH 12/13] xen: address violations of MISRA C:2012 Directive 4.10

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.09.2023 00:27, Stefano Stabellini wrote:
> On Thu, 31 Aug 2023, Simone Ballarin wrote:
>> On 29/08/23 00:51, Stefano Stabellini wrote:
>>> On Mon, 28 Aug 2023, Simone Ballarin wrote:
>>>> Move or amended inclusion guards to address violations of
>>>> MISRA C:2012 Directive 4.10 ("Precautions shall be taken in order
>>>> to prevent the contents of a header file being included more than
>>>> once").
>>>>
>>>> Inclusion guards must appear at the beginning of the headers
>>>> (comments are permitted anywhere) and the #if directive cannot
>>>> be used for other checks.
>>>>
>>>> Mechanical change.
>>>>
>>>> Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
>>>> ---
>>>>   xen/include/xen/err.h       | 4 +++-
>>>>   xen/include/xen/pci_ids.h   | 5 +++++
>>>>   xen/include/xen/softirq.h   | 4 +++-
>>>>   xen/include/xen/unaligned.h | 7 ++++---
>>>>   xen/include/xen/vmap.h      | 4 +++-
>>>>   5 files changed, 18 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
>>>> index 2f29b57d28..a6323d82d7 100644
>>>> --- a/xen/include/xen/err.h
>>>> +++ b/xen/include/xen/err.h
>>>> @@ -1,5 +1,6 @@
>>>> -#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__)
>>>> +#if !defined(__XEN_ERR_H__)
>>>>   #define __XEN_ERR_H__
>>>> +#if !defined(__ASSEMBLY__)
>>>
>>> The original pattern was also guarding the header file sufficiently,
>>> protecting it from double-inclusion. In fact, it is posing stricter
>>> restrictions than usual (not laxer). This change is unnecessary?
>>
>> The MISRA directive asks to use one of the two following forms:
>>
>> <start-of-file>
>> #if !defined ( identifier )
>> #define identifier
>> /* Contents of file */
>> #endif
>> <end-of-file>
>>
>> <start-of-file>
>> #ifndef identifier
>> #define identifier
>> /* Contents of file */
>> #endif
>> <end-of-file>
>>
>> I do not see any reason for deviating, but if you ask that, I can do it.
> 
> Let's follow MISRA's form.

This is what I strongly dislike: They could be less restrictive on the
exact patterns permitted without impacting the goal intended to be
reached. But it's all as simple as possible, not as flexible as possible.

In any event, if a transformation like what can still be seen in context
is to be made, then please #ifdef / #ifndef instead of defined(...)
whenever possible.

Jan