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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list