[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/3] x86/p2m.h: Add include guards


  • To: Roberto Bagnara <roberto.bagnara@xxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 18 May 2022 08:26:04 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=z6DU8D9HB3drtRuuSaHOegBjamoMEXyE7bavKOxLjNM=; b=ZhjvuW1p76mcCJkvhlV8fVoLrJutgvfszFihGWHNNdPGiDzmWoMbL8k9C3WyXdASjP9ZXWI76e9VZaRdSsNQx32Z2CuDBp5b6aoOo1IkIfrMHZb3dIqfISaW+Wo4lYJSp5QVlYRMfz9HPhgPM3D6pxBsABlwB2trtfBSIdREdysd/IWdR3fJc8Rv2/x7YfFC+lHzV6EXYxXZO7IwAhhY0L2nTpu6dQvKCAdIQQ6+2VWGYeNDWQNrUYqgtK2mLPVtlkFmacog+EvFLApyF7JIPTn1EDBaWG8NlYOY1S6dsc1rhfuaU1V0W7qab3lmwMIc8FkBg6oVmguxgMDEZhFjEw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PzZnXAfwYteEGdGsk+MHOv/Lh5RBtzxYn66BWSi2sW20ID7OMmzGMH18PaauAuy+6g8r5vY9jHSig7uVmmCxKBjfqhpzm4bnX4n0hOC69sZ6GeeBIAqaATizHJk+48aWA1wtIiMQ2kYrQxEN7oY+oCdiHFNrKhhMUObUy2svakNuGu1F3XFxFe3Yf4azPfgsOUIljL7kPZpsKKPXV1LK5aMfpaTEfKY8By+cwIR6ujyfUTBaiOQmfRdArOv4g9kcGCH0FH283QP2H6ndxovDtnwVp5bCZFdjvcwrNpe702617zzGxw6jxHA81hYqfNlW4/CG4/QzxDr6baxLuQIM2w==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Wed, 18 May 2022 06:26:29 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 17.05.2022 20:42, Roberto Bagnara wrote:
> 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.

This, in particular, is known to happen in our and other source
bases despite the use of guards, hence I view this point as at
best partly related. Nevertheless I agree with and understand the
reasons for using guards _where they are needed_. I do not agree
that guards need to be there for no specific reason.

Jan

> - 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 <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Wei Liu <wl@xxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: Julien Grall <julien@xxxxxxx>
>>> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
>>> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>>> ---
>>>   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 <http://www.gnu.org/licenses/>.
>>>    */
>>>   
>>> +#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
>>
>>
> 




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.