Re: [libvirt] [libvirt-python PATCH 22/23] use VIR_PY_DICT_SET_GOTO

On 09/24/2015 10:01 AM, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-override.c | 275 +++++++++++++++++++---------------------------------- libvirt-utils.c | 13 +-- 2 files changed, 99 insertions(+), 189 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c index 1f795d9..3192e74 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -403,18 +403,14 @@ libvirt_virDomainMemoryStats(PyObject *self ATTRIBUTE_UNUSED, } val = libvirt_ulonglongWrap(stats[i].val);

- if (!key || !val || PyDict_SetItem(info, key, val) < 0) { - Py_CLEAR(info); - goto cleanup; - } - Py_CLEAR(key); - Py_CLEAR(val); + VIR_PY_DICT_SET_GOTO(info, key, val, error); }

- cleanup: - Py_XDECREF(key); - Py_XDECREF(val); return info; + + error: + Py_DECREF(info); + return NULL; }

static PyObject * @@ -3358,23 +3354,16 @@ libvirt_virNodeGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, key = libvirt_constcharPtrWrap(stats[i].field); val = libvirt_ulonglongWrap(stats[i].value);

- if (!key || !val || PyDict_SetItem(ret, key, val) < 0) { - Py_CLEAR(ret); - goto error; - } - - Py_DECREF(key); - Py_DECREF(val); + VIR_PY_DICT_SET_GOTO(ret, key, val, error); }

+ cleanup: VIR_FREE(stats); return ret;

error: - VIR_FREE(stats); - Py_XDECREF(key); - Py_XDECREF(val); - return ret; + Py_CLEAR(ret); + goto cleanup; }

static PyObject * @@ -3423,23 +3412,16 @@ libvirt_virNodeGetMemoryStats(PyObject *self ATTRIBUTE_UNUSED, key = libvirt_constcharPtrWrap(stats[i].field); val = libvirt_ulonglongWrap(stats[i].value);

- if (!key || !val || PyDict_SetItem(ret, key, val) < 0) { - Py_CLEAR(ret); - goto error; - } - - Py_DECREF(key); - Py_DECREF(val); + VIR_PY_DICT_SET_GOTO(ret, key, val, error); }

+ cleanup: VIR_FREE(stats); return ret;

error: - VIR_FREE(stats); - Py_XDECREF(key); - Py_XDECREF(val); - return ret; + Py_CLEAR(ret); + goto cleanup; }

static PyObject * @@ -4747,11 +4729,8 @@ libvirt_virDomainGetJobStats(PyObject *self ATTRIBUTE_UNUSED, if (!(dict = getPyVirTypedParameter(params, nparams))) goto cleanup;

- if (PyDict_SetItem(dict, libvirt_constcharPtrWrap("type"), - libvirt_intWrap(type)) < 0) { - Py_CLEAR(dict); - goto cleanup; - } + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("type"), + libvirt_intWrap(type), cleanup);

it's not clear in this case on error that there's a "Py_CLEAR(dict);" - think you may need a goto error which has the Py_CLEAR(dict) and goto cleanup

cleanup: virTypedParamsFree(params, nparams); @@ -4790,34 +4769,19 @@ libvirt_virDomainGetBlockJobInfo(PyObject *self ATTRIBUTE_UNUSED, if (c_ret == 0) return dict;

- if ((type = libvirt_intWrap(info.type)) == NULL || - PyDict_SetItemString(dict, "type", type) < 0) - goto error; - Py_DECREF(type); - - if ((bandwidth = libvirt_ulongWrap(info.bandwidth)) == NULL || - PyDict_SetItemString(dict, "bandwidth", bandwidth) < 0) - goto error; - Py_DECREF(bandwidth); - - if ((cur = libvirt_ulonglongWrap(info.cur)) == NULL || - PyDict_SetItemString(dict, "cur", cur) < 0) - goto error; - Py_DECREF(cur); - - if ((end = libvirt_ulonglongWrap(info.end)) == NULL || - PyDict_SetItemString(dict, "end", end) < 0) - goto error; - Py_DECREF(end); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("type"), + libvirt_intWrap(info.type), error); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("bandwidth"), + libvirt_ulongWrap(info.bandwidth), error); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("cur"), + libvirt_ulonglongWrap(info.cur), error); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("end"), + libvirt_ulonglongWrap(info.end), error);

