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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 3 Jul 2024, Jan Beulich wrote:
> On 01.07.2024 15:46, Alessandro Zucchelli wrote:
> > This section explains which format should be followed by header
> > inclusion guards via a drop-down list of rules.
> 
> Ah, so this answers my earlier question regarding where the naming
> rules are spelled out. Yet why is this not much earlier in the series,
> /before/ anything trying to follow these rules?
> 
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -167,3 +167,22 @@ the end of files.  It should be:
> >   * indent-tabs-mode: nil
> >   * End:
> >   */
> 
> This footer is not just an example; it also serves that function here.
> While not strictly needed in this file, I think it should still remain
> last.

+1


> > +Header inclusion guards
> > +-----------------------
> > +
> > +Unless differently specified all header files should have proper inclusion
> > +guards in order to avoid being included multiple times.
> > +The following naming conventions have been devised:
> > +- 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 */
> 
> GENERIC contradicts X86. Please try to avoid giving confusing / possibly
> misleading examples.

For clarity, Jan means that GENERIC by definition is not arch-specific
so GENERIC_X86 or GENERIC_ARM is a contradiction and it would be better
not to use it as reference example for this rule


> > +- 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 */
> 
> I'm afraid I can't connect the example to the filename pattern given:
> The example has 4 components separated by 3 underscores, when the
> pattern suggests 5 and 4 respectively.

I read the above with <subdir> being optional. But yes it is unclear
because the example should have both the header guard but also the
related file path. In this case the file path corresponding to
ASM_X86_DOMAIN_H would be arch/x86/include/asm/domain.h



> Please avoid empty lines at the bottom of files.
> 
> Having reached the end, I don't see common headers (the ones under
> xen/include/ in the tree) covered. I can only conclude that the odd
> INCLUDE_ prefixes I had asked about were derived from the "private
> headers" rule. Yet what's in xen/include/ aren't private headers.

Yeah. I proposed in a previous email to use:

- xen/include/xen/<filename>.h -> XEN_<filename>_H
- xen/include/xen/<subdir>/<filename>.h -> XEN_<subdir>_<filename>_H

with <subdir> being optional.


> 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."


> Finally, it shouldn't be silently assumed that all name components are
> to be converted to upper case; everything wants spelling out imo.
 
+1