Re: [XEN PATCH v2 00/10] address violations of MISRA C:2012 Directive 4.10

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

On 28/09/23 17:00, Jan Beulich wrote:
> On 28.09.2023 15:17, Simone Ballarin wrote:
> > On 28/09/23 14:51, Jan Beulich wrote:
> > > On 28.09.2023 14:46, Simone Ballarin wrote:
> > > > On 13/09/23 10:02, Jan Beulich wrote:
> > > > > On 12.09.2023 11:36, Simone Ballarin wrote:
> > > > > > Add or move 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.
> > > > > >
> > > > > > Simone Ballarin (10):
> > > > > >      misra: add deviation for headers that explicitly avoid guards
> > > > > >      misra: modify deviations for empty and generated headers
> > > > > >      misra: add deviations for direct inclusion guards
> > > > > >      xen/arm: address violations of MISRA C:2012 Directive 4.10
> > > > > >      xen/x86: address violations of MISRA C:2012 Directive 4.10
> > > > > >      x86/EFI: address violations of MISRA C:2012 Directive 4.10
> > > > > >      xen/common: address violations of MISRA C:2012 Directive 4.10
> > > > > >      xen/efi: address violations of MISRA C:2012 Directive 4.10
> > > > > >      xen: address violations of MISRA C:2012 Directive 4.10
> > > > > >      x86/asm: address violations of MISRA C:2012 Directive 4.10
> > > > > Just to mention it here again for the entire series, seeing that despite
> > > > > my earlier comments to this effect a few R-b have arrived: If private
> > > > > headers need to gain guards (for, imo, no real reason), we first need to
> > > > > settle on a naming scheme for these guards, such that guards used in
> > > > > private headers aren't at risk of colliding with ones used headers
> > > > > living in one of the usual include directories. IOW imo fair parts of
> > > > > this series may need redoing.
> > > > >
> > > > > Jan
> > > > My proposal is:
> > > > - the relative path from "xen/arch" for files in this directory
> > > >      (i.e. X86_X86_X86_MMCONFIG_H for "xen/arch/x86/x86_64/mmconfig.h";
> > > X86_X86_64_MMCONFIG_H that is?
> > >
> > > Yet then this scheme won't hold for xen/arch/include/asm/... ? It's also
> > > not clear whether you're deliberately omitting leading/trailing underscores
> > > here, which may be a way to distinguish private from global headers.
> > Each name that begins with a double or single underscore (__, _)
> > followed by an uppercase letter is reserved. Using a reserved identifier
> > is an undefined-b.
> >
> > I would be better to avoid them.
> I'm with you about avoiding them, except that we use such all over the
> place. Taking this together with ...
>
> > > > - for the others, the entire path.
> > > What exactly is "entire" here?
> > Let me try again.
> >
> > If we are inside xen/arch the relative path starting from this directory:
> >     | xen/arch/x86/include/asm/compat.h
> >     X86_INCLUDE_ASM_COMPAT_H
> >
> > For xen/include, the current convention.
> > Maybe, in a future patch, we can consider removing the leading _.
> >
> > For the others, the relative path after xen:
> >     | xen/common/efi/efi.h
> >     COMMON_EFI_EFI_H
> ... this you're effectively suggesting to change all existing guards.
> That's an option, but likely not a preferred one. Personally I'd prefer
> if in particular the headers in xen/include/ and in xen/arch/*include/
> didn't needlessly include _INCLUDE_ in their guard names.
>
> I'm really curious what others think.
>
> Jan
I suggest starting using this new naming schema just on the files 
touched by this series: I do not think is a good idea changing all
guards now.

This should be done in any case if you will decide to be compliant
with Rule 21.2 (Set3):
(required) A reserved identifier or reserved macro name shall not be 
declared.


--
Simone Ballarin, M.Sc.

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