[PATCH 0/6] MISRA D4.10: fix header guards

[PATCH 0/6] MISRA D4.10: fix header guards

[Stefano Stabellini]

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".

Fix few remaining header guards, and update ECLAIR configuration

Federico Serafini (6):
  xen/arm: add inclusion guards
  xen/x86: add inclusion guards
  xen: add inclusion guards
  xen: refactor include guards
  x86/asm: refactor inclusion guards
  automation/eclair: update configuration of D4.10

 automation/eclair_analysis/ECLAIR/deviations.ecl | 14 +++++++++++---
 automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
 docs/misra/deviations.rst                        | 15 +++++++++++++++
 xen/arch/arm/efi/efi-boot.h                      |  6 ++++++
 xen/arch/arm/include/asm/efibind.h               |  5 +++++
 xen/arch/x86/Makefile                            |  8 ++++----
 xen/arch/x86/cpu/cpu.h                           |  6 ++++++
 xen/arch/x86/efi/efi-boot.h                      |  6 ++++++
 xen/arch/x86/efi/runtime.h                       |  5 +++++
 xen/arch/x86/include/asm/compat.h                |  5 +++++
 xen/arch/x86/include/asm/efibind.h               |  5 +++++
 xen/arch/x86/x86_64/mmconfig.h                   |  5 +++++
 xen/arch/x86/x86_emulate/private.h               |  5 +++++
 xen/common/decompress.h                          |  5 +++++
 xen/common/efi/efi.h                             |  5 +++++
 xen/common/event_channel.h                       |  5 +++++
 xen/include/xen/err.h                            | 10 +++++++---
 xen/include/xen/pci_ids.h                        |  5 +++++
 xen/include/xen/softirq.h                        | 10 +++++++---
 19 files changed, 113 insertions(+), 13 deletions(-)

-- 
2.25.1

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

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

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@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 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
@@ -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
+
 #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_BOOT_H */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/include/asm/efibind.h b/xen/arch/arm/include/asm/efibind.h
index 09dca7a8c9..92b8bad0bb 100644
--- a/xen/arch/arm/include/asm/efibind.h
+++ b/xen/arch/arm/include/asm/efibind.h
@@ -1,2 +1,7 @@
+#ifndef ARM_EFIBIND_H
+#define ARM_EFIBIND_H
+
 #include <xen/types.h>
 #include <asm/arm64/efibind.h>
+
+#endif /* ARM_EFIBIND_H */
-- 
2.25.1

[PATCH 2/6] xen/x86: add inclusion guards

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

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@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/arch/x86/cpu/cpu.h             | 6 ++++++
 xen/arch/x86/efi/efi-boot.h        | 6 ++++++
 xen/arch/x86/efi/runtime.h         | 5 +++++
 xen/arch/x86/include/asm/compat.h  | 5 +++++
 xen/arch/x86/include/asm/efibind.h | 5 +++++
 xen/arch/x86/x86_64/mmconfig.h     | 5 +++++
 xen/arch/x86/x86_emulate/private.h | 5 +++++
 7 files changed, 37 insertions(+)

diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h
index 8be65e975a..cbb434f3a2 100644
--- a/xen/arch/x86/cpu/cpu.h
+++ b/xen/arch/x86/cpu/cpu.h
@@ -1,4 +1,8 @@
 /* attempt to consolidate cpu attributes */
+
+#ifndef X86_CPU_H
+#define X86_CPU_H
+
 struct cpu_dev {
 	void		(*c_early_init)(struct cpuinfo_x86 *c);
 	void		(*c_init)(struct cpuinfo_x86 * c);
@@ -26,3 +30,5 @@ void amd_init_spectral_chicken(void);
 void detect_zen2_null_seg_behaviour(void);
 
 void intel_unlock_cpuid_leaves(struct cpuinfo_x86 *c);
+
+#endif /* X86_CPU_H */
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 1d8902a9a7..0ecf4ca53f 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/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 X86_EFI_EFI_BOOT_H
+#define X86_EFI_EFI_BOOT_H
+
 #include <xen/vga.h>
 
 #include <asm/boot-helpers.h>
@@ -908,6 +912,8 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle,
     efi_exit_boot(ImageHandle, SystemTable);
 }
 
