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

[Pavel Hrdina]

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 at redhat.com>
> > ---
> >  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;
> >  }
> >  

[...]