[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 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?



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.