[libvirt] [PATCH 2/2] python: add error checking for calls to PyList_New

[Martin Kletzander]

On Tue, Nov 12, 2013 at 07:00:30AM -0700, Eric Blake wrote:
> On 11/12/2013 06:53 AM, Martin Kletzander wrote:
> >> @@ -2621,7 +2630,8 @@ libvirt_virDomainSnapshotListChildrenNames(PyObject *self ATTRIBUTE_UNUSED,
> >>              return VIR_PY_NONE;
> >>          }
> >>      }
> >> -    py_retval = PyList_New(c_retval);
> >> +    if (!(py_retval = PyList_New(c_retval)))
> >> +        goto cleanup;
> >>  
> > 
> > This function should follow others and return VIR_PY_NONE, but cleanup
> > path returns py_retval.
> 
> Actually, you WANT to return NULL, not VIR_PY_NONE, when PyList_New()
> fails.  Returning NULL is the hint to python to report the OOM
> exception; returning VIR_PY_NONE is not NULL and therefore silently
> papers over the exception.  Worse, the rest of the libvirt python code
> treats a return of the python object 'None' as meaning 'the API call
> failed, dig out the last virError and turn it into a python exception';
> but in this case, there is no virError (our failure was not related to a
> failed C API call).  This code rewrite is correct as-is.
> 

Oh, I learned something new again.  Thanks for the explanation.  In
that case, I'm afraid there are more places where VIR_PY_NONE is returned
instead of NULL when allocation failed.  I'll check the code and add
it to my whitespace cleanup.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131112/62c17531/attachment-0001.sig>