+#endif /* X86_EFI_EFI_BOOT_H */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/efi/runtime.h b/xen/arch/x86/efi/runtime.h
index 77866c5f21..88ab5651e9 100644
--- a/xen/arch/x86/efi/runtime.h
+++ b/xen/arch/x86/efi/runtime.h
@@ -1,3 +1,6 @@
+#ifndef X86_EFI_RUNTIME_H
+#define X86_EFI_RUNTIME_H
+
 #include <xen/domain_page.h>
 #include <xen/mm.h>
 #include <asm/atomic.h>
@@ -17,3 +20,5 @@ void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e)
     }
 }
 #endif
+
+#endif /* X86_EFI_RUNTIME_H */
diff --git a/xen/arch/x86/include/asm/compat.h b/xen/arch/x86/include/asm/compat.h
index 818cad87db..30ed8f2fd0 100644
--- a/xen/arch/x86/include/asm/compat.h
+++ b/xen/arch/x86/include/asm/compat.h
@@ -2,6 +2,9 @@
  * compat.h
  */
 
+#ifndef X86_COMPAT_H
+#define X86_COMPAT_H
+
 #ifdef CONFIG_COMPAT
 
 #define COMPAT_BITS_PER_LONG 32
@@ -18,3 +21,5 @@ int switch_compat(struct domain *);
 #include <xen/errno.h>
 static inline int switch_compat(struct domain *d) { return -EOPNOTSUPP; }
 #endif
+
+#endif /* X86_COMPAT_H */
diff --git a/xen/arch/x86/include/asm/efibind.h b/xen/arch/x86/include/asm/efibind.h
index bce02f3707..ab46341281 100644
--- a/xen/arch/x86/include/asm/efibind.h
+++ b/xen/arch/x86/include/asm/efibind.h
@@ -1,2 +1,7 @@
+#ifndef X86_EFIBIND_H
+#define X86_EFIBIND_H
+
 #include <xen/types.h>
 #include <asm/x86_64/efibind.h>
+
+#endif /* X86_EFIBIND_H */
diff --git a/xen/arch/x86/x86_64/mmconfig.h b/xen/arch/x86/x86_64/mmconfig.h
index 3da4b21e9b..722bf67975 100644
--- a/xen/arch/x86/x86_64/mmconfig.h
+++ b/xen/arch/x86/x86_64/mmconfig.h
@@ -5,6 +5,9 @@
  * Author: Allen Kay <allen.m.kay@intel.com> - adapted from linux
  */
 
+#ifndef X86_64_MMCONFIG_H
+#define X86_64_MMCONFIG_H
+
 #define PCI_DEVICE_ID_INTEL_E7520_MCH    0x3590
 #define PCI_DEVICE_ID_INTEL_82945G_HB    0x2770
 
