Re: [RFC PATCH 17/17] CODING_STYLE: Add a section on header guards naming conventions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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


> > two directories like this could collide:
> > 
> > - arch/arm/arm64/lib/something.h -> LIB_SOMETHING_H
> > - arch/arm/arm32/lib/something.h -> LIB_SOMETHING_H
> > - arch/x86/lib/something.h -> LIB_SOMETHING_H
> > 
> > (Leaving aside that in this example it would not be an issue because the
> > three headers are not meant to be all included in the same file.)
> > 
> > Can we specify that <dir> should go all the way back to the arch/ or or
> > common or drivers directory?
> > 
> > - 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
> 
> We can of course, so long as we're okay(ish) with the length and redundancy. 
> As
> already indicated before, there are downsides to this. Yet a firm scheme with
> few rules has the benefit that it might even be possible to use a script to do
> all the guard adjustments.

I also like the firm scheme with few rules