Re: [XEN PATCH v3 07/12] x86/hvm: address violations of MISRA C Rule 16.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 2 Jul 2024, Federico Serafini wrote:
> On 01/07/24 10:47, Jan Beulich wrote:
> > On 26.06.2024 11:28, Federico Serafini wrote:
> > > @@ -2798,11 +2800,12 @@ void hvm_emulate_one_vm_event(enum emul_kind kind,
> > > unsigned int trapnr,
> > >           hvio->mmio_insn_bytes = sizeof(hvio->mmio_insn);
> > >           memcpy(hvio->mmio_insn, curr->arch.vm_event->emul.insn.data,
> > >                  hvio->mmio_insn_bytes);
> > > +        fallthrough;
> > >       }
> > > -    /* Fall-through */
> > >       default:
> > 
> > Can you clarify for me please whether this arrangement actually helps?
> > I'm pretty sure it'll result in a Coverity complaint, as my understanding
> > is that for them the marker (comment or pseudo-keyword) has to immediately
> > precede the subsequent label. IOW even if you confirmed that Eclair is
> > smarter in this regard, it may still need converting to
> > 
> >          hvio->mmio_insn_bytes = sizeof(hvio->mmio_insn);
> >          memcpy(hvio->mmio_insn, curr->arch.vm_event->emul.insn.data,
> >                 hvio->mmio_insn_bytes);
> >      }
> >          fallthrough;
> >      default:
> > 
> 
> Yes, this is ok for ECLAIR.

Given that Jan might be right that Coverity and others would prefer the
keyword on the line immediately above "default", and given that it works
anyway for ECLAIR, then I think it would be better to stay on the safe
side and move the "fallback" right on top of default.

If you are OK with it, please resend this patch and following patches.
Patches 1-6 are fully acked and I'd be happy to take them in my for-4.20
branch.