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

[Julien Grall <julien@xxxxxxx>]

On 28/07/2022 16:15, Xenia Ragiadakou wrote:
> On 7/28/22 16:56, Julien Grall wrote:
> > Hi,
> >
> > On 28/07/2022 14:49, Xenia Ragiadakou wrote:
> > > The macro parameter 'v' is used as an expression and needs to be 
> > > enclosed in
> > > parentheses.
> > >
> > > Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> > > ---
> > >   xen/arch/arm/include/asm/arm64/sysregs.h | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h 
> > > b/xen/arch/arm/include/asm/arm64/sysregs.h
> > > index 54670084c3..f5a7269a27 100644
> > > --- 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?
> Here v is supposed to be used as an expression. So maybe the rule wants 
> to enforce that in the macro we will pass an expression and not multiple 
> statements (?) ... not sure.
Do you mean something like below?

#include <stdio.h>
#include <inttypes.h>

#define foo(v)  do {       \
    uint64_t _r = v;    \
                        \
        printf("_r %lu\n", _r); \
    } while (0)

int main(void)
{
    foo(1; printf("Bar\n"));
}

If yes, then I agree that the compiler will still be happy. Or if you 
pass "1; 2", v would be set to 1 rather than 2. I am not sure why one 
would want to write such code. But I guess we want to prevent that.
Cheers,

--
Julien Grall