diff mbox series

[10/17] x86/asm: address violations of MISRA C:2012 Directive 4.10

Message ID efb272c9eaad24875f34b396a75fed9577dedbda.1719829101.git.alessandro.zucchelli@bugseng.com (mailing list archive)
State Superseded
Headers show
Series xen: address violation of MISRA C:2012 Directive 4.10 | expand

Commit Message

Alessandro Zucchelli July 1, 2024, 1:36 p.m. UTC 
From: Simone Ballarin <simone.ballarin@bugseng.com>

Amend generation script, add 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").

This patch amends the Makefile adding the required inclusion guards
for xlat.h.

Add deviation comment for files intended for multiple inclusion.

Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>

---
Changes in v4:
- changed guard creation for autogenerated headers in Makefile
Changes in v3:
- fix inconsistent deviation ID
- remove trailing underscores
Changes in v2:
- merge patches 7/13 and 13/13 of v1 as they had the same
  commit message
- amend the Makefile to produce the required inclusion guard
- use the format introduced with doc/misra/safe.json instead of
  a generic text-based deviation
---
 docs/misra/safe.json                   |  8 ++++++++
 xen/arch/x86/include/asm/compat.h      |  5 +++++
 xen/arch/x86/include/asm/cpufeatures.h |  5 +----
 xen/arch/x86/include/asm/efibind.h     |  5 +++++
 xen/include/Makefile                   | 11 +++++++++--
 5 files changed, 28 insertions(+), 6 deletions(-)

Comments

Jan Beulich July 3, 2024, 12:49 p.m. UTC | #1 
On 01.07.2024 15:36, Alessandro Zucchelli wrote:
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -90,6 +90,14 @@
>              "name": "Dir 4.10: direct inclusion guard before",
>              "text": "Headers with just the direct inclusion guard before the inclusion guard are safe."
>          },
> +        {
> +            "id": "SAF-11-safe",
> +            "analyser": {
> +                "eclair": "MC3R1.D4.10"
> +            },
> +            "name": "Dir 4.10: file intended for multiple inclusion",
> +            "text": "Files intended for multiple inclusion are not supposed to comply with D4.10."
> +        },
>          {
>              "id": "SAF-11-safe",

This can't be right; the sentinel must have its number changed.

> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -104,10 +104,17 @@ $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
>  xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
>  xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
>  
> +ARCHDIR = $(shell echo $(SRCARCH) | tr a-z A-Z)

Why is this being added here? It's not used ...

>  quiet_cmd_xlat_h = GEN     $@
> -cmd_xlat_h = \
> -	cat $(filter %.h,$^) >$@.new; \
> +define cmd_xlat_h
> +	guard=$$(echo ASM_${SRCARCH}_COMPAT_XLAT_H | tr a-z A-Z); \
> +	echo "#ifndef $$guard" > $@.new; \
> +	echo "#define $$guard" >> $@.new; \
> +	cat $(filter %.h,$^) >> $@.new; \
> +	echo "#endif /* $$guard */" >> $@.new; \
>  	mv -f $@.new $@
> +endef
>  
>  $(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) FORCE
>  	$(call if_changed,xlat_h)

... anywhere. Did you mean to use it in place of ${SRCARCH}?

Jan
Alessandro Zucchelli July 4, 2024, 7:50 a.m. UTC | #2 
On 2024-07-03 14:49, Jan Beulich wrote:
> On 01.07.2024 15:36, Alessandro Zucchelli wrote:
>> --- a/docs/misra/safe.json
>> +++ b/docs/misra/safe.json
>> @@ -90,6 +90,14 @@
>>              "name": "Dir 4.10: direct inclusion guard before",
>>              "text": "Headers with just the direct inclusion guard 
>> before the inclusion guard are safe."
>>          },
>> +        {
>> +            "id": "SAF-11-safe",
>> +            "analyser": {
>> +                "eclair": "MC3R1.D4.10"
>> +            },
>> +            "name": "Dir 4.10: file intended for multiple inclusion",
>> +            "text": "Files intended for multiple inclusion are not 
>> supposed to comply with D4.10."
>> +        },
>>          {
>>              "id": "SAF-11-safe",
> 
> This can't be right; the sentinel must have its number changed.

Yes, this must have been overlooked during the rebasing of the previous 
patch version's commits.

> 
>> --- a/xen/include/Makefile
>> +++ b/xen/include/Makefile
>> @@ -104,10 +104,17 @@ $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst 
>> FORCE
>>  xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 
>> 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | 
>> uniq)
>>  xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
>> 
>> +ARCHDIR = $(shell echo $(SRCARCH) | tr a-z A-Z)
> 
> Why is this being added here? It's not used ...