build issue - 'end', 'cur', 'bandwidth', and 'type' no longer necessary

return dict;

error: Py_DECREF(dict);

Should this by Py_CLEAR or XDECREF? (I forget already)

- Py_XDECREF(type); - Py_XDECREF(bandwidth); - Py_XDECREF(cur); - Py_XDECREF(end); return NULL; }

@@ -4983,9 +4947,10 @@ libvirt_virDomainGetDiskErrors(PyObject *self ATTRIBUTE_UNUSED, goto cleanup;

for (i = 0; i < count; i++) { - PyDict_SetItem(py_retval, - libvirt_constcharPtrWrap(disks[i].disk), - libvirt_intWrap(disks[i].error)); + VIR_PY_DICT_SET_GOTO(py_retval, + libvirt_constcharPtrWrap(disks[i].disk), + libvirt_intWrap(disks[i].error), + error); }

cleanup: @@ -4995,6 +4960,10 @@ libvirt_virDomainGetDiskErrors(PyObject *self ATTRIBUTE_UNUSED, VIR_FREE(disks); } return py_retval; + + error: + Py_CLEAR(py_retval); + goto cleanup; }

@@ -5038,12 +5007,8 @@ libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED, if (!(py_iface = PyDict_New())) goto error;

- if ((py_iname = libvirt_charPtrWrap(iface->name)) == NULL || - PyDict_SetItem(py_retval, py_iname, py_iface) < 0) { - Py_XDECREF(py_iname); - Py_DECREF(py_iface); - goto error; - } + VIR_PY_DICT_SET_GOTO(py_retval, libvirt_charPtrWrap(iface->name), + py_iface, error);

build issue - 'py_iname' no longer necessary

