Re: [PATCH] CODING_STYLE: Updated header guard recommendations

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 16 May 2025, Andrew Cooper wrote:
> Despite the best intentions at the time, the current recommendation
> lead to very long identifiers, bordering on the max limit we've chosen
> for certification.
> 
> One observation is that we do have static analysis which will
> highlight if duplicate guards are created accidentally.
> 
> Therefore, relax the recommendations and in particular remove the
> specific tie to the directory structure.  This has the other advantage
> of being more similar to other projects.
> 
> This will hopefully mean there's less churn getting the tree in shape,
> and a random contributor is more likely to pick an appropriate guard
> given no specific knowledge of Xen.
> 
> As always, it's something reviewers and maintainers should be aware
> of, and to advise on.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
>  CODING_STYLE | 49 ++++++++++++++++++-------------------------------
>  1 file changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index e3b1da604802..5644f1697fc7 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -157,43 +157,30 @@ Header inclusion guards
>  -----------------------
>  
>  Unless otherwise specified, all header files should include proper
> -guards to prevent multiple inclusions. The following naming conventions
> -apply:
> -
> -- Guard names are derived from directory path underneath xen/ and the
> -  actual file name.  Path components are separated by double
> -  underscores.  Alphabetic characters are converted to upper case.  Non-
> -  alphanumeric characters are replaced by single underscores.
> -- Certain directory components are omitted, to keep identifier length
> -  bounded:
> -  - the top level include/,
> -  - architecture-specific private files' arch/,
> -  - any architecture's arch/<arch>/include/asm/ collapses to
> -    ASM__<ARCH>__.
> +guards to prevent multiple inclusions.  Guards need to be unique, and
> +this property is checked by static analysis.
>  
> -For example:
> +Guards should be chosen based on the logical area, with enough
> +disambiguation when the same filename exits in multiple locations in
> +the source tree.  Commonly there should be a XEN or <arch> prefix.
> +The guard should be spelt in ALL CAPITALS, ending with _H.
>  
> -- Xen headers: XEN__<filename>_H
> -  - include/xen/something.h -> XEN__SOMETHING_H
> +For example:
>  
> -- asm-generic headers: ASM_GENERIC__<filename>_H
> -  - include/asm-generic/something.h -> ASM_GENERIC__SOMETHING_H
> +- Xen headers: XEN_<something>_H
> +  - include/xen/something.h -> XEN_SOMETHING_H
>  
> -- arch-specific headers: ASM__<architecture>__<subdir>__<filename>_H
> -  - arch/x86/include/asm/something.h -> ASM__X86__SOMETHING_H
> +- arch-specific headers: <arch>_<something>_H
> +  - arch/x86/include/asm/something.h -> X86_SOMETHING_H
> +  - arch/x86/include/asm/hvm/something.h -> X86_HVM_SOMETHING_H
> +  - arch/x86/include/asm/pv/something.h -> X86_PV_SOMETHING_H
>  
> -- Private headers: <dir>__<filename>_H
> -  - arch/arm/arm64/lib/something.h -> ARM__ARM64__LIB__SOMETHING_H
> -  - arch/x86/lib/something.h -> X86__LIB__SOMETHING_H
> -  - common/something.h -> COMMON__SOMETHING_H
> +- Private headers: <something>_PRIVATE_H
> +  - common/something/private.h -> <SOMETHING>_PRIVATE_H
> +  - drivers/foo/something.h -> <SOMETHING>_H
>  
> -Note that this requires some discipline on the naming of future new
> -sub-directories: There shouldn't be any other asm/ one anywhere, for
> -example.  Nor should any new ports be named the same as top-level
> -(within xen/) directories.  Which may in turn require some care if any
> -new top-level directories were to be added.  Rule of thumb: Whenever
> -adding a new subdirectory, check the rules to prevent any potential
> -collisions.
> +A good choice of guard is one that wont become stale if the
> +driver/subsystem/etc is shuffled around the source tree.
>  
>  Emacs local variables
>  ---------------------
> -- 
> 2.34.1
>