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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.10.2023 02:44, Stefano Stabellini wrote:
> On Wed, 18 Oct 2023, Jan Beulich wrote:
>> On 18.10.2023 02:48, Stefano Stabellini wrote:
>>> On Mon, 16 Oct 2023, Jan Beulich wrote:
>>>> On 29.09.2023 00:24, Stefano Stabellini wrote:
>>>>> If it is not a MISRA requirement, then I think we should go for the path
>>>>> of least resistance and try to make the smallest amount of changes
>>>>> overall, which seems to be:
>>>>
>>>> ... "least resistance" won't gain us much, as hardly any guards don't
>>>> start with an underscore.
>>>>
>>>>> - for xen/include/blah.h, __BLAH_H__
>>>>> - for xen/arch/arm/asm/include/blah.h, __ASM_ARM_BLAH_H__
>>>>> - for xen/arch/x86/asm/include/blah.h, it is far less consistent, maybe 
>>>>> __ASM_X86_BLAH_H__ ?
>>>>
>>>> There are no headers in xen/include/. For (e.g.) xen/include/xen/ we
>>>> may go with XEN_BLAH_H; whether ASM prefixes are needed I'm not sure;
>>>> we could go with just ARM_BLAH_H and X86_BLAH_H?
>>>>
>>>> The primary question though is (imo) how to deal with private headers,
>>>> such that the risk of name collisions is as small as possible.
>>>
>>> Looking at concrete examples under xen/include/xen:
>>> xen/include/xen/mm.h __XEN_MM_H__
>>> xen/include/xen/dm.h __XEN_DM_H__
>>> xen/include/xen/hypfs.h __XEN_HYPFS_H__
>>>
>>> So I think we should do for consistency:
>>> xen/include/xen/blah.h __XEN_BLAH_H__
>>>
>>> Even if we know the leading underscore are undesirable, in this case I
>>> would prefer consistency.
>>
>> I'm kind of okay with that. FTAOD - here and below you mean to make this
>> one explicit first exception from the "no new leading underscores" goal,
>> for newly added headers?
> 
> Yes. The reason is for consistency with the existing header files.
> 
> 
>>> On the other hand looking at ARM examples:
>>> xen/arch/arm/include/asm/traps.h __ASM_ARM_TRAPS__
>>> xen/arch/arm/include/asm/time.h __ARM_TIME_H__
>>> xen/arch/arm/include/asm/sysregs.h __ASM_ARM_SYSREGS_H
>>> xen/arch/arm/include/asm/io.h _ASM_IO_H
>>>
>>> And also looking at x86 examples:
>>> xen/arch/x86/include/asm/paging.h _XEN_PAGING_H
>>> xen/arch/x86/include/asm/p2m.h _XEN_ASM_X86_P2M_H
>>> xen/arch/x86/include/asm/io.h _ASM_IO_H
>>>
>>> Thet are very inconsistent.
>>>
>>>
>>> So for ARM and X86 headers I think we are free to pick anything we want,
>>> including your suggested ARM_BLAH_H and X86_BLAH_H. Those are fine by
>>> me.
>>
>> To be honest, I'd prefer a global underlying pattern, i.e. if common
>> headers are "fine" to use leading underscores for guards, arch header
>> should, too.
> 
> I am OK with that too. We could go with:
> __ASM_ARM_BLAH_H__
> __ASM_X86_BLAH_H__
> 
> I used "ASM" to make it easier to differentiate with the private headers
> below. Also the version without "ASM" would work but it would only
> differ with the private headers in terms of leading underscores. I
> thought that also having "ASM" would help readability and help avoid
> confusion.
> 
> 
>>> For private headers such as:
>>> xen/arch/arm/vuart.h __ARCH_ARM_VUART_H__
>>> xen/arch/arm/decode.h __ARCH_ARM_DECODE_H_
>>> xen/arch/x86/mm/p2m.h __ARCH_MM_P2M_H__
>>> xen/arch/x86/hvm/viridian/private.h X86_HVM_VIRIDIAN_PRIVATE_H
>>>
>>> More similar but still inconsistent. I would go with ARCH_ARM_BLAH_H and
>>> ARCH_X86_BLAH_H for new headers.
>>
>> I'm afraid I don't like this, as deeper paths would lead to unwieldy
>> guard names. If we continue to use double-underscore prefixed names
>> in common and arch headers, why don't we demand no leading underscores
>> and no path-derived prefixes in private headers? That'll avoid any
>> collisions between the two groups.
> 
> OK, so for private headers:
> 
> ARM_BLAH_H
> X86_BLAH_H
> 
> What that work for you?

What are the ARM_ and X86_ prefixes supposed to indicate here? Or to ask
differently, how would you see e.g. common/decompress.h's guard named?

Jan