@@ -72,3 +75,5 @@ int pci_mmcfg_reserved(uint64_t address, unsigned int segment,
 int pci_mmcfg_arch_init(void);
 int pci_mmcfg_arch_enable(unsigned int idx);
 void pci_mmcfg_arch_disable(unsigned int idx);
+
+#endif /* X86_64_MMCONFIG_H */
diff --git a/xen/arch/x86/x86_emulate/private.h b/xen/arch/x86/x86_emulate/private.h
index 30be595470..467bce3c84 100644
--- a/xen/arch/x86/x86_emulate/private.h
+++ b/xen/arch/x86/x86_emulate/private.h
@@ -6,6 +6,9 @@
  * Copyright (c) 2005-2007 XenSource Inc.
  */
 
+#ifndef X86_EMULATE_PRIVATE_H
+#define X86_EMULATE_PRIVATE_H
+
 #ifdef __XEN__
 
 # include <xen/bug.h>
@@ -843,3 +846,5 @@ static inline int read_ulong(enum x86_segment seg,
     *val = 0;
     return ops->read(seg, offset, val, bytes, ctxt);
 }
+
+#endif /* X86_EMULATE_PRIVATE_H */
-- 
2.25.1

[PATCH 3/6] xen: add inclusion guards

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

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@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/common/decompress.h    | 5 +++++
 xen/common/efi/efi.h       | 5 +++++
 xen/common/event_channel.h | 5 +++++
 xen/include/xen/pci_ids.h  | 5 +++++
 4 files changed, 20 insertions(+)

diff --git a/xen/common/decompress.h b/xen/common/decompress.h
index 4683eb6c7e..034c833665 100644
--- a/xen/common/decompress.h
+++ b/xen/common/decompress.h
@@ -1,3 +1,6 @@
+#ifndef DECOMPRESS_H
+#define DECOMPRESS_H
+
 #ifdef __XEN__
 
 #include <xen/decompress.h>
@@ -22,3 +25,5 @@
 #define large_free free
 
 #endif
+
+#endif /* DECOMPRESS_H */
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index c02fbb7b69..b02aa2775d 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -1,3 +1,6 @@
+#ifndef EFI_EFI_H
+#define EFI_EFI_H
+
 #include <asm/efibind.h>
 #include <efi/efidef.h>
 #include <efi/efierr.h>
@@ -51,3 +54,5 @@ void free_ebmalloc_unused_mem(void);
 
 const void *pe_find_section(const void *image, const UINTN image_size,
                             const CHAR16 *section_name, UINTN *size_out);
+
+#endif /* EFI_EFI_H */
diff --git a/xen/common/event_channel.h b/xen/common/event_channel.h
index a778ae775b..dc94a43cc2 100644
--- a/xen/common/event_channel.h
+++ b/xen/common/event_channel.h
@@ -1,5 +1,8 @@
 /* Event channel handling private header. */
 
+#ifndef EVENT_CHANNEL_H
+#define EVENT_CHANNEL_H
+
 #include <xen/event.h>
 
 static inline unsigned int max_evtchns(const struct domain *d)
@@ -67,6 +70,8 @@ static inline void evtchn_fifo_destroy(struct domain *d)
 }
 #endif /* CONFIG_EVTCHN_FIFO */
 
+#endif /* EVENT_CHANNEL_H */
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/pci_ids.h b/xen/include/xen/pci_ids.h
index e798477a7e..5884a20b8f 100644
--- a/xen/include/xen/pci_ids.h
+++ b/xen/include/xen/pci_ids.h
@@ -1,3 +1,6 @@
+#ifndef XEN_PCI_IDS_H
+#define XEN_PCI_IDS_H
+
 #define PCI_VENDOR_ID_AMD                0x1022
 
 #define PCI_VENDOR_ID_NVIDIA             0x10de
@@ -11,3 +14,5 @@
 #define PCI_VENDOR_ID_BROADCOM           0x14e4
 
 #define PCI_VENDOR_ID_INTEL              0x8086
+
+#endif /* XEN_PCI_IDS_H */
-- 
2.25.1

[PATCH 4/6] xen: refactor include guards

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

Refactor inclusion guards:
1) use a syntax that is more likely to be recognized by static
   analyzers;
2) follow the CODING_STYLE.

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/include/xen/err.h     | 10 +++++++---
 xen/include/xen/softirq.h | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
index cbdd1bf7f8..5bdf8b215c 100644
--- a/xen/include/xen/err.h
+++ b/xen/include/xen/err.h
@@ -1,5 +1,7 @@
-#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__)
-#define __XEN_ERR_H__
+#if !defined(XEN_ERR_H)
+#define XEN_ERR_H
+
+#if !defined(__ASSEMBLY__)
 
 #include <xen/compiler.h>
 #include <xen/errno.h>
