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
