[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/11] qemu: extract helper to gather vcpu data



----- Original Message -----
> From: "Eric Blake" <eblake redhat com>
> To: "Francesco Romani" <fromani redhat com>, libvir-list redhat com
> Sent: Tuesday, September 2, 2014 11:42:09 PM
> Subject: Re: [libvirt] [PATCH 02/11] qemu: extract helper to gather vcpu data
> 
> On 09/02/2014 06:31 AM, Francesco Romani wrote:
> > Extracts an helper to gether the VCpu
> 
> s/an/a/
> s/gether/gather/

Will fix,

> >  virQEMUDriverPtr qemu_driver = NULL;
> >  
> >  
> > @@ -4974,10 +4980,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
> >                     int maplen)
> >  {
> >      virDomainObjPtr vm;
> > -    size_t i;
> > -    int v, maxcpu, hostcpus;
> >      int ret = -1;
> > -    qemuDomainObjPrivatePtr priv;
> >  
> >      if (!(vm = qemuDomObjFromDomain(dom)))
> >          goto cleanup;
> > @@ -4992,7 +4995,25 @@ qemuDomainGetVcpus(virDomainPtr dom,
> >          goto cleanup;
> >      }
> >  
> > -    priv = vm->privateData;
> > +    ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen);
> > +
> > + cleanup:
> > +    if (vm)
> > +        virObjectUnlock(vm);
> 
> Ouch.  You have a double free.  This frees vm, even though it was calling...
> 
> > +    return ret;
> > +}
> > +
> > +static int
> > +qemuDomainHelperGetVcpus(virDomainObjPtr vm,
> > +                         virVcpuInfoPtr info,
> > +                         int maxinfo,
> > +                         unsigned char *cpumaps,
> > +                         int maplen)
> > +{
> > +    int ret = -1;
> > +    int v, maxcpu, hostcpus;
> > +    size_t i;
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> 
> ...a function that now has transfer semantics.  But unlike patch 1,
> where transfer semantics were necessary because of the way you drop lock
> in order to do a monitor call, this patch appears to not need them; and
> the solution is to just sanitize the cleanup label (at which point it
> becomes a mere 'return ret', so you could then replace all 'goto
> cleanup' with a direct return).

Thanks, will fix.

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]