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

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxxxxx>
>>
>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>
>>
>>> ---
>>> 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