Re: [PATCH] xen/arm64: sysreg.h: Fix MISRA C 2012 Rule 20.7 violation

From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Xenia Ragiadakou <burzalodowa@gmail.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH] xen/arm64: sysreg.h: Fix MISRA C 2012 Rule 20.7 violation
Date: Thu, 28 Jul 2022 15:34:20 +0100	[thread overview]
Message-ID: <c71c9522-2df5-35a3-d39c-706d5c9d5263@xen.org> (raw)
In-Reply-To: <83c17bf6-b9b2-a297-6f7f-dd08231d0f90@suse.com>

Hi,

On 28/07/2022 15:20, Jan Beulich wrote:
> On 28.07.2022 15:56, Julien Grall wrote:
>> On 28/07/2022 14:49, Xenia Ragiadakou wrote:
>>> --- a/xen/arch/arm/include/asm/arm64/sysregs.h
>>> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h
>>> @@ -461,7 +461,7 @@
>>>    /* Access to system registers */
>>>    
>>>    #define WRITE_SYSREG64(v, name) do {                    \
>>> -    uint64_t _r = v;                                    \
>>> +    uint64_t _r = (v);                                              \
>>
>> I am failing to see why the parentheses are necessary here. Could you
>> give an example where the lack of them would end up to different code?
> 
> I think it is merely good practice to parenthesize the right sides of =.
> Indeed with assignment operators having second to lowest precedence, and
> with comma (the lowest precedence one) requiring parenthesization at the
> macro invocation site, there should be no real need for parentheses here.

I am not really happy with adding those parentheses because they are 
pointless. But if there are a consensus to use it, then the commit 
message should be updated to clarify this is just here to please MISRA 
(to me "need" implies it would be bug).

Cheers,

-- 
Julien Grall