Re: [XEN PATCH v3 00/16] xen: address violation of MISRA C:2012 Directive 4.10

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

On 11/03/24 10:59, Jan Beulich wrote:
> On 11.03.2024 09:59, Simone Ballarin wrote:
> > The Xen sources contain violations of MISRA C:2012 Directive 4.10 whose 
> > headline states:
> > "Precautions shall be taken in order to prevent the contents of a header file
> > being included more than once".
> >
> > As stated in v2, the following naming convention has been estabilished:
> > - arch/.../include/asm/ headers -> ASM_<filename>_H
> > - private headers -> <dir>_<filename>_H
> > - asm-generic headers -> ASM_GENERIC_<filename>_H
> >
> > Since there would have been conflicting guards between architectures (which is 
> > a violation
> > of the directive),
> Why would this be a violation? The two sets of headers aren't use together,
> and any scan should only point out collisions in what's actively used, I'd
> hope.
A header guard cannot be used multiple times in the same project, so it's a 
question
of whether or not we want to consider each arch variant part of the same 
project.
In case, we can define a new rule for these files.

> > there has been a need for ASM headers to specify  if the inclusion guard
> > referred to ARM or X86. Hence it has been decided to adopt instead:
> > - arch/<architecture>/include/asm/<subdir>/<filename>.h -> 
> > ASM_<architecture>_<subdir>_<filename>_H
> >
> > The subdir used is the smallest possible to avoid collisions. For example, it 
> > has been
> > observed that in "xen/arch/arm/include/asm/arm32" and 
> > "xen/arch/arm/include/asm/arm64" there
> > are plenty of header files with the same name, hence  _ARMxx_ was added as 
> > subdirectory.
> Why couldn't <arch>_<subdir> together be ARMxx there, allowing us to get
> away with one less name component.
I agree.

> > There has been a need to define a standard for generated headers too:
> >   - include/generated/<subdir>/<filename>.h-> GENERATED_<subdir>_<filename>_H
> >   - arch/<architecture>/include/generated/asm/<filename>.h-> 
> > <arch>_GENERATED_ASM_<name>_H
> >
> > To summarize, here are all the rules that have been applied:
> > - private headers -> <dir>_<filename>_H
> And <dir> here is the full path? I thought I had indicated before that
> this is going to lead to excessively long identifiers.
Yes, dir is the full path. This general fallback rule to use when more specific 
rules do not apply.
If this generates excessively long guards, we can simply add more rules.
> > - asm-generic headers -> ASM_GENERIC_<filename>_H
> > - arch/<architecture>/include/asm/<subdir>/<filename>.h -> 
> > ASM_<architecture>_<subdir>_<filename>_H
> > - include/generated/<subdir>/<filename>.h-> GENERATED_<subdir>_<filename>_H
> > - arch/<architecture>/include/generated/asm/<filename>.h-> 
> > <arch>_GENERATED_ASM_<name>_H
> And _ASM_ is merely a precaution for stuff appearing outside of asm/ (which
> doesn't seem very likely)?
Yes, it is. Do you prefer dropping the _ARM_?

> Jan
--
Simone Ballarin, M.Sc.

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