On 09/24/2015 10:01 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> 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;
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list