Re: [XEN PATCH v3 05/16] xen/x86: address violations of MISRA C:2012 Directive 4.10

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan