[libvirt] [libvirt-python PATCH 03/23] remove useless check for NULL before Py_XDECREF

Pavel Hrdina phrdina at redhat.com
Tue Sep 29 14:29:49 UTC 2015


On Sat, Sep 26, 2015 at 09:08:35AM -0400, John Ferlan wrote:
> 
> 
> On 09/24/2015 10:01 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  libvirt-override.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libvirt-override.c b/libvirt-override.c
> > index 14aa0e9..114104b 100644
> > --- a/libvirt-override.c
> > +++ b/libvirt-override.c
> > @@ -2303,12 +2303,9 @@ libvirt_virRegisterErrorHandler(ATTRIBUTE_UNUSED PyObject * self,
> >            pyobj_f);
> >  
> >      virSetErrorFunc(NULL, libvirt_virErrorFuncHandler);
> > -    if (libvirt_virPythonErrorFuncHandler != NULL) {
> > -        Py_XDECREF(libvirt_virPythonErrorFuncHandler);
> > -    }
> > -    if (libvirt_virPythonErrorFuncCtxt != NULL) {
> > -        Py_XDECREF(libvirt_virPythonErrorFuncCtxt);
> > -    }
> > +
> > +    Py_XDECREF(libvirt_virPythonErrorFuncHandler);
> > +    Py_XDECREF(libvirt_virPythonErrorFuncCtxt);
> 
> I keep looking at this and thinking why?  Why was it added and what is
> it protecting.  Looking at libvirt_virErrorFuncHandler it seems only
> libvirt_virPythonErrorFuncCtxt gets the Py_XINCREF and that's based on
> whether libvirt_virPythonErrorFuncHandler is NULL or not.

I don't understand what do you mean by this??

> 
> Just feels like something subtle is going on. Also it's a register
> handler and we're decrementing something that never got incremented.

Check the whole function.  It will decrement the old handlers, check for
existence of new handlers and clean/set new handlers.  And before setting the
new handlers, we need to increment them, otherwise they will be cleared by GC.

Everything that is parsed by 'PyArg_ParseTuple' has to be incremented, if you
need to use those object outside of the function scope, where you've called that
parser.

Pavel

> 
> John
> >  
> >      if ((pyobj_f == Py_None) && (pyobj_ctx == Py_None)) {
> >          libvirt_virPythonErrorFuncHandler = NULL;
> > 




More information about the libvir-list mailing list