Re: [PATCH v5] automation/eclair: update configuration of D4.10

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sun, 22 Jun 2025, Nicola Vetrini wrote:
> On 2025-06-21 04:19, Stefano Stabellini wrote:
> > MISRA C Directive 4.10 states that "Precautions shall be taken in order
> > to prevent the contents of a header file being included more than
> > once".
> > 
> > Add a SAF tag to the existing comment on top of cpufeatures.h.
> 
> You say this, but technically the comment is not a SAF comment, just a regular
> one that is interpreted to tailor the guideline.

No, that is a mistake in the commit message. It should be changed.


> > Add a header inclusion guard to compile.h.
> > 
> > Update ECLAIR configuration to:
> > - extend existing deviation to other comments explicitly saying a file
> >   is intended for multiple inclusion;
> > - extend existing deviation to other autogenerated files;
> > - tag the guidelines as clean.
> > 
> > Update deviations.rst accordingly.
> > 
> > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> > ---
> > Changes in v5:
> > - add missing spaces in in-code comment
> > ---
> >  automation/eclair_analysis/ECLAIR/deviations.ecl | 11 +++++++----
> >  automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
> >  docs/misra/deviations.rst                        | 12 ++++++++++++
> >  xen/arch/x86/include/asm/cpufeatures.h           |  8 +++++---
> >  xen/include/xen/compile.h.in                     |  3 +++
> >  xen/tools/process-banner.sed                     |  5 +++++
> >  6 files changed, 33 insertions(+), 7 deletions(-)
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index 9c67358d46..3b5bc87e1d 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -72,11 +72,14 @@ they are not instances of commented-out code."
> >  -config=MC3A2.D4.3,reports+={deliberate,
> > "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
> >  -doc_end
> > 
> > --doc_begin="Files that are intended to be included more than once do not
> > need to
> > -conform to the directive."
> > +-doc_begin="Files that are intended to be included more than once (and have
> > +a comment that says this explicitly) do not need to conform to the
> > directive."
> >  -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file is
> > intended to be included multiple times\\. \\*/$, begin-4))"}
> 
> [1] Here
> 
> > --config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file,
> > do not edit! \\*/$, begin-3))"}
> > --config=MC3A2.D4.10,reports+={safe,
> > "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
> > +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file,
> > do not edit! \\*/$, begin-3...begin-2))"}
> > +-doc_end
> > +
> > +-doc_begin="Autogenerated files that do not need to conform to the
> > directive."
> > +-config=MC3A2.D4.10,reports+={safe,
> > "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
> >  -doc_end
> > 
> >  -doc_begin="Including multiple times a .c file is safe because every
> > function or data item
> > diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl
> > b/automation/eclair_analysis/ECLAIR/tagging.ecl
> > index f9da5d5f4d..b95f07feb0 100644
> > --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> > @@ -23,6 +23,7 @@
> >  "MC3A2.D1.1||
> >  MC3A2.D2.1||
> >  MC3A2.D4.1||
> > +MC3A2.D4.10||
> >  MC3A2.D4.11||
> >  MC3A2.D4.14||
> >  MC3A2.R1.1||
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > index fe0b1e10a2..63caa8f4a2 100644
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -30,6 +30,18 @@ Deviations related to MISRA C:2012 Directives:
> >         not to add an additional encapsulation layer.
> >       - Tagged as `deliberate` for ECLAIR.
> > 
> > +   * - D4.10
> > +     - Files that are intended to be included more than once (and have
> > +       a comment that says this explicitly) do not need to conform to the
> > +       directive.
> > +     - Tagged as `safe` for ECLAIR.
> > +
> > +   * - D4.10
> > +     - There are autogenerated files that do not need to comply to the
> > +       directive.
> > +     - Tagged as `safe` for ECLAIR. Such files are:
> > +        - xen/include/generated/autoconf.h
> > +
> >     * - D4.10
> >       - Including multiple times a .c file is safe because every function or
> > data item
> >         it defines would in (the common case) be already defined.
> > diff --git a/xen/arch/x86/include/asm/cpufeatures.h
> > b/xen/arch/x86/include/asm/cpufeatures.h
> > index 9e3ed21c02..69041219cb 100644
> > --- a/xen/arch/x86/include/asm/cpufeatures.h
> > +++ b/xen/arch/x86/include/asm/cpufeatures.h
> > @@ -1,6 +1,6 @@
> > -/*
> > - * Explicitly intended for multiple inclusion.
> > - */
> > +/* This file is intended to be included multiple times. */
> > +/* #ifndef X86_CPUFEATURES_H */
> > +/* #define X86_CPUFEATURES_H */
> > 
> 
> Are these two lines really needed? I may be mistaken, but I think the
> violation's first location would be the #include below with a comment a couple
> of lines above captured by the config at the top [1]. @Federico thoughts?

Without these 2 lines, ECLAIR complains about the following:

#include <xen/lib/x86/cpuid-autogen.h>
       ^
 MC3A2.D4.10non-compliant start of header file


https://gitlab.com/xen-project/people/sstabellini/xen/-/jobs/10420715821

https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/sstabellini/xen/ECLAIR_normal/ppp3/X86_64/10420715821/PROJECT.ecd;/sources/xen/arch/x86/include/asm/cpufeatures.h.html#R1_1{%22select%22:true,%22selection%22:{%22hiddenAreaKinds%22:[],%22hiddenSubareaKinds%22:[],%22show%22:false,%22selector%22:{%22enabled%22:true,%22negated%22:true,%22kind%22:0,%22domain%22:%22kind%22,%22inputs%22:[{%22enabled%22:true,%22text%22:%22violation%22}]}}}