Re: [libvirt] [PATCH v3 7/7] python: add bindings for virConnectGetCPUModelNames
On 09/11/2013 08:13 AM, Giuseppe Scrivano wrote:
Signed-off-by: Giuseppe Scrivano <gscrivan(a)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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)