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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 6 Sep 2023, Jan Beulich wrote:
> 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.

In all fairness I dislike this too. However the rule is clear that to
make it easier to implement MISRA C checkers MISRA only supports 2
specific patterns. And I can see they have a point there in making it
easier to automatically check for correctness.

So I would go ahead with the change.