On Sat, Sep 26, 2015 at 09:56:03AM -0400, John Ferlan wrote:
>
>
> 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...
That's true, I can probably updated it during second "phase" of improving
libvirt-python. I'm planning to improve the generator in order to automatically
generate more APIs.
>
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> > 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 'info' is inserted to list right after it's allocation, so it's sufficient
to decref only list.
Pavel
>
> The rest seemed OK - a couple were tough to follow, but not impossible.
>
> John
>
> > LIBVIRT_RELEASE_THREAD_STATE;
> > }
> >
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list