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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.03.2024 12:41, Simone Ballarin wrote:
> 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_?

Well, I prefer any measure keeping down the length of these identifiers.

Jan