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

From: "Eric Blake" <eblake(a)redhat.com&gt; To: "Francesco Romani" <fromani(a)redhat.com&gt;, libvir-list(a)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/

> 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).