Re: [XEN PATCH 2/3] xen/gnttab: address violations of MISRA C Rule 13.6

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 11 Sep 2024, Jan Beulich wrote:
> On 10.09.2024 21:06, Federico Serafini wrote:
> > --- a/xen/common/compat/grant_table.c
> > +++ b/xen/common/compat/grant_table.c
> > @@ -78,12 +78,15 @@ int compat_grant_table_op(
> >          cmd_op = cmd;
> >      switch ( cmd_op )
> >      {
> > -#define CASE(name) \
> > -    case GNTTABOP_##name: \
> > -        if ( unlikely(!guest_handle_okay(guest_handle_cast(uop, \
> > -                                                           
> > gnttab_##name##_compat_t), \
> > -                                         count)) ) \
> > -            rc = -EFAULT; \
> > +#define CASE(name)                                                      \
> > +        case GNTTABOP_ ## name:                                         \
> 
> Why the re-indentation? The earlier way was pretty intentional, to match
> what a non-macroized case label would look like in this switch.
> 
> > +        {                                                               \
> > +            XEN_GUEST_HANDLE_PARAM(gnttab_ ## name ## _compat_t) h =    \
> > +                guest_handle_cast(uop, gnttab_ ## name ## _compat_t);   \
> > +                                                                        \
> > +            if ( unlikely(!guest_handle_okay(h, count)) )               \
> > +                rc = -EFAULT;                                           \
> 
> Same question as for the earlier patch - where's the potential side
> effect?

Leaving aside the re-indentation / readability changes, I think the fix
is to move the call to guest_handle_cast out of guest_handle_okay.

Since guest_handle_okay is implemented by calling sizeof on *h.p, I am
guessing that passing guest_handle_cast() as h causes problems. I am not
sure if there is a side effect, I cannot spot one, but I can see that
the nested macros could cause issues to a static analyzer.