Re: [XEN PATCH v3 03/16] misra: add deviations for direct inclusion guards

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

On 11/03/24 11:08, Jan Beulich wrote:
> On 11.03.2024 09:59, Simone Ballarin wrote:
> > Add deviation comments 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).
> >
> > This patch adds deviation comments using the format specified
> > in docs/misra/safe.json for headers with just the direct
> > inclusion guard before the inclusion guard since they are
> > safe and not supposed to comply with the directive.
> >
> > Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
> >
> > ---
> > Changes in v3:
> > - fix inconsistent deviation ID
> > The patch has been introduced in v2.
> > ---
> >   docs/misra/safe.json                 | 8 ++++++++
> >   xen/arch/arm/include/asm/hypercall.h | 1 +
> >   xen/arch/x86/include/asm/hypercall.h | 1 +
> >   3 files changed, 10 insertions(+)
> What about asm-generic/hypercall.h?
Apparently the file is not part of the analysed build.

> > --- a/xen/arch/arm/include/asm/hypercall.h
> > +++ b/xen/arch/arm/include/asm/hypercall.h
> > @@ -1,3 +1,4 @@
> > +/* SAF-5-safe direct inclusion guard before */
> >   #ifndef __XEN_HYPERCALL_H__
> >   #error "asm/hypercall.h should not be included directly - include xen/hypercall.h 
> > instead"
> >   #endif
> > --- a/xen/arch/x86/include/asm/hypercall.h
> > +++ b/xen/arch/x86/include/asm/hypercall.h
> > @@ -2,6 +2,7 @@
> >    * asm-x86/hypercall.h
> >    */
> >   
> > +/* SAF-5-safe direct inclusion guard before */
> >   #ifndef __XEN_HYPERCALL_H__
> >   #error "asm/hypercall.h should not be included directly - include xen/hypercall.h 
> > instead"
> >   #endif
> Iirc it was said that this way checking for correct guards is suppressed
> altogether in Eclair, which is not what we want. Can you clarify this,
> please?
My first change was moving this check inside the guard.
You commented my patch saying that this would be an error because someone can
include it directly if it has already been included indirectly.
I replied telling that this was the case also before the change.
You agreed with me, and we decided that the correct thing would be fixing the
check and not apply my temporary change to address the finding.

Considering that the code should be amended, a SAF deviation seems to me
the most appropriate way for suppressing these findings.

> Jan
--
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)