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@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