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
