Re: [Qemu-ppc] [PATCH] spapr: add missing break in h_get_cpu_characteristics()

[Daniel Henrique Barboza]

On 02/02/2018 08:00 AM, Greg Kurz wrote:
> On Fri, 2 Feb 2018 07:11:08 -0200
> Daniel Henrique Barboza <address@hidden> wrote:
>
> > On 02/01/2018 05:47 PM, Greg Kurz wrote:
> > > Detected by Coverity (CID 1385702). This fixes the recently added hypercall
> > > to let guests properly apply Spectre and Meltdown workarounds.
> > Paolo Bonzini reported this error in a reply to the pull request that
> > added the patch:
> >
> > "Re: [Qemu-ppc] [Qemu-devel] [PULL 12/12] target/ppc/spapr: Add H-Call
> > H_GET_CPU_CHARACTERISTICS
> >
> > On 28/01/2018 22:28, David Gibson wrote:
> >
> > > +    switch (safe_indirect_branch) {
> > > +    case SPAPR_CAP_FIXED:
> > > +        characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> > Missing "break;" here.
> >
> > Paolo
> >
> > "
> >
> > I think it is nice to mention in the commit msg that Paolo also detected
> > this same error,
> > specially given that his email was sent before this patch.
> Heh, Paolo's mail landed in the pull req thread in my mail client and I saw
> it after sending the patch :P ... also I'm pretty sure Paolo was made aware
> of this issue by Coverity, just as I was :)
>
> From: address@hidden
> To: address@hidden
> Subject: New Defects reported by Coverity Scan for QEMU
> Date: Thu, 01 Feb 2018 18:11:33 +0000 (UTC)
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to QEMU found with
> Coverity Scan.
>
> ...
>
> *** CID 1385702:  Control flow issues  (MISSING_BREAK)
> /hw/ppc/spapr_hcall.c: 1700 in h_get_cpu_characteristics()
> 1694             break;
> 1695         }
> 1696
> 1697         switch (safe_indirect_branch) {
> 1698         case SPAPR_CAP_FIXED:
> 1699             characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> > > >      CID 1385702:  Control flow issues  (MISSING_BREAK)
> > > >      The above case falls through to this one.
> 1700         default: /* broken */
> 1701             assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
> 1702             break;
> 1703         }
> 1704
> 1705         args[0] = characteristics;
>
>
> No big deal I guess :)

Roger that!

> > Thanks,
> >
> >
> > Daniel
> >
> >
> > > Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
> > > Signed-off-by: Greg Kurz <address@hidden>
> > > ---
> > >    hw/ppc/spapr_hcall.c |    1 +
> > >    1 file changed, 1 insertion(+)
> > >
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index 4d0e6eb0cf1d..596f58378a40 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU 
> > > *cpu,
> > >        switch (safe_indirect_branch) {
> > >        case SPAPR_CAP_FIXED:
> > >            characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> > > +        break;
> > >        default: /* broken */
> > >            assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
> > >            break;
> > >
> > >