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