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)