From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3D1E1C27C4F for ; Wed, 26 Jun 2024 10:25:18 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.748712.1156516 (Exim 4.92) (envelope-from ) id 1sMPpv-0007hx-Cd; Wed, 26 Jun 2024 10:25:11 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 748712.1156516; Wed, 26 Jun 2024 10:25:11 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sMPpv-0007hq-9q; Wed, 26 Jun 2024 10:25:11 +0000 Received: by outflank-mailman (input) for mailman id 748712; Wed, 26 Jun 2024 10:25:10 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1sMPpu-0007gX-Fv for xen-devel@lists.xenproject.org; Wed, 26 Jun 2024 10:25:10 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 5d286a5b-33a6-11ef-90a3-e314d9c70b13; Wed, 26 Jun 2024 12:25:09 +0200 (CEST) Received: from support.bugseng.com (support.bugseng.com [162.55.131.47]) by support.bugseng.com (Postfix) with ESMTPA id D65374EE0738; Wed, 26 Jun 2024 12:25:08 +0200 (CEST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 5d286a5b-33a6-11ef-90a3-e314d9c70b13 MIME-Version: 1.0 Date: Wed, 26 Jun 2024 12:25:08 +0200 From: Nicola Vetrini To: Jan Beulich Cc: Simone Ballarin , consulting@bugseng.com, sstabellini@kernel.org, Andrew Cooper , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Wei Liu , xen-devel@lists.xenproject.org Subject: Re: [XEN PATCH v3 05/16] xen/x86: address violations of MISRA C:2012 Directive 4.10 In-Reply-To: References: <797b00049612507d273facc581b2c2c5@bugseng.com> Message-ID: X-Sender: nicola.vetrini@bugseng.com Organization: BUGSENG s.r.l. Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit 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)