Re: [XEN PATCH v6] CODING_STYLE: Add a section on header guards naming conventions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 10 Sep 2024, Jan Beulich wrote:
> On 10.09.2024 06:57, Stefano Stabellini wrote:
> > On Mon, 9 Sep 2024, Jan Beulich wrote:
> >> On 05.09.2024 17:48, Alessandro Zucchelli wrote:
> >>> This section explains which format should be followed by header
> >>> inclusion guards via a drop-down list of rules.
> >>>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@xxxxxxxxxxx>
> >>>
> >>> ---
> >>> Changes in v6:
> >>> - edit inclusion guards naming conventions, including more details
> >>
> >> Yet I'm afraid that from my pov we're still not there. Specifically ...
> >>
> >>> --- a/CODING_STYLE
> >>> +++ b/CODING_STYLE
> >>> @@ -159,6 +159,34 @@ Emacs local variables
> >>>  A comment block containing local variables for emacs is permitted at
> >>>  the end of files.  It should be:
> >>>  
> >>> +Header inclusion guards
> >>> +-----------------------
> >>> +
> >>> +Unless otherwise specified, all header files should include proper
> >>> +guards to prevent multiple inclusions. The following naming conventions
> >>> +apply:
> >>
> >> ... reading this, I can't derive ...
> >>
> >>> +- 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
> >>
> >> ... the absence of an equivalent of the arch/ part of the path. As per
> >> my recollection we agreed on that shortening, but it needs spelling out
> >> in the textual description. Such that it is possible to derived what to
> >> uses as a name for, say, a header under common/, crypto/, or drivers/
> >> (or anywhere else of course). Specifically with the further examples ...
> > 
> > Are you asking for something like this?
> > 
> > Omit the word "arch" from the filepath.
> > 
> > If you prefer an alternative wording please suggest the text. 
> > 
> > 
> >>> +- asm-generic headers: ASM_GENERIC__<filename>_H
> >>> +    - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H
> >>> +
> >>> +- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H
> >>> +    - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H
> >>
> >> ... here and ...
> > 
> > Suggested text:
> > 
> > Omit the words "arch" and "include/asm" from the filepath, ASM is also
> > prefixed.
> > 
> > 
> >>> +- Xen headers: XEN__<filename>_H
> >>> +    - include/xen/something.h -> XEN__SOMETHING_H
> >>
> >> ... here, where more than just one path component is omitted, deriving
> >> what's meant can end up ambiguous. Yet ambiguity is what we absolutely
> >> want to avoid, to preempt later discussions on any such naming.
> > 
> > Suggested text:
> > 
> > Omit the words "include/xen" from the filepath, XEN is always prefixed.
> > 
> > Please suggest a specific alternative if you prefer
> 
> Looks like I still didn't get across my point: The verbal description
> that's ahead of all of the examples should be complete enough to describe
> the whole set of rules, in sufficiently abstract terms. Then the examples
> will be easy to prove as fitting those rules, and it will be easy to
> derive the naming for further identifiers. IOW - no, I'm not asking for
> the examples to be further commented, but for the naming rules to be
> _fully_ spelled out.


Hi Jan, we have gone back and forth on this a few times, but neither
Alessandro nor I fully understand your perspective. To help streamline
the process and save time for everyone, I suggest you provide an example
of the rules written in the style you believe is appropriate. Once you
set the initial direction, Alessandro and I can continue and complete
the rest in that preferred style.

On a related note, I have encountered formal specifications that use less
formal language than this simple code style and naming convention
adjustment. I feel we might be over-engineering this, and in my opinion,
the current version is sufficient. Any additional time spent on this
could be better used addressing MISRA violations that pose real safety
risks for Xen users.