Re: [XEN PATCH v2 1/3] drivers: char: address violation of MISRA C Rule 20.7

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 2024-05-03 12:10, Jan Beulich wrote:
> On 03.05.2024 09:29, Nicola Vetrini wrote:
>> On 2024-05-01 21:57, Stefano Stabellini wrote:
>>> On Tue, 30 Apr 2024, Nicola Vetrini wrote:
>>>> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
>>>> of macro parameters shall be enclosed in parentheses". Therefore, 
>>>> some
>>>> macro definitions should gain additional parentheses to ensure that
>>>> all
>>>> current and future users will be safe with respect to expansions 
>>>> that
>>>> can possibly alter the semantics of the passed-in macro parameter.
>>>> 
>>>> No functional chage.
>>>> 
>>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>> 
>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>> 
>>> 
>>>> ---
>>>> Changes in v2:
>>>> - drop excess parentheses from val parameter.
>>>> ---
>>>>  xen/drivers/char/omap-uart.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/xen/drivers/char/omap-uart.c
>>>> b/xen/drivers/char/omap-uart.c
>>>> index 03b5b66e7acb..e0128225f927 100644
>>>> --- a/xen/drivers/char/omap-uart.c
>>>> +++ b/xen/drivers/char/omap-uart.c
>>>> @@ -48,8 +48,9 @@
>>>>  /* System configuration register */
>>>>  #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup 
>>>> is
>>>> enabled */
>>>> 
>>>> -#define omap_read(uart, off)       readl((uart)->regs +
>>>> (off<<REG_SHIFT))
>>>> -#define omap_write(uart, off, val) writel((val), (uart)->regs +
>>>> (off<<REG_SHIFT))
>>>> +#define omap_read(uart, off)       readl((uart)->regs + ((off) <<
>>>> REG_SHIFT))
>>>> +#define omap_write(uart, off, val) writel(val, (uart)->regs + \
>>>> +                                               ((off) << 
>>>> REG_SHIFT))
>>> 
>>> the alignment looks off but could be fixed on commit
>>> 
>> 
>> Can you clarify what you mean here? I aligned readl and writeln and 
>> the
>> operands in writel to avoid the line being too long.
> 
> #define omap_write(uart, off, val) writel(val, \
>                                           (uart)->regs + ((off) << 
> REG_SHIFT))
> 
> The main point being that before you start splitting an argument 
> following
> another one on the same line, you'd move that argument to a new line.
> 
> Jan

Oh, ok. Didn't think of that. Thanks, can amend it

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)