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

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 13/07/24 01:25, Stefano Stabellini wrote:
> 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.
V4 sent:
https://lists.xenproject.org/archives/html/xen-devel/2024-07/msg00823.html

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)