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 8DEAEC4332F for ; Tue, 17 May 2022 18:42:47 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.331426.554937 (Exim 4.92) (envelope-from ) id 1nr29B-0002fw-2s; Tue, 17 May 2022 18:42:17 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 331426.554937; Tue, 17 May 2022 18:42:17 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1nr29A-0002fp-Vv; Tue, 17 May 2022 18:42:16 +0000 Received: by outflank-mailman (input) for mailman id 331426; Tue, 17 May 2022 18:42:15 +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 1nr299-0002fU-Pc for xen-devel@lists.xenproject.org; Tue, 17 May 2022 18:42:15 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id 115fda80-d611-11ec-bd2c-47488cf2e6aa; Tue, 17 May 2022 20:42:13 +0200 (CEST) Received: from [192.168.1.39] (unknown [81.93.39.129]) by support.bugseng.com (Postfix) with ESMTPSA id D41284EE077A; Tue, 17 May 2022 20:42:12 +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: 115fda80-d611-11ec-bd2c-47488cf2e6aa Message-ID: Date: Tue, 17 May 2022 20:42:12 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20050929 Thunderbird/1.0.7 Fedora/1.0.7-1.1.fc4 Mnenhy/0.7.3.0 Subject: Re: [PATCH 1/3] x86/p2m.h: Add include guards Content-Language: en-US To: Jan Beulich , Andrew Cooper Cc: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , Wei Liu , Stefano Stabellini , Julien Grall , Volodymyr Babchuk , Bertrand Marquis , Xen-devel References: <20220509122409.14184-1-andrew.cooper3@citrix.com> <20220509122409.14184-2-andrew.cooper3@citrix.com> <000745e7-0b0e-386e-861e-901319defde3@suse.com> From: Roberto Bagnara In-Reply-To: <000745e7-0b0e-386e-861e-901319defde3@suse.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 17/05/22 17:38, Jan Beulich wrote: > On 09.05.2022 14:24, Andrew Cooper wrote: >> Spotted by Eclair MISRA scanner. > > I'm sorry, but what exactly was it that the scanner spotted? It was > actually deliberate to introduce this file without guards. I'm of > the general opinion that (private) headers not to be included by > other headers (but only by .c files) are not in need of guards. If > it is project-wide consensus that _all_ header files should have > guards, then I'll try to keep this in mind (in "x86emul: a few > small steps towards disintegration" for example I introduce > another such instance), but then it should also be put down in > ./CODING_STYLE. The rationale of this rule is as follows: - With a complex hierarchy of nested header files, it is possible for a header file to be included more than once. - This can bring to circular references of header files, which can result in undefined behavior and/or be difficult to debug. - If multiple inclusion leads to multiple or conflicting definitions, then this can result in undefined or erroneous behavior. - Compilation and analysis time is needlessly increased. There has been a period (which lasted until the end of the '70s or the beginning of the '80s, I would have to dig up to be more precise) when the solution was thought to be "headers shall not to be included by other headers but only by .c files." Experience then showed that, in medium to large projects, each .c file had to begin with a long list of #include directives; such lists needed to be ordered to accommodate the dependencies between header files; in some cases the lists were so long that: a) it was a kind of black magic to find out the right inclusion order, one that would work in any of possibly many project configurations; b) the lists of #include directives often contained duplicates, possibly because the desperate programmers where trying to find the right order. In the end, the software engineering community converged on the idea that guards against multiple inclusion are a much better alternative. Of course there are valid reasons to deviate the rule: some header files might be conceived to be included multiple times. A one-line configuration for ECLAIR will do the trick to make sure such header files are not reported. Kind regards, Roberto >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Roger Pau Monné >> CC: Wei Liu >> CC: Stefano Stabellini >> CC: Julien Grall >> CC: Volodymyr Babchuk >> CC: Bertrand Marquis >> --- >> xen/arch/x86/mm/p2m.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/xen/arch/x86/mm/p2m.h b/xen/arch/x86/mm/p2m.h >> index cc0f6766e4df..dc706b8e4799 100644 >> --- a/xen/arch/x86/mm/p2m.h >> +++ b/xen/arch/x86/mm/p2m.h >> @@ -15,6 +15,9 @@ >> * along with this program; If not, see . >> */ >> >> +#ifndef __ARCH_MM_P2M_H__ >> +#define __ARCH_MM_P2M_H__ >> + >> struct p2m_domain *p2m_init_one(struct domain *d); >> void p2m_free_one(struct p2m_domain *p2m); >> >> @@ -39,6 +42,8 @@ int ept_p2m_init(struct p2m_domain *p2m); >> void ept_p2m_uninit(struct p2m_domain *p2m); >> void p2m_init_altp2m_ept(struct domain *d, unsigned int i); >> >> +#endif /* __ARCH_MM_P2M_H__ */ >> + >> /* >> * Local variables: >> * mode: C > >