Re: [XEN PATCH v5 12/17] xen: address violations of MISRA C:2012 Directive 4.10

[Alessandro Zucchelli <alessandro.zucchelli@xxxxxxxxxxx>]

On 2024-07-24 00:29, Stefano Stabellini wrote:
> On Tue, 23 Jul 2024, Alessandro Zucchelli wrote:
> > From: Maria Celeste Cesario <maria.celeste.cesario@xxxxxxxxxxx>
> >
> > Modify creation rule for asm-offsets.h to conform to
> > the new standard and to not generate conflicting guards
> > between architectures (which is a violation of the directive).
> > Modify generic-y creation rule to generate code without violations
> > and to conform to the new standard.
> >
> > Mechanical change.
> >
> > Signed-off-by: Maria Celeste Cesario  
> > <maria.celeste.cesario@xxxxxxxxxxx>
> > Signed-off-by: Simone Ballarin  <simone.ballarin@xxxxxxxxxxx>
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@xxxxxxxxxxx>
> I know I gave my Reviewed-by already but I have a question below...
>
>
> > ---
> > Changes in v5:
> > - edit inclusion guards of autogenerated files
> > Commit introduced in v3
> > ---
> >  xen/build.mk                     | 7 ++++---
> >  xen/scripts/Makefile.asm-generic | 8 +++++++-
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/build.mk b/xen/build.mk
> > index 0f490ca71b..32624d3097 100644
> > --- a/xen/build.mk
> > +++ b/xen/build.mk
> > @@ -47,6 +47,7 @@ asm-offsets.s: arch/$(SRCARCH)/$(ARCH)/asm-offsets.c
> >
> >  arch/$(SRCARCH)/include/asm/asm-offsets.h: asm-offsets.s
> >         @(set -e; \
> > +         guard=$$(echo ASM__${SRCARCH}__ASM_OFFSETS_H | tr a-z A-Z); \
> >           echo "/*"; \
> >           echo " * DO NOT MODIFY."; \
> >           echo " *"; \
> > @@ -54,12 +55,12 @@ arch/$(SRCARCH)/include/asm/asm-offsets.h: 
> > asm-offsets.s
> >           echo " *"; \
> >           echo " */"; \
> >           echo ""; \
> > -         echo "#ifndef __ASM_OFFSETS_H__"; \
> > -         echo "#define __ASM_OFFSETS_H__"; \
> > +         echo "#ifndef $$guard"; \
> > +         echo "#define $$guard"; \
> >           echo ""; \
> >           sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \
> >           echo ""; \
> > -         echo "#endif") <$< >$@
> > +         echo "#endif /* $$guard */") <$< >$@
> >
> >  build-dirs := $(patsubst %/built_in.o,%,$(filter 
> > %/built_in.o,$(ALL_OBJS) $(ALL_LIBS)))
> > diff --git a/xen/scripts/Makefile.asm-generic 
> > b/xen/scripts/Makefile.asm-generic
> > index b0d356bfa3..2d2fd9f76e 100644
> > --- a/xen/scripts/Makefile.asm-generic
> > +++ b/xen/scripts/Makefile.asm-generic
> > @@ -32,7 +32,13 @@ old-headers := $(wildcard $(obj)/*.h)
> >  unwanted    := $(filter-out $(generic-y) 
> > $(generated-y),$(old-headers))
> >  quiet_cmd_wrap = WRAP    $@
> > -      cmd_wrap = echo "\#include <asm-generic/$*.h>" > $@
> > +cmd_wrap = \
> > +    guard=$$(echo ASM_GENERIC__${SRCARCH}__$*_H | tr a-z A-Z); \
> Shouldn't this be: ASM_GENERIC__$*_H  according to the coding style?
Yes, you are right, this slipped through.

> > +    echo "\#ifndef $$guard" >$@.new; \
> > +    echo "\#define $$guard" >>$@.new; \
> > +    echo "\#include <asm-generic/$*.h>" >>$@.new; \
> > +    echo "\#endif /* $$guard */" >>$@.new; \
> > +    mv -f $@.new $@
> >
> >  quiet_cmd_remove = REMOVE  $(unwanted)
> >        cmd_remove = rm -f $(unwanted)
> > --
> > 2.34.1
--
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)