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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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@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
Oh, ok. Didn't think of that. Thanks, can amend it

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