On 09/24/2015 10:01 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@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. Just feels like something subtle is going on. Also it's a register handler and we're decrementing something that never got incremented. John
if ((pyobj_f == Py_None) && (pyobj_ctx == Py_None)) { libvirt_virPythonErrorFuncHandler = NULL;