[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN PATCH v3 05/16] xen/x86: address violations of MISRA C:2012 Directive 4.10
On 2024-06-26 11:26, Jan Beulich wrote:
On 26.06.2024 11:20, Nicola Vetrini wrote:
On 2024-06-26 11:06, Jan Beulich wrote:
On 25.06.2024 21:31, Nicola Vetrini wrote:
On 2024-03-12 09:16, Jan Beulich wrote:
On 11.03.2024 09:59, Simone Ballarin wrote:
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -258,18 +258,20 @@ $(obj)/asm-macros.i: CFLAGS-y += -P
$(objtree)/arch/x86/include/asm/asm-macros.h: $(obj)/asm-macros.i
$(src)/Makefile
$(call filechk,asm-macros.h)
+ARCHDIR = $(shell echo $(SRCARCH) | tr a-z A-Z)
This wants to use :=, I think - there's no reason to invoke the
shell
...
I agree on this
define filechk_asm-macros.h
+ echo '#ifndef ASM_$(ARCHDIR)_ASM_MACROS_H'; \
+ echo '#define ASM_$(ARCHDIR)_ASM_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 /* ASM_$(ARCHDIR)_ASM_MACROS_H */'
endef
... three times while expanding this macro. Alternatively (to avoid
an unnecessary shell invocation when this macro is never expanded
at
all) a shell variable inside the "define" above would want
introducing.
Whether this 2nd approach is better depends on whether we
anticipate
further uses of ARCHDIR.
However here I'm not entirely sure about the meaning of this latter
proposal.
My proposal is the following:
ARCHDIR := $(shell echo $(SRCARCH) | tr a-z A-Z)
in a suitably generic place (such as Kbuild.include or maybe
xen/Makefile) as you suggested in subsequent patches that reused
this
pattern.
If $(ARCHDIR) is going to be used elsewhere, then what you suggest is
fine.
My "whether" in the earlier reply specifically left open for
clarification
what the intentions with the variable are. The alternative I had
described
makes sense only when $(ARCHDIR) would only ever be used inside the
filechk_asm-macros.h macro.
Yes, the intention is to reuse $(ARCHDIR) in the formation of other
places, as you can tell from the fact that subsequent patches
replicate
the same pattern. This is going to save some duplication.
The only matter left then is whether xen/Makefile (around line 250,
just
after setting SRCARCH) would be better, or Kbuild.include. To me the
former place seems more natural, but I'm not totally sure.
Depends on where all the intended uses are. If they're all in
xen/Makefile,
then having the macro just there is of course sufficient. Whereas when
it's
needed elsewhere, instead of exporting putting it in Kbuild.include
would
seem more natural / desirable to me.
The places where this would be used are these:
file: target (or define)
xen/build.mk: arch/$(SRCARCH)/include/asm/asm-offsets.h: asm-offsets.s
xen/include/Makefile: define cmd_xlat_h
xen/arch/x86/Makefile: define filechk_asm-macros.h
The only issue that comes to my mind (it may not be one at all) is that
SRCARCH is defined and exported in xen/Makefile after including
Kbuild.include, so it would need to be defined after SRCARCH is
assigned:
include scripts/Kbuild.include
# Don't break if the build process wasn't called from the top level
# we need XEN_TARGET_ARCH to generate the proper config
include $(XEN_ROOT)/Config.mk
# Set ARCH/SRCARCH appropriately.
ARCH := $(XEN_TARGET_ARCH)
SRCARCH := $(shell echo $(ARCH) | \
sed -e 's/x86.*/x86/' -e 's/arm\(32\|64\)/arm/g' \
-e 's/riscv.*/riscv/g' -e 's/ppc.*/ppc/g')
export ARCH SRCARCH
Am I missing something?
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
|