@@ -41,4 +43,6 @@ static inline int __must_check PTR_RET(const void *ptr)
 	return IS_ERR(ptr) ? PTR_ERR(ptr) : 0;
 }
 
-#endif /* __XEN_ERR_H__ */
+#endif /* __ASSEMBLY__ */
+
+#endif /* XEN_ERR_H */
diff --git a/xen/include/xen/softirq.h b/xen/include/xen/softirq.h
index 33d6f2ecd2..5593c7b0a9 100644
--- a/xen/include/xen/softirq.h
+++ b/xen/include/xen/softirq.h
@@ -1,5 +1,7 @@
-#if !defined(__XEN_SOFTIRQ_H__) && !defined(__ASSEMBLY__)
-#define __XEN_SOFTIRQ_H__
+#if !defined(XEN_SOFTIRQ_H)
+#define XEN_SOFTIRQ_H
+
+#if !defined(__ASSEMBLY__)
 
 /* Low-latency softirqs come first in the following list. */
 enum {
@@ -40,4 +42,6 @@ void cpu_raise_softirq_batch_finish(void);
  */
 void process_pending_softirqs(void);
 
-#endif /* __XEN_SOFTIRQ_H__ */
+#endif /* __ASSEMBLY__ */
+
+#endif /* XEN_SOFTIRQ_H */
-- 
2.25.1

[PATCH 5/6] x86/asm: refactor inclusion guards

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

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".

Refactor inclusion guards to address a violation of Directive 4.10
and follow CODING_STYLE.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 xen/arch/x86/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index bedb97cbee..ce724a9daa 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -261,17 +261,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 X86_MACROS_H'; \
+    echo '#define X86_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 /* X86_MACROS_H */'
 endef
 
 $(obj)/efi.lds: AFLAGS-y += -DEFI
-- 
2.25.1

[PATCH 6/6] automation/eclair: update configuration of D4.10

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

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".

Update ECLAIR configuration to:
- extend existing deviation to other comments explicitly saying a file
  is intended for multiple inclusion;
- extend existing deviation to other autogenerated files;
- tag the guidelines as clean.

Update deviations.rst accordingly.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 14 +++++++++++---
 automation/eclair_analysis/ECLAIR/tagging.ecl    |  1 +
 docs/misra/deviations.rst                        | 15 +++++++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 9c67358d46..3fb6d9f971 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -72,11 +72,19 @@ they are not instances of commented-out code."
 -config=MC3A2.D4.3,reports+={deliberate, "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
 -doc_end
 
--doc_begin="Files that are intended to be included more than once do not need to
-conform to the directive."
+-doc_begin="Files that are intended to be included more than once (and have
+a comment that says this explicitly) do not need to conform to the directive."
 -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file is intended to be included multiple times\\. \\*/$, begin-4))"}
+-config=MC3A2.D4.10,reports+={safe, "first_area(text(^.*Explicitly intended for multiple inclusion.*$, begin-3))"}
+-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-2))"}
 -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-3))"}
--config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
+-doc_end
+
+-doc_begin="Autogenerated files that do not need to conform to the directive."
+-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
+-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/xen/compile\\.h$)))"}
+-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/compat/xlat\\.h$)))"}
+-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/arch/(arm||x86)/include/generated/asm/.*$)))"}
 -doc_end
 
 -doc_begin="Including multiple times a .c file is safe because every function or data item
diff --git a/automation/eclair_analysis/ECLAIR/tagging.ecl b/automation/eclair_analysis/ECLAIR/tagging.ecl
index 5bc35db1fd..7e3095423b 100644
--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -23,6 +23,7 @@
 "MC3A2.D1.1||
 MC3A2.D2.1||
 MC3A2.D4.1||
+MC3A2.D4.10||
 MC3A2.D4.11||
 MC3A2.D4.14||
 MC3A2.R1.1||
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index fe0b1e10a2..87ed81c918 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -30,6 +30,21 @@ Deviations related to MISRA C:2012 Directives:
        not to add an additional encapsulation layer.
      - Tagged as `deliberate` for ECLAIR.
 
