[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v2 00/10] address violations of MISRA C:2012 Directive 4.10
On Mon, 23 Oct 2023, Jan Beulich wrote: > On 21.10.2023 01:26, Stefano Stabellini wrote: > > On Fri, 20 Oct 2023, Jan Beulich wrote: > >> On 19.10.2023 18:19, Stefano Stabellini wrote: > >>> On Thu, 19 Oct 2023, Jan Beulich wrote: > >>>> 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? > >>> > >>> I meant that: > >>> > >>> xen/arch/arm/blah.h would use ARM_BLAH_H > >>> xen/arch/x86/blah.h would use X86_BLAH_H > >>> > >>> You have a good question on something like xen/common/decompress.h and > >>> xen/common/event_channel.h. What do you think about: > >>> > >>> COMMON_BLAH_H, so specifically COMMON_DECOMPRESS_H > >>> > >>> otherwise: > >>> > >>> XEN_BLAH_H, so specifically XEN_DECOMPRESS_H > >>> > >>> I prefer COMMON_BLAH_H but I think both versions are OK. > >> > >> IOW you disagree with my earlier "... and no path-derived prefixes", > >> and you prefer e.g. DRIVERS_PASSTHROUGH_VTD_DMAR_H as a consequence? > >> FTAOD my earlier suggestion was simply based on the observation that > >> the deeper the location of a header in the tree, the more unwieldy > >> its guard name would end up being if path prefixes were to be used. > > > > I don't have a strong opinion on "path-derived prefixes". I prefer > > consistency and easy-to-figure-out guidelines over shortness. > > > > The advantage of a path-derived prefix is that it is trivial to figure > > out at first glance. If we can come up with another system that is also > > easy then fine. Do you have a suggestion? If so, sorry I missed it. > > Well, I kind of implicitly suggested "no path derived prefixes for private > headers", albeit realizing that there's a chance then of guards colliding. > I can't think of a good scheme which would fit all goals (no collisions, > uniformity, and not unduly long). Here I think we would benefit from a third opinion. Julien? Anyone?
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |