Re: [XEN PATCH 04/13] xen/x86: address violations of MISRA C:2012 Directive 4.10

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.08.2023 15:20, Simone Ballarin wrote:
> Add or move inclusion guards to address violations of
> MISRA C:2012 Directive 4.10 ("Precautions shall be taken in order
> to prevent the contents of a header file being included more than
> once").
> 
> Inclusion guards must appear at the beginning of the headers
> (comments are permitted anywhere).
> 
> Also C files, if included somewhere, need to comply with the guideline.
> 
> Mechanical change.
> 
> Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
> ---
>  xen/arch/x86/Makefile                  | 8 ++++----
>  xen/arch/x86/cpu/cpu.h                 | 5 +++++
>  xen/arch/x86/physdev.c                 | 4 ++++
>  xen/arch/x86/platform_hypercall.c      | 5 +++++
>  xen/arch/x86/x86_64/compat/mm.c        | 5 +++++
>  xen/arch/x86/x86_64/mmconfig.h         | 5 +++++
>  xen/arch/x86/x86_emulate/private.h     | 5 +++++
>  xen/arch/x86/x86_emulate/x86_emulate.c | 5 +++++
>  8 files changed, 38 insertions(+), 4 deletions(-)

Considering that the description talks of header files alone, there's a
lot of non-header-file churn here.

> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -259,17 +259,17 @@ $(objtree)/arch/x86/include/asm/asm-macros.h: 
> $(obj)/asm-macros.i $(src)/Makefil
>       $(call filechk,asm-macros.h)
>  
>  define filechk_asm-macros.h
> +    echo '#ifndef __ASM_MACROS_H__'; \
> +    echo '#define __ASM_MACROS_H__'; \
>      echo '#if 0'; \
>      echo '.if 0'; \
>      echo '#endif'; \
> -    echo '#ifndef __ASM_MACROS_H__'; \
> -    echo '#define __ASM_MACROS_H__'; \
>      echo 'asm ( ".include \"$@\"" );'; \
> -    echo '#endif /* __ASM_MACROS_H__ */'; \
>      echo '#if 0'; \
>      echo '.endif'; \
>      cat $<; \
> -    echo '#endif'
> +    echo '#endif'; \
> +    echo '#endif /* __ASM_MACROS_H__ */'
>  endef

Can you please explain why this needs adjustment? While I think things
are going to be okay with the adjustment, this dual C and assembler
construct would imo better be left alone. Plus as per context found in
patch 2, aren't generated headers excluded anyway?

Jan