[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN PATCH 03/13] xen/arm: address violations of MISRA C:2012 Directive 4.10
On 29/08/23 00:10, Julien Grall wrote:
Hi,
On Mon, 28 Aug 2023 at 09:20, Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
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).
Mechanical change.
Signed-off-by: Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
---
xen/arch/arm/efi/efi-boot.h | 6 ++++++
xen/arch/arm/include/asm/hypercall.h | 6 +++---
xen/arch/arm/include/asm/iocap.h | 6 +++---
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 1c3640bb65..aba522ead5 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -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_EFI_BOOT_H__
+#define __ARM_EFI_EFI_BOOT_H__
+
#include <xen/device_tree.h>
#include <xen/libfdt/libfdt.h>
#include <asm/setup.h>
@@ -1003,6 +1007,8 @@ static void __init efi_arch_flush_dcache_area(const
void *vaddr, UINTN size)
__flush_dcache_area(vaddr, size);
}
+#endif /* __ARM_EFI_EFI_BOOT_H__*/
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/include/asm/hypercall.h
b/xen/arch/arm/include/asm/hypercall.h
index ccd26c5184..4f4d96f1f2 100644
--- a/xen/arch/arm/include/asm/hypercall.h
+++ b/xen/arch/arm/include/asm/hypercall.h
@@ -1,10 +1,10 @@
+#ifndef __ASM_ARM_HYPERCALL_H__
+#define __ASM_ARM_HYPERCALL_H__
+
#ifndef __XEN_HYPERCALL_H__
#error "asm/hypercall.h should not be included directly - include
xen/hypercall.h instead"
#endif
-#ifndef __ASM_ARM_HYPERCALL_H__
-#define __ASM_ARM_HYPERCALL_H__
-
I understand that you are trying to fix a misra violation. However, this
feels like it was done on purpose.
With the new change, you would not always check that the file were included
at the correct place. I am not against this change but this ought to be
explained.
I don't think the semantics have changed. Please correct me if I'm wrong.
With this change, the only situation where the check is not performed is
when __ASM_ARM_HYPERCALL_H__ is defined (i.e. the file has already been
successfully included). This implies that if __ASM_ARM_HYPERCALL_H__ is
defined, then __XEN_HYPERCALL_H__ is also defined, so the check would be
useless.
The same thing happened with the code before the change: if I include
the file after xen/hypercall.h, the check will always succeed.
#include <public/domctl.h> /* for arch_do_domctl */
long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
diff --git a/xen/arch/arm/include/asm/iocap.h
b/xen/arch/arm/include/asm/iocap.h
index 276fefbc59..4db1b16839 100644
--- a/xen/arch/arm/include/asm/iocap.h
+++ b/xen/arch/arm/include/asm/iocap.h
@@ -1,10 +1,10 @@
-#ifndef __X86_IOCAP_H__
-#define __X86_IOCAP_H__
+#ifndef __ASM_ARM_IOCAP_H__
+#define __ASM_ARM_IOCAP_H__
#define cache_flush_permitted(d) \
(!rangeset_is_empty((d)->iomem_caps))
-#endif
+#endif /* __ASM_ARM_IOCAP_H__ */
I don’t understand how this is related to the rest of the patch. You wrote
that inclusion must appear first and this is the case here.
However the name is technically not correct. Is this really related to
directive 4.10? If so, this should be clarified in the commit message. If
not, then I think this should be in a separate commit.
Yes, you are right. This is not correlated to this series. I will put it
on a separate commit.
Cheers,
/*
* Local variables:
--
2.34.1
--
Simone Ballarin, M.Sc.
Field Application Engineer, BUGSENG (https://bugseng.com)
|
