Re: [libvirt] [libvirt-python PATCH 16/23] change the order of some statements

On 09/24/2015 10:01 AM, Pavel Hrdina wrote: > This change makes it easier to free allocated object especially for > python objects. We can benefit from the fact, that if you call > Py_DECREF on eny python object it will also remove reference for all s/eny/any > assigned object to the root object. For example, calling Py_DECREF on > dict will also remove reference recursively on all elements in that > dictionary. Our job is then just call Py_DECREF on the root element and > don't care about anything else. > Something else that could go in HACKING - usage of Py_[X]DECREF...

> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > libvirt-override.c | 189 ++++++++++++++++++++++++----------------------------- > 1 file changed, 85 insertions(+), 104 deletions(-) > > diff --git a/libvirt-override.c b/libvirt-override.c > index 92c31b8..63a469b 100644 > --- a/libvirt-override.c > +++ b/libvirt-override.c > @@ -1238,9 +1238,16 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, > goto cleanup; > if ((pycpuinfo = PyList_New(dominfo.nrVirtCpu)) == NULL) > goto cleanup; > + > + if (PyTuple_SetItem(pyretval, 0, pycpuinfo) < 0) > + goto cleanup; > + > if ((pycpumap = PyList_New(dominfo.nrVirtCpu)) == NULL) > goto cleanup; > > + if (PyTuple_SetItem(pyretval, 1, pycpumap) < 0) > + goto cleanup; > + > for (i = 0; i < dominfo.nrVirtCpu; i++) { > PyObject *info = PyTuple_New(4); > PyObject *item = NULL; > @@ -1248,54 +1255,42 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, > if (info == NULL) > goto cleanup; > > + if (PyList_SetItem(pycpuinfo, i, info) < 0) > + goto cleanup; > + > if ((item = libvirt_intWrap((long)cpuinfo[i].number)) == NULL || > PyTuple_SetItem(info, 0, item) < 0) > - goto itemError; > + goto cleanup; > > if ((item = libvirt_intWrap((long)cpuinfo[i].state)) == NULL || > PyTuple_SetItem(info, 1, item) < 0) > - goto itemError; > + goto cleanup; > > if ((item = libvirt_ulonglongWrap(cpuinfo[i].cpuTime)) == NULL || > PyTuple_SetItem(info, 2, item) < 0) > - goto itemError; > + goto cleanup; > > if ((item = libvirt_intWrap((long)cpuinfo[i].cpu)) == NULL || > PyTuple_SetItem(info, 3, item) < 0) > - goto itemError; > - > - if (PyList_SetItem(pycpuinfo, i, info) < 0) > - goto itemError; > - > - continue; > - itemError: > - Py_DECREF(info); > - Py_XDECREF(item); > - goto cleanup; > + goto cleanup; > } > for (i = 0; i < dominfo.nrVirtCpu; i++) { > PyObject *info = PyTuple_New(cpunum); > size_t j; > if (info == NULL) > goto cleanup; > + > + if (PyList_SetItem(pycpumap, i, info) < 0) > + goto cleanup; > + > for (j = 0; j < cpunum; j++) { > PyObject *item = NULL; > if ((item = PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, > i, j))) == NULL || > - PyTuple_SetItem(info, j, item) < 0) { > - Py_DECREF(info); > - Py_XDECREF(item); > + PyTuple_SetItem(info, j, item) < 0) > goto cleanup; > - } > - } > - if (PyList_SetItem(pycpumap, i, info) < 0) { > - Py_DECREF(info); > - goto cleanup; > } > } > - if (PyTuple_SetItem(pyretval, 0, pycpuinfo) < 0 || > - PyTuple_SetItem(pyretval, 1, pycpumap) < 0) > - goto cleanup; > > VIR_FREE(cpuinfo); > VIR_FREE(cpumap); > @@ -1306,8 +1301,6 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED, > VIR_FREE(cpuinfo); > VIR_FREE(cpumap); > Py_XDECREF(pyretval); > - Py_XDECREF(pycpuinfo); > - Py_XDECREF(pycpumap); > return error; > } Splatter from bleeding eyeballs - perhaps would have been easier to read with incremental change ;-) > > @@ -1489,12 +1482,13 @@ libvirt_virDomainGetVcpuPinInfo(PyObject *self ATTRIBUTE_UNUSED, > if (mapinfo == NULL) > goto cleanup; > > + PyList_SetItem(pycpumaps, vcpu, mapinfo); > + > for (pcpu = 0; pcpu < cpunum; pcpu++) { > PyTuple_SetItem(mapinfo, pcpu, > PyBool_FromLong(VIR_CPU_USABLE(cpumaps, cpumaplen, > vcpu, pcpu))); > } > - PyList_SetItem(pycpumaps, vcpu, mapinfo); > } > > VIR_FREE(cpumaps); > @@ -1877,12 +1871,14 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, > if ((list = PyTuple_New(2)) == NULL) > goto cleanup; > > + Py_XINCREF(libvirt_virPythonErrorFuncCtxt); > + PyTuple_SetItem(list, 0, libvirt_virPythonErrorFuncCtxt); > + > if ((info = PyTuple_New(9)) == NULL) > goto cleanup; > > - PyTuple_SetItem(list, 0, libvirt_virPythonErrorFuncCtxt); > PyTuple_SetItem(list, 1, info); > - Py_XINCREF(libvirt_virPythonErrorFuncCtxt); > + > PyTuple_SetItem(info, 0, libvirt_intWrap((long) err->code)); > PyTuple_SetItem(info, 1, libvirt_intWrap((long) err->domain)); > PyTuple_SetItem(info, 2, libvirt_constcharPtrWrap(err->message)); > @@ -1894,14 +1890,11 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, > PyTuple_SetItem(info, 8, libvirt_intWrap((long) err->int2)); > /* TODO pass conn and dom if available */ > result = PyEval_CallObject(libvirt_virPythonErrorFuncHandler, list); > - Py_XDECREF(list); > Py_XDECREF(result); > } > > cleanup: > Py_XDECREF(list); > - Py_XDECREF(info); > - This one I'm confused by why 'info' is removed...

The rest seemed OK - a couple were tough to follow, but not impossible. John > LIBVIRT_RELEASE_THREAD_STATE; > } >