[libvirt] [PATCH 1/2] python: prefer PyList_SET_ITEM to PyList_SetItem

[Martin Kletzander]

On Thu, Nov 07, 2013 at 03:44:22PM +0100, Giuseppe Scrivano wrote:
> The PyList_SET_ITEM macro, differently from PyList_SetItem, doesn't do
> any error checking and overwrites anything that was previously stored
> in the list at the chosen destination position.
> 
> PyList_SET_ITEM is usually faster than PyList_SetItem, and since it is
> used only to fill freshly created lists, it is safe to be used here.
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  python/libvirt-override.c | 197 +++++++++++++++++++++-------------------------
>  1 file changed, 90 insertions(+), 107 deletions(-)
> 
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index 2e58bf9..83bca94 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
[...]
> @@ -2575,13 +2569,12 @@ libvirt_virDomainListAllSnapshots(PyObject *self ATTRIBUTE_UNUSED,
>          goto cleanup;
>  
>      for (i = 0; i < c_retval; i++) {
> -        if ((pyobj_snap = libvirt_virDomainSnapshotPtrWrap(snaps[i])) == NULL ||
> -            PyList_SetItem(py_retval, i, pyobj_snap) < 0) {
> -            Py_XDECREF(pyobj_snap);
> +        if (!(pyobj_snap = libvirt_virDomainSnapshotPtrWrap(snaps[i]))) {

Not that it's a problem, just note that you're changing the '== NULL'
to '!' here, but not elsewhere...

>              Py_DECREF(py_retval);
>              py_retval = NULL;
>              goto cleanup;
>          }
> +        PyList_SET_ITEM(py_retval, i, pyobj_snap);
>          snaps[i] = NULL;
>      }
>  
[...]
> @@ -3219,7 +3209,7 @@ libvirt_virNodeGetCellsFreeMemory(PyObject *self ATTRIBUTE_UNUSED, PyObject *arg
>      }
>      py_retval = PyList_New(c_retval);
>      for (i = 0; i < c_retval; i++) {
> -        PyList_SetItem(py_retval, i,
> +        PyList_SET_ITEM(py_retval, i,
>                  libvirt_longlongWrap((long long) freeMems[i]));

Pre-existing bad indentation.

>      }
>      VIR_FREE(freeMems);
> @@ -3398,7 +3388,7 @@ libvirt_virConnectListStoragePools(PyObject *self ATTRIBUTE_UNUSED,
>  
>      if (names) {
>          for (i = 0; i < c_retval; i++) {
> -            PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
> +            PyList_SET_ITEM(py_retval, i, libvirt_constcharPtrWrap(names[i]));
>              VIR_FREE(names[i]);
>          }
>          VIR_FREE(names);
[...]
> @@ -3654,12 +3642,12 @@ libvirt_virStoragePoolGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
>      if ((py_retval = PyList_New(4)) == NULL)
>          return VIR_PY_NONE;
>  
> -    PyList_SetItem(py_retval, 0, libvirt_intWrap((int) info.state));
> -    PyList_SetItem(py_retval, 1,
> +    PyList_SET_ITEM(py_retval, 0, libvirt_intWrap((int) info.state));
> +    PyList_SET_ITEM(py_retval, 1,
>                     libvirt_longlongWrap((unsigned long long) info.capacity));
> -    PyList_SetItem(py_retval, 2,
> +    PyList_SET_ITEM(py_retval, 2,
>                     libvirt_longlongWrap((unsigned long long) info.allocation));
> -    PyList_SetItem(py_retval, 3,
> +    PyList_SET_ITEM(py_retval, 3,
>                     libvirt_longlongWrap((unsigned long long) info.available));
>      return py_retval;
>  }

Indentation is off after your patch in this whole hunk.

> @@ -3685,10 +3673,10 @@ libvirt_virStorageVolGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
>  
>      if ((py_retval = PyList_New(3)) == NULL)
>          return VIR_PY_NONE;
> -    PyList_SetItem(py_retval, 0, libvirt_intWrap((int) info.type));
> -    PyList_SetItem(py_retval, 1,
> +    PyList_SET_ITEM(py_retval, 0, libvirt_intWrap((int) info.type));
> +    PyList_SET_ITEM(py_retval, 1,
>                     libvirt_longlongWrap((unsigned long long) info.capacity));
> -    PyList_SetItem(py_retval, 2,
> +    PyList_SET_ITEM(py_retval, 2,
>                     libvirt_longlongWrap((unsigned long long) info.allocation));
>      return py_retval;
>  }

ditto

[...]

... but ACK as-is, because there are more indentation problems,
anyway.  So I've prepared a follow-up patch cleaning few bits in this
file.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131112/411ef0ba/attachment-0001.sig>