if (iface->naddrs) { if (!(py_addrs = PyList_New(iface->naddrs))) { @@ -5053,20 +5018,11 @@ libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED, py_addrs = VIR_PY_NONE; }

- if ((py_iname = libvirt_constcharPtrWrap("addrs")) == NULL || - PyDict_SetItem(py_iface, py_iname, py_addrs) < 0) { - Py_XDECREF(py_iname); - Py_DECREF(py_addrs); - goto error; - } + VIR_PY_DICT_SET_GOTO(py_iface, libvirt_constcharPtrWrap("addrs"), + py_addrs, error);

- if ((py_iname = libvirt_constcharPtrWrap("hwaddr")) == NULL || - (py_ivalue = libvirt_constcharPtrWrap(iface->hwaddr)) == NULL || - PyDict_SetItem(py_iface, py_iname, py_ivalue) < 0) { - Py_XDECREF(py_iname); - Py_XDECREF(py_ivalue); - goto error; - } + VIR_PY_DICT_SET_GOTO(py_iface, libvirt_constcharPtrWrap("hwaddr"), + libvirt_constcharPtrWrap(iface->hwaddr), error);

build issue - 'py_ivalue' no longer necessary

for (j = 0; j < iface->naddrs; j++) { virDomainIPAddressPtr addr = &(iface->addrs[j]); @@ -5077,27 +5033,12 @@ libvirt_virDomainInterfaceAddresses(PyObject *self ATTRIBUTE_UNUSED,

VIR_PY_LIST_SET_GOTO(py_addrs, j, py_addr, error);

- if ((py_iname = libvirt_constcharPtrWrap("addr")) == NULL || - (py_ivalue = libvirt_constcharPtrWrap(addr->addr)) == NULL || - PyDict_SetItem(py_addr, py_iname, py_ivalue) < 0) { - Py_XDECREF(py_iname); - Py_XDECREF(py_ivalue); - goto error; - } - if ((py_iname = libvirt_constcharPtrWrap("prefix")) == NULL || - (py_ivalue = libvirt_intWrap(addr->prefix)) == NULL || - PyDict_SetItem(py_addr, py_iname, py_ivalue) < 0) { - Py_XDECREF(py_iname); - Py_XDECREF(py_ivalue); - goto error; - } - if ((py_iname = libvirt_constcharPtrWrap("type")) == NULL || - (py_ivalue = libvirt_intWrap(addr->type)) == NULL || - PyDict_SetItem(py_addr, py_iname, py_ivalue) < 0) { - Py_XDECREF(py_iname); - Py_XDECREF(py_ivalue); - goto error; - } + VIR_PY_DICT_SET_GOTO(py_addr, libvirt_constcharPtrWrap("addr"), + libvirt_constcharPtrWrap(addr->addr), error); + VIR_PY_DICT_SET_GOTO(py_addr, libvirt_constcharPtrWrap("prefix"), + libvirt_uintWrap(addr->prefix), error); + VIR_PY_DICT_SET_GOTO(py_addr, libvirt_constcharPtrWrap("type"), + libvirt_intWrap(addr->type), error); } }

@@ -6171,28 +6112,34 @@ libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE if ((pyobj_local = PyDict_New()) == NULL) goto cleanup;

- PyDict_SetItem(pyobj_local, - libvirt_constcharPtrWrap("family"), - libvirt_intWrap(local->family)); - PyDict_SetItem(pyobj_local, - libvirt_constcharPtrWrap("node"), - libvirt_constcharPtrWrap(local->node)); - PyDict_SetItem(pyobj_local, - libvirt_constcharPtrWrap("service"), - libvirt_constcharPtrWrap(local->service)); + VIR_PY_DICT_SET_GOTO(pyobj_local, + libvirt_constcharPtrWrap("family"), + libvirt_intWrap(local->family), + cleanup); + VIR_PY_DICT_SET_GOTO(pyobj_local, + libvirt_constcharPtrWrap("node"), + libvirt_constcharPtrWrap(local->node), + cleanup); + VIR_PY_DICT_SET_GOTO(pyobj_local, + libvirt_constcharPtrWrap("service"), + libvirt_constcharPtrWrap(local->service), + cleanup);

if ((pyobj_remote = PyDict_New()) == NULL) goto cleanup;

- PyDict_SetItem(pyobj_remote, - libvirt_constcharPtrWrap("family"), - libvirt_intWrap(remote->family)); - PyDict_SetItem(pyobj_remote, - libvirt_constcharPtrWrap("node"), - libvirt_constcharPtrWrap(remote->node)); - PyDict_SetItem(pyobj_remote, - libvirt_constcharPtrWrap("service"), - libvirt_constcharPtrWrap(remote->service)); + VIR_PY_DICT_SET_GOTO(pyobj_remote, + libvirt_constcharPtrWrap("family"), + libvirt_intWrap(remote->family), + cleanup); + VIR_PY_DICT_SET_GOTO(pyobj_remote, + libvirt_constcharPtrWrap("node"), + libvirt_constcharPtrWrap(remote->node), + cleanup); + VIR_PY_DICT_SET_GOTO(pyobj_remote, + libvirt_constcharPtrWrap("service"), + libvirt_constcharPtrWrap(remote->service), + cleanup);

if ((pyobj_subject = PyList_New(subject->nidentity)) == NULL) goto cleanup; @@ -8011,15 +7958,10 @@ libvirt_virDomainGetTime(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; }

- if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) || - PyDict_SetItemString(dict, "seconds", pyobj_seconds) < 0) - goto cleanup; - Py_DECREF(pyobj_seconds); - - if (!(pyobj_nseconds = libvirt_uintWrap(nseconds)) || - PyDict_SetItemString(dict, "nseconds", pyobj_nseconds) < 0) - goto cleanup; - Py_DECREF(pyobj_nseconds); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("seconds"), + libvirt_longlongWrap(seconds), cleanup); + VIR_PY_DICT_SET_GOTO(dict, libvirt_constcharPtrWrap("nseconds"), + libvirt_longlongWrap(nseconds), cleanup);

build issue - 'pyobj_seconds' and 'pyobj_nseconds' no longer necessary The rest seemed OK John

