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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 1 Oct 2024, Jan Beulich wrote:
> On 01.10.2024 00:53, Stefano Stabellini wrote:
> > On Mon, 30 Sep 2024, 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:                                         \
> >> +    {                                                               \
> >> +        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;                                           \
> >> +    }                                                               \
> >>          break
> > 
> > We would typically put the break within the case { }
> 
> That won't work very well with the break not having a semicolon, for the
> semicolon to actually be used when invoking the macro. Moving the break
> (while adding a semicolon there) as you suggest would then mean the use
> site semicolon to end up being an unreachable statement.

I didn't think of the extra semicolon posing a problem. In that case, it
is better as it is in this patch