+   * - D4.10
+     - Files that are intended to be included more than once (and have
+       a comment that says this explicitly) do not need to conform to the
+       directive.
+     - Tagged as `safe` for ECLAIR.
+
+   * - D4.10
+     - There are autogenerated files that do not need to comply to the
+       directive.
+     - Tagged as `safe` for ECLAIR. Such files are:
+        - xen/include/generated/autoconf.h
+        - xen/include/compat/xlat.h
+        - xen/include/xen/compile.h
+        - xen/arch/{arm,x86}/include/generated/asm/\*
+
    * - D4.10
      - Including multiple times a .c file is safe because every function or data item
        it defines would in (the common case) be already defined.
-- 
2.25.1

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

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
>
> 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@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 2/6] xen/x86: add inclusion guards

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
>
> 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@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 3/6] xen: add inclusion guards

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
>
> 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@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> diff --git a/xen/common/decompress.h b/xen/common/decompress.h
> index 4683eb6c7e..034c833665 100644
> --- a/xen/common/decompress.h
> +++ b/xen/common/decompress.h
> @@ -1,3 +1,6 @@
> +#ifndef DECOMPRESS_H
> +#define DECOMPRESS_H
> +
>  #ifdef __XEN__
>  
>  #include <xen/decompress.h>
> @@ -22,3 +25,5 @@
>  #define large_free free
>  
>  #endif
> +
> +#endif /* DECOMPRESS_H */

This is borderline too generic, but a COMMON_ prefix doesn't make it any
better.  Oh well...

~Andrew

P.S. the decompressor infrastructure is several stacked disasters and
needs redoing from scratch.

Re: [PATCH 4/6] xen: refactor include guards

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
> index cbdd1bf7f8..5bdf8b215c 100644
> --- a/xen/include/xen/err.h
> +++ b/xen/include/xen/err.h
> @@ -1,5 +1,7 @@
> -#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__)
> -#define __XEN_ERR_H__
> +#if !defined(XEN_ERR_H)
> +#define XEN_ERR_H

I know this is just rearranging the existing like, but both the
defined()'s should turn into the more normal #ifndef's now they're split.

Same for softirq.h

> +
> +#if !defined(__ASSEMBLY__)
>  
>  #include <xen/compiler.h>
>  #include <xen/errno.h>
> @@ -41,4 +43,6 @@ static inline int __must_check PTR_RET(const void *ptr)
>  	return IS_ERR(ptr) ? PTR_ERR(ptr) : 0;
>  }
>  
> -#endif /* __XEN_ERR_H__ */
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* XEN_ERR_H */

I realise this is personal preference, but for the end of a header like
this where each is annotated properly, I don't see much value having the
extra blank line.

~Andrew

Re: [PATCH 6/6] automation/eclair: update configuration of D4.10

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index 9c67358d46..3fb6d9f971 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -72,11 +72,19 @@ they are not instances of commented-out code."
>  -config=MC3A2.D4.3,reports+={deliberate, "any_area(any_loc(file(arm64_bitops))&&context(name(int_clear_mask16)))"}
>  -doc_end
>  
> --doc_begin="Files that are intended to be included more than once do not need to
> -conform to the directive."
> +-doc_begin="Files that are intended to be included more than once (and have
> +a comment that says this explicitly) do not need to conform to the directive."
>  -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* This file is intended to be included multiple times\\. \\*/$, begin-4))"}
> +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^.*Explicitly intended for multiple inclusion.*$, begin-3))"}

xen.git/xen$ git grep "Explicitly intended for multiple"
arch/x86/include/asm/cpufeatures.h:2: * Explicitly intended for multiple
inclusion.

I'd suggest altering that one file, rather than adding an special
exclusion pattern.

> +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-2))"}
>  -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-3))"}

These seem to only differ by the begin-$N.  Why doesn't the regex work
in both cases?

