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

[Jan Beulich <jbeulich@xxxxxxxx>]

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?

> 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.

> 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.

Jan