py_retval = dict; dict = NULL; @@ -8148,35 +8090,18 @@ libvirt_virNodeGetFreePages(PyObject *self ATTRIBUTE_UNUSED,

for (i = 0; i < c_retval;) { PyObject *per_node = NULL; - PyObject *node = NULL;

- if (!(per_node = PyDict_New()) || - !(node = libvirt_intWrap(startCell + i/pyobj_pagesize_size))) { - Py_XDECREF(per_node); - Py_XDECREF(node); + if (!(per_node = PyDict_New())) goto cleanup; - }

- if (PyDict_SetItem(pyobj_counts, node, per_node) < 0) { - Py_XDECREF(per_node); - Py_XDECREF(node); - goto cleanup; - } + VIR_PY_DICT_SET_GOTO(pyobj_counts, + libvirt_intWrap(startCell + i/pyobj_pagesize_size), + per_node, cleanup); + + for (j = 0; j < pyobj_pagesize_size; j ++) + VIR_PY_DICT_SET_GOTO(per_node, libvirt_intWrap(pages[j]), + libvirt_intWrap(counts[i + j]), cleanup);

- for (j = 0; j < pyobj_pagesize_size; j ++) { - PyObject *page = NULL; - PyObject *count = NULL; - - if (!(page = libvirt_intWrap(pages[j])) || - !(count = libvirt_intWrap(counts[i + j])) || - PyDict_SetItem(per_node, page, count) < 0) { - Py_XDECREF(page); - Py_XDECREF(count); - Py_XDECREF(per_node); - Py_XDECREF(node); - goto cleanup; - } - } i += pyobj_pagesize_size; }

@@ -8230,30 +8155,24 @@ libvirt_virNetworkGetDHCPLeases(PyObject *self ATTRIBUTE_UNUSED,

VIR_PY_LIST_SET_GOTO(py_retval, i, py_lease, error);

-#define VIR_SET_LEASE_ITEM(NAME, VALUE_OBJ_FUNC) \ - do { \ - PyObject *tmp_val; \ - \ - if (!(tmp_val = VALUE_OBJ_FUNC)) \ - goto error; \ - \ - if (PyDict_SetItemString(py_lease, NAME, tmp_val) < 0) { \ - Py_DECREF(tmp_val); \ - goto error; \ - } \ - } while (0) - - VIR_SET_LEASE_ITEM("iface", libvirt_charPtrWrap(lease->iface)); - VIR_SET_LEASE_ITEM("expirytime", libvirt_longlongWrap(lease->expirytime)); - VIR_SET_LEASE_ITEM("type", libvirt_intWrap(lease->type)); - VIR_SET_LEASE_ITEM("mac", libvirt_charPtrWrap(lease->mac)); - VIR_SET_LEASE_ITEM("ipaddr", libvirt_charPtrWrap(lease->ipaddr)); - VIR_SET_LEASE_ITEM("prefix", libvirt_uintWrap(lease->prefix)); - VIR_SET_LEASE_ITEM("hostname", libvirt_charPtrWrap(lease->hostname)); - VIR_SET_LEASE_ITEM("clientid", libvirt_charPtrWrap(lease->clientid)); - VIR_SET_LEASE_ITEM("iaid", libvirt_charPtrWrap(lease->iaid)); - -#undef VIR_SET_LEASE_ITEM + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("iface"), + libvirt_charPtrWrap(lease->iface), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("expirytime"), + libvirt_longlongWrap(lease->expirytime), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("type"), + libvirt_intWrap(lease->type), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("mac"), + libvirt_charPtrWrap(lease->mac), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("ipaddr"), + libvirt_charPtrWrap(lease->ipaddr), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("prefix"), + libvirt_uintWrap(lease->prefix), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("hostname"), + libvirt_charPtrWrap(lease->hostname), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("clientid"), + libvirt_charPtrWrap(lease->clientid), error); + VIR_PY_DICT_SET_GOTO(py_lease, libvirt_constcharPtrWrap("iaid"), + libvirt_charPtrWrap(lease->iaid), error); }

cleanup: diff --git a/libvirt-utils.c b/libvirt-utils.c index 02a28ac..2bf7519 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -267,22 +267,13 @@ getPyVirTypedParameter(const virTypedParameter *params, }

key = libvirt_constcharPtrWrap(params[i].field); - if (!key || !val) - goto cleanup; - - if (PyDict_SetItem(info, key, val) < 0) { - Py_DECREF(info); - goto cleanup; - }

- Py_DECREF(key); - Py_DECREF(val); + VIR_PY_DICT_SET_GOTO(info, key, val, cleanup); } return info;

cleanup: - Py_XDECREF(key); - Py_XDECREF(val); + Py_DECREF(info); return NULL; }