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
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
