Re: [PATCH 1/6] xen/arm: add inclusion guards

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 17/05/2025 00:21, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@xxxxxxxxxxx>
>
> MISRA C Directive 4.10 states that:
> "Precautions shall be taken in order to prevent the contents of a
> header file being included more than once".
>
> Add inclusion guards where missing to address violations of the
> guideline.
>
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
With one remark below:

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

> ---
>   xen/arch/arm/efi/efi-boot.h        | 6 ++++++
>   xen/arch/arm/include/asm/efibind.h | 5 +++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index dcad46ca72..d2a09ad3a1 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
I always found weird that this file is treated as a header when in fact 
this is just a disguised source file. So in some way...
> @@ -3,6 +3,10 @@
>    * is intended to be included by common/efi/boot.c _only_, and
>    * therefore can define arch specific global variables.
>    */
> +
> +#ifndef ARM_EFI_BOOT_H
> +#define ARM_EFI_BOOT_H
... without the header guard, we could catch two inclusions of 
efi-boot.h. I would consider to use:
#ifdef ARM_EFI_BOOT_H
# error ...
#else
# define ...

#endif ...

Cheers,

--
Julien Grall