> --config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf.h$)))"}
> +-doc_end
> +
> +-doc_begin="Autogenerated files that do not need to conform to the directive."
> +-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/generated/autoconf\\.h$)))"}
> +-config=MC3A2.D4.10,reports+={safe, "all_area(all_loc(file(^xen/include/xen/compile\\.h$)))"}

I see your exception, and raise you some sed.

diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 3151d1e7d1bf..9206341ba692 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -1,3 +1,6 @@
+#ifndef XEN_COMPILE_H
+#define XEN_COMPILE_H
+
 #define XEN_COMPILE_DATE       "@@date@@"
 #define XEN_COMPILE_TIME       "@@time@@"
 #define XEN_COMPILE_BY         "@@whoami@@"
diff --git a/xen/tools/process-banner.sed b/xen/tools/process-banner.sed
index 56c76558bcd9..4cf3f9a1163a 100755
--- a/xen/tools/process-banner.sed
+++ b/xen/tools/process-banner.sed
@@ -12,3 +12,8 @@ s_(.*)_"\1\\n"_
 
 # Trailing \ on all but the final line.
 $!s_$_ \\_
+
+# Append closing header guard
+$a\
+\
+#endif /* XEN_COMPILE_H */

and now compile.h looks like a normal header.

~Andrew

[PATCH v2 4/6] xen: refactor include guards

[Stefano Stabellini]

From: Federico Serafini <federico.serafini@bugseng.com>

Refactor inclusion guards:
1) use a syntax that is more likely to be recognized by static
   analyzers;
2) follow the CODING_STYLE.

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
Changes in v2:
- use normal #ifndef
---
 xen/include/xen/err.h     | 10 +++++++---
 xen/include/xen/softirq.h | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
index cbdd1bf7f8..a5971e290c 100644
--- a/xen/include/xen/err.h
+++ b/xen/include/xen/err.h
@@ -1,5 +1,7 @@
-#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__)
-#define __XEN_ERR_H__
+#ifndef XEN_ERR_H
+#define XEN_ERR_H
+
+#ifndef __ASSEMBLY__
 
 #include <xen/compiler.h>
 #include <xen/errno.h>
@@ -41,4 +43,6 @@ static inline int __must_check PTR_RET(const void *ptr)
 	return IS_ERR(ptr) ? PTR_ERR(ptr) : 0;
 }
 
-#endif /* __XEN_ERR_H__ */
+#endif /* __ASSEMBLY__ */
+
+#endif /* XEN_ERR_H */
diff --git a/xen/include/xen/softirq.h b/xen/include/xen/softirq.h
index 33d6f2ecd2..e9f79ec0ce 100644
--- a/xen/include/xen/softirq.h
+++ b/xen/include/xen/softirq.h
@@ -1,5 +1,7 @@
-#if !defined(__XEN_SOFTIRQ_H__) && !defined(__ASSEMBLY__)
-#define __XEN_SOFTIRQ_H__
+#ifndef XEN_SOFTIRQ_H
+#define XEN_SOFTIRQ_H
+
+#ifndef __ASSEMBLY__
 
 /* Low-latency softirqs come first in the following list. */
 enum {
@@ -40,4 +42,6 @@ void cpu_raise_softirq_batch_finish(void);
  */
 void process_pending_softirqs(void);
 
-#endif /* __XEN_SOFTIRQ_H__ */
+#endif /* __ASSEMBLY__ */
+
+#endif /* XEN_SOFTIRQ_H */
-- 
2.25.1

Re: [PATCH 5/6] x86/asm: refactor inclusion guards

[Andrew Cooper]

On 17/05/2025 12:21 am, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
>
> 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".
>
> Refactor inclusion guards to address a violation of Directive 4.10
> and follow CODING_STYLE.
>
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH v2 4/6] xen: refactor include guards

[Andrew Cooper]

