On Wed, Nov 15, 2006 at 02:20:33AM +0000, Daniel P. Berrange wrote: > The attached patch implements the virListDefinedDomains method in the python > bindings, since it wasn't being automatically generated by the generator.py > script. > + c_retval = virConnectNumOfDefinedDomains(conn); > + if (c_retval < 0) { > + Py_INCREF(Py_None); > + return (Py_None); > + } > + > + if (c_retval) { > + names = malloc(sizeof(char *) * c_retval); Miss NULL return check here I think. > + c_retval = virConnectListDefinedDomains(conn, names, c_retval); > + if (c_retval < 0) { > + free(names); > + Py_INCREF(Py_None); > + return(Py_None); > + } > + } > + py_retval = PyList_New(c_retval); the paranoid in me would feel safer if the for loop was protected by a test for names not being NULL even if I agree it's not strictly needed :-) > + for (i = 0;i < c_retval;i++) { > + PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i])); > + free(names[i]); > + } > + > + if (names) > + free(names); > + > + return(py_retval); > + } Please commit after adding the malloc return check :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/