[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 17/17] CODING_STYLE: Add a section on header guards naming conventions
On 18.07.2024 01:02, Stefano Stabellini wrote: > On Wed, 17 Jul 2024, Jan Beulich wrote: >> On 17.07.2024 02:20, Stefano Stabellini wrote: >>> On Tue, 16 Jul 2024, Jan Beulich wrote: >>>> On 16.07.2024 02:43, Stefano Stabellini wrote: >>>>> On Mon, 15 Jul 2024, Jan Beulich wrote: >>>>>> On 13.07.2024 00:38, Stefano Stabellini wrote: >>>>>>> On Wed, 3 Jul 2024, Jan Beulich wrote: >>>>>>>> I further have to note that, as indicated during the earlier >>>>>>>> discussion, >>>>>>>> I still cannot see how occasional ambiguity is going to be dealt with. >>>>>>>> IOW from the rules above two different headers could still end up with >>>>>>>> the same guard identifier. >>>>>>> >>>>>>> Maybe something like this? >>>>>>> >>>>>>> "In the event of naming collisions, exceptions to the coding style may >>>>>>> be made at the discretion of the contributor and maintainers." >>>>>> >>>>>> Hmm, maybe I wasn't clear enough then. My take is that the scheme should >>>>>> simply not allow for possible collisions. Neither the contributor nor the >>>>>> reviewer may spot such a collision, and it may therefore take until the >>>>>> first full scan that one is actually noticed. Which I consider too late >>>>>> in the process, even if we already were at the point where commits were >>>>>> checked pre-push. >>>>> >>>>> Looking at the proposal, copy/pasted here for convenience: >>>>> >>>>> - private headers -> <dir>_<filename>_H >>>>> - asm-generic headers -> ASM_GENERIC_<filename>_H >>>>> - #ifndef ASM_GENERIC_X86_PERCPU_H >>>>> #define ASM_GENERIC_X86_PERCPU_H >>>>> //... >>>>> #endif /* ASM_GENERIC_X86_PERCPU_H */ >>>>> - arch/<architecture>/include/asm/<subdir>/<filename>.h -> >>>>> ASM_<architecture>_<subdir>_<filename>_H >>>>> - #ifndef ASM_X86_DOMAIN_H >>>>> #define ASM_X86_DOMAIN_H >>>>> //... >>>>> #endif /* ASM_X86_DOMAIN_H */ >>>>> - xen/include/xen/<filename>.h -> XEN_<filename>_H >>>>> - xen/include/xen/<subdir>/<filename>.h -> XEN_<subdir>_<filename>_H >>>>> >>>>> >>>>> The only possibility for collision that I can see is from the first >>>>> point: >>>>> >>>>> - private headers -> <dir>_<filename>_H >>>> >>>> I don't think this is the only possibility of collisions. The >>>> <subdir>_<filename> >>>> parts can similarly cause problems if either of the two involved names >>>> contains >>>> e.g. a dash (which would need converting to an underscore) or an >>>> underscore. To >>>> avoid this, the name separators (slashes in the actual file names) there >>>> may need >>>> representing by double underscores. >>> >>> I am OK with you two underscores as name separator (slashes in the >>> actual file names). Would you do it for all levels like this? >>> >>> - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H >>> - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H >>> - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H >>> >>> >>> I think it is better than the below: >>> >>> - arch/arm/arm64/lib/something.h -> ARM_ARM64__LIB__SOMETHING_H >>> - arch/arm/arm32/lib/something.h -> ARM_ARM32__LIB__SOMETHING_H >>> - arch/x86/lib/something.h -> X86_LIB__SOMETHING_H >> >> Hmm, maybe it's indeed better to do it entirely uniformly then. > > > Do we have agreement on the naming convention then? > > > - private headers -> <dir>__<filename>__H > - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H > - arch/arm/arm32/lib/something.h -> ARM__ARM32__LIB__SOMETHING_H > - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H > > - asm-generic headers -> ASM_GENERIC_<filename>_H > - include/asm-generic/percpu.h -> ASM_GENERIC_X86_PERCPU_H > > - arch/<architecture>/include/asm/<subdir>/<filename>.h -> > ASM_<architecture>_<subdir>_<filename>_H > - arch/x86/include/asm/domain.h -> ASM_X86_DOMAIN_H > > - include/xen -> XEN_<filename>_H > - include/xen/percpu.h -> XEN_PERCPU_H > > > Or do you prefer the double underscore __ in all cases? It's not so much prefer, but a requirement if we want to be future proof. Even for ASM_GENERIC_* that'll be needed, as your outline above simply doesn't mention the (future) case of there being subdir-s there (see how Linux already has some). Imo the question doesn't even arise for XEN_*, as xen/ has subdir-s already. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |