Re: [PATCH] xen/char: imx-lpuart: Fix MISRA C 2012 Rule 20.7 violation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.08.2022 14:40, Xenia Ragiadakou wrote:
> On 8/2/22 14:58, Jan Beulich wrote:
>> On 02.08.2022 09:54, Xenia Ragiadakou wrote:
>>> --- a/xen/drivers/char/imx-lpuart.c
>>> +++ b/xen/drivers/char/imx-lpuart.c
>>> @@ -26,8 +26,8 @@
>>>   #include <asm/imx-lpuart.h>
>>>   #include <asm/io.h>
>>>   
>>> -#define imx_lpuart_read(uart, off)       readl((uart)->regs + off)
>>> -#define imx_lpuart_write(uart, off, val) writel((val), (uart)->regs + off)
>>> +#define imx_lpuart_read(uart, off)       readl((uart)->regs + (off))
>>> +#define imx_lpuart_write(uart, off, val) writel((val), (uart)->regs + 
>>> (off))
>>
>> As elsewhere before I think at the same time you want to drop the
>> parentheses from the single use of "val".
>>
> 
> In general I do not want to include irrelevant changes in my patches.
> Also, here, I do not want to drop the parentheses from val because the 
> removal of the parentheses
> - consists a violation of the rule 20.7
> - would allow the following to compile
> #define VAL x, y, z);(
> imx_lpuart_write(uart, off, VAL)

Parenthesization won't help against all forms of odd use of parentheses
in macro expansions anyway. Maybe MISRA should (or even does) have a
rule disallowing unbalanced parentheses (an square brackets) in macro
expansions ...

> - is not justifiable (i.e does not fix a bug, does not result in more 
> readable code etc)

As said before, I very much view too many parentheses as affecting
readability.

> I understand that the rest of the community does not agree with adding 
> parentheses around macro parameters used as function arguments

This may indeed be the case, while ...

> and as 
> the right-side operand of the assignment operator,

... iirc for this one it was only Julien so far who disliked the
addition of parentheses.

> but I consider them 
> useful and I do not want to remove them myself. Maybe somebody else from 
> the community could do that.

Fair enough; my remark was indeed _only_ a remark - the maintainers
will have to judge in the end.

Jan