[libvirt] [PATCH 1/2] python: cleanup vcpu related binding APIs

[Eric Blake]

On 04/17/2012 11:45 AM, Guannan Ren wrote:
>     *libvirt_virDomainGetVcpus: add error handling, return -1 instead of None
>     *libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags:
>          make use of libvirt_boolUnwrap
> 
>          set vcpus which are list in tuple pycpumap, trun off the rest
>          in cpumap. The original way ignored the error info from PyTuple_GetItem
>          if pos is out of bounds. "IndexError: tuple index out of range"
>          The error message will only be raised on next command in interactive mode.
> ---
>  python/libvirt-override.c |  173 +++++++++++++++++++++++++++++---------------
>  1 files changed, 114 insertions(+), 59 deletions(-)

> @@ -1388,54 +1394,80 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED,
>          PyObject *info = PyTuple_New(4);
>          if (info == NULL)
>              goto cleanup;
> -        PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number));
> -        PyTuple_SetItem(info, 1, PyInt_FromLong((long)cpuinfo[i].state));
> -        PyTuple_SetItem(info, 2, PyLong_FromLongLong((long long)cpuinfo[i].cpuTime));
> -        PyTuple_SetItem(info, 3, PyInt_FromLong((long)cpuinfo[i].cpu));
> -        PyList_SetItem(pycpuinfo, i, info);
> +        if (PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number)) < 0)

Ouch.  PyInt_FromLong can return NULL (on OOM situations), but
PyTuple_SetItem() doesn't take too kindly to NULL arguments.  You need
to use temporary variables through here.

>      for (i = 0 ; i < dominfo.nrVirtCpu ; i++) {
>          PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo));
> -        int j;
>          if (info == NULL)
>              goto cleanup;
> +
> +        int j;

Why did you sink the declaration of j?  We require C99, so it works, but
it's not our usual style.

>          for (j = 0 ; j < VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) {
> -            PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j)));
> +            if (PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, j))) < 0)

Again, this needs a temporary to avoid calling PyTuple_SetItem(NULL).

> +
> +        if ((!flag) || (libvirt_boolUnwrap(flag, &b) < 0))

Redundant parenthesis.  This could be written:

if (!flag || libvirt_boolUnwrap(flag, &b) < 0)

Not quite ready, but I like the direction that your headed.

-- 
Eric Blake   eblake at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120417/08fbce02/attachment-0001.sig>