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
