[libvirt] [PATCH v3 7/7] python: add bindings for virConnectGetCPUModelNames

[Eric Blake]

On 09/11/2013 08:13 AM, Giuseppe Scrivano wrote:
> Signed-off-by: Giuseppe Scrivano <gscrivan at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130913/d1f839f2/attachment-0001.sig>