On 09/11/2013 08:13 AM, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- python/generator.py | 2 +- python/libvirt-override-api.xml | 7 ++++++ python/libvirt-override.c | 52 +++++++++++++++++++++++++++++++++++++++++ python/libvirt-override.py | 11 +++++++++ 4 files changed, 71 insertions(+), 1 deletion(-)
+ + LIBVIRT_BEGIN_ALLOW_THREADS; + + c_retval = virConnectGetCPUModelNames(conn, arch, &models, flags); + + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval == -1) + return VIR_PY_INT_FAIL; + + if ((rv = PyList_New(c_retval)) == NULL) + goto error; + + for (i = 0; i < c_retval; i++) { + PyObject *str; + if ((str = PyString_FromString(models[i])) == NULL) + goto error; + + PyList_SET_ITEM(rv, i, str);
Elsewhere, we've used PyList_New(0)/PyList_Append() rather than PyList_New(count)/PyList_SET_ITEM(); but that's not universal; also, I see uses of PyList_SetItem but not PyList_SET_ITEM; what's the difference? More importantly, you STILL have a data leak. If c_retval is 2 but PyString_FromString(models[1]) fails (typically only possible on OOM), then we goto error and never free models. Maybe you can get some inspiration from libvirt_virDomainSnapshotListNames for how to properly have a single cleanup: label (instead of done:/error:), while handling error cases and setting up proper transfer semantics from an array of strings to a python list of strings. I'm looking forward to v4; the changes I suggested squashing in have all worked in my testing, and it is merely this patch and patch 2 that need actual fixes that I have not written. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org