On 17/05/2025 1:10 am, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
>
> Refactor inclusion guards:
> 1) use a syntax that is more likely to be recognized by static
>    analyzers;
> 2) follow the CODING_STYLE.
>
> No functional change.
>
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 6/6] automation/eclair: update configuration of D4.10

[Federico Serafini]

Hi,

On 17/05/25 01:57, Andrew Cooper wrote:
> 
>> +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-2))"}
>>   -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated file, do not edit! \\*/$, begin-3))"}
> 
> These seem to only differ by the begin-$N.  Why doesn't the regex work
> in both cases?

"begin-N" expresses the position of a single line, not a range.
For example, begin-2 means "two lines before the first reported area"
and deviates:

https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/hardware/xen/ECLAIR_normal/staging/X86_64/10063944407/PROJECT.ecd;/sources/xen/include/xen/hypercall-defs.h.html#R174_1{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":false,"selector":{"enabled":true,"negated":false,"kind":2,"children":[]}}}

If you prefer, I think we can use ranges and merge the two
configurations.

-- 
Federico Serafini, MSc
Software Engineer, BUGSENG (https://bugseng.com)
LinkedIn: https://linkedin.com/in/federico-serafini

Re: [PATCH 6/6] automation/eclair: update configuration of D4.10

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Mon, 19 May 2025, Federico Serafini wrote:
> Hi,
> 
> On 17/05/25 01:57, Andrew Cooper wrote:
> > 
> > > +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated
> > > file, do not edit! \\*/$, begin-2))"}
> > >   -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated
> > > file, do not edit! \\*/$, begin-3))"}
> > 
> > These seem to only differ by the begin-$N.  Why doesn't the regex work
> > in both cases?
> 
> "begin-N" expresses the position of a single line, not a range.
> For example, begin-2 means "two lines before the first reported area"
> and deviates:
> 
> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/hardware/xen/ECLAIR_normal/staging/X86_64/10063944407/PROJECT.ecd;/sources/xen/include/xen/hypercall-defs.h.html#R174_1{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":false,"selector":{"enabled":true,"negated":false,"kind":2,"children":[]}}}
> 
> If you prefer, I think we can use ranges and merge the two
> configurations.

I think that would be better

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

[Julien Grall]

Hi Stefano,

On 17/05/2025 00:21, Stefano Stabellini wrote:
> From: Federico Serafini <federico.serafini@bugseng.com>
> 
> 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@bugseng.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

With one remark below:

Acked-by: Julien Grall <jgrall@amazon.com>

> ---
>   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

Re: [PATCH 6/6] automation/eclair: update configuration of D4.10

[Federico Serafini]

On 19/05/25 23:36, Stefano Stabellini wrote:
> On Mon, 19 May 2025, Federico Serafini wrote:
>> Hi,
>>
>> On 17/05/25 01:57, Andrew Cooper wrote:
>>>
>>>> +-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated
>>>> file, do not edit! \\*/$, begin-2))"}
>>>>    -config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated
>>>> file, do not edit! \\*/$, begin-3))"}
>>>
>>> These seem to only differ by the begin-$N.  Why doesn't the regex work
>>> in both cases?
>>
>> "begin-N" expresses the position of a single line, not a range.
>> For example, begin-2 means "two lines before the first reported area"
>> and deviates:
>>
>> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/hardware/xen/ECLAIR_normal/staging/X86_64/10063944407/PROJECT.ecd;/sources/xen/include/xen/hypercall-defs.h.html#R174_1{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":false,"selector":{"enabled":true,"negated":false,"kind":2,"children":[]}}}
>>
>> If you prefer, I think we can use ranges and merge the two
>> configurations.
> 
> I think that would be better


The configurations can be merged into a single one:

-config=MC3A2.D4.10,reports+={safe, "first_area(text(^/\\* Generated 
file, do not edit! \\*/$, begin-3...begin-2))"}

-- 
Federico Serafini, MSc
Software Engineer, BUGSENG (https://bugseng.com)
LinkedIn: https://linkedin.com/in/federico-serafini