It was mistakenly left here from the previous version of the patch 
series.

> 
>>  quiet_cmd_xlat_h = GEN     $@
>> -cmd_xlat_h = \
>> -	cat $(filter %.h,$^) >$@.new; \
>> +define cmd_xlat_h
>> +	guard=$$(echo ASM_${SRCARCH}_COMPAT_XLAT_H | tr a-z A-Z); \
>> +	echo "#ifndef $$guard" > $@.new; \
>> +	echo "#define $$guard" >> $@.new; \
>> +	cat $(filter %.h,$^) >> $@.new; \
>> +	echo "#endif /* $$guard */" >> $@.new; \
>>  	mv -f $@.new $@
>> +endef
>> 
>>  $(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) 
>> FORCE
>>  	$(call if_changed,xlat_h)
> 
> ... anywhere. Did you mean to use it in place of ${SRCARCH}?

No, SRCARCH is correct, as ARCHDIR was supposed to be removed.
diff mbox series

Patch

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 3e9ebff8fe..0739eac806 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -90,6 +90,14 @@ 
             "name": "Dir 4.10: direct inclusion guard before",
             "text": "Headers with just the direct inclusion guard before the inclusion guard are safe."
         },
+        {
+            "id": "SAF-11-safe",
+            "analyser": {
+                "eclair": "MC3R1.D4.10"
+            },
+            "name": "Dir 4.10: file intended for multiple inclusion",
+            "text": "Files intended for multiple inclusion are not supposed to comply with D4.10."
+        },
         {
             "id": "SAF-11-safe",
             "analyser": {},
diff --git a/xen/arch/x86/include/asm/compat.h b/xen/arch/x86/include/asm/compat.h
index 818cad87db..36e7750843 100644
--- a/xen/arch/x86/include/asm/compat.h
+++ b/xen/arch/x86/include/asm/compat.h
@@ -2,6 +2,9 @@ 
  * compat.h
  */
 
+#ifndef ASM_X86_COMPAT_H
+#define ASM_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 /* ASM_X86_COMPAT_H */
diff --git a/xen/arch/x86/include/asm/cpufeatures.h b/xen/arch/x86/include/asm/cpufeatures.h
index ba3df174b7..d590fe31ea 100644
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -1,7 +1,4 @@ 
-/*
- * Explicitly intended for multiple inclusion.
- */
-
+/* SAF-11-safe file intended for multiple inclusion */
 #include <xen/lib/x86/cpuid-autogen.h>
 
 /* Number of capability words covered by the featureset words. */
diff --git a/xen/arch/x86/include/asm/efibind.h b/xen/arch/x86/include/asm/efibind.h
index bce02f3707..75684742be 100644
--- a/xen/arch/x86/include/asm/efibind.h
+++ b/xen/arch/x86/include/asm/efibind.h
@@ -1,2 +1,7 @@ 
+#ifndef ASM_X86_EFIBIND_H
+#define ASM_X86_EFIBIND_H
+
 #include <xen/types.h>
 #include <asm/x86_64/efibind.h>
+
+#endif /* ASM_X86_EFIBIND_H */
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 058b0a566b..eccc2e017a 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -104,10 +104,17 @@  $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
 xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
 xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
 
+ARCHDIR = $(shell echo $(SRCARCH) | tr a-z A-Z)
+
 quiet_cmd_xlat_h = GEN     $@
-cmd_xlat_h = \
-	cat $(filter %.h,$^) >$@.new; \
+define cmd_xlat_h
+	guard=$$(echo ASM_${SRCARCH}_COMPAT_XLAT_H | tr a-z A-Z); \
+	echo "#ifndef $$guard" > $@.new; \
+	echo "#define $$guard" >> $@.new; \
+	cat $(filter %.h,$^) >> $@.new; \
+	echo "#endif /* $$guard */" >> $@.new; \
 	mv -f $@.new $@
+endef
 
 $(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) FORCE
 	$(call if_changed,xlat_h)