Re: [RFC PATCH] x86/vlapic: address a violation of MISRA C:2012 Rule 16.2

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2023-11-21 16:36, Jan Beulich wrote:
> On 25.10.2023 15:22, Nicola Vetrini wrote:
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -1034,10 +1034,10 @@ int guest_wrmsr_x2apic(struct vcpu *v, 
> > uint32_t msr, uint64_t val)
> >      case APIC_EOI:
> >      case APIC_ESR:
> >          if ( val )
> > -        {
> > -    default:
> >              return X86EMUL_EXCEPTION;
> > -        }
> > +        break;
> > +    default:
> > +        return X86EMUL_EXCEPTION;
> >      }
> >
> >      vlapic_reg_write(v, array_index_nospec(offset, PAGE_SIZE), val);
> Considering the plan to confine applicability of the rule, one style 
> aspect
> which would need to be taken into account is that the entire rest of 
> this
> switch() has blank lines between case blocks.
>
> The other is that imo the overall result would be closer to what we 
> have
> right now if the new code was
>
>     case APIC_EOI:
>     case APIC_ESR:
>         if ( !val )
>             break;
>         fallthrough;
>     default:
>         return X86EMUL_EXCEPTION;
>      }
>
> at which point the need for the blank line would also disappear.
This is also a fine solution. I'll keep this in mind when this patch 
will be revisited.
> As to the description - isn't this change (whichever way done) also
> addressing another violation, requiring "break" (or alike according to
> our interpretation) at the end of each case block?
Correct. It's probably a good idea to mention that, but the 
"fallthrough" is also a candidate for a deviation from R16.3, so we'll 
see about that.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)