[PATCH 00/23] Remove return value from VIR_APPEND_ELEMENT and cleanups
Since this touches all kind of conditionally compiled code you can find the pipeline build results here: https://gitlab.com/pipo.sk/libvirt/-/pipelines/347734973 Peter Krempa (23): virInsertElementsN: Rename 'add' argument virInsertElementsN: Split out actual insertion code util: alloc: Introduce virAppendElement helper util: alloc: Reimplement VIR_APPEND_ELEMENT_(COPY_)INPLACE using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT_COPY using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT_QUIET using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT using virAppendElement util: alloc: Completely replace VIR_APPEND_ELEMENT_QUIET by VIR_APPEND_ELEMENT prlsdkAddDomainVideoInfoCt: Remove pointless cleanup section xenParseXMDisk: Use automatic memory clearing and remove 'ret' variable qemuDomainUSBAddressAddHubs: Refactor cleanup virSecuritySELinuxContextListAppend: Remove unreachable cleanup qemuDomainAttachDeviceConfig: Remove pointless assignment virNWFilterRuleDefToRuleInst: Remove pointless assignment virObjectEventCallbackListAddID: Remove pointless cleanup of 'cb' virNWFilterRuleDefToRuleInst: Restructure code to avoid cleanup virNWFilterIncludeDefToRuleInst: Refactor cleanup qemuProcessSetupHotpluggableVcpus: Use automatic memory freeing virNetServerGetClients: Remove pointless cleanup lxcNetworkParseDataIPs: Automatically free string list virStorageBackendLogicalParseVolExtents: Declare one variable per line virStorageBackendLogicalParseVolExtents: Move 'extents' inside the loop virStorageBackendLogicalParseVolExtents: Remove 'cleanup' and 'ret' src/bhyve/bhyve_parse_command.c | 18 +-- src/conf/capabilities.c | 11 +- src/conf/domain_conf.c | 95 ++++++---------- src/conf/network_conf.c | 9 +- src/conf/node_device_conf.c | 24 ++-- src/conf/nwfilter_conf.c | 12 +- src/conf/object_event.c | 9 +- src/conf/storage_conf.c | 6 +- src/conf/virdomainobjlist.c | 6 +- src/conf/virnwfilterobj.c | 8 +- src/cpu/cpu_arm.c | 6 +- src/cpu/cpu_ppc64.c | 6 +- src/cpu/cpu_x86.c | 21 ++-- src/esx/esx_driver.c | 7 +- src/esx/esx_network_driver.c | 3 +- src/hyperv/hyperv_driver.c | 22 +--- src/hyperv/hyperv_network_driver.c | 3 +- src/libvirt_private.syms | 1 + src/libxl/xen_common.c | 17 +-- src/libxl/xen_xl.c | 21 +--- src/libxl/xen_xm.c | 21 +--- src/logging/log_handler.c | 6 +- src/lxc/lxc_container.c | 5 +- src/lxc/lxc_controller.c | 3 +- src/lxc/lxc_native.c | 12 +- src/network/bridge_driver.c | 4 +- src/node_device/node_device_driver.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 3 +- src/nwfilter/nwfilter_gentech_driver.c | 57 ++++------ src/openvz/openvz_conf.c | 8 +- src/qemu/qemu_block.c | 9 +- src/qemu/qemu_command.c | 14 +-- src/qemu/qemu_domain.c | 6 +- src/qemu/qemu_domain_address.c | 9 +- src/qemu/qemu_driver.c | 17 +-- src/qemu/qemu_firmware.c | 3 +- src/qemu/qemu_hotplug.c | 6 +- src/qemu/qemu_monitor_json.c | 3 +- src/qemu/qemu_namespace.c | 5 +- src/qemu/qemu_process.c | 17 +-- src/remote/remote_daemon_dispatch.c | 56 ++++------ src/rpc/virnetdaemon.c | 4 +- src/rpc/virnetmessage.c | 3 +- src/rpc/virnetserver.c | 12 +- src/security/security_dac.c | 3 +- src/security/security_manager.c | 5 +- src/security/security_selinux.c | 15 +-- src/storage/storage_backend_logical.c | 38 +++---- src/storage/storage_backend_rbd.c | 3 +- src/util/viralloc.c | 129 +++++++++++++++++----- src/util/viralloc.h | 48 ++++---- src/util/virfirewall.c | 8 +- src/util/viriscsi.c | 3 +- src/util/virjson.c | 5 +- src/util/virlog.c | 10 +- src/util/virmdev.c | 7 +- src/util/virnetdev.c | 3 +- src/util/virnvme.c | 7 +- src/util/virpci.c | 13 +-- src/util/virprocess.c | 3 +- src/util/virresctrl.c | 6 +- src/util/virscsi.c | 7 +- src/util/virscsivhost.c | 4 +- src/util/virusb.c | 4 +- src/util/virutil.c | 9 +- src/vbox/vbox_common.c | 13 +-- src/vmx/vmx.c | 3 +- src/vz/vz_sdk.c | 32 ++---- tests/nwfilterxml2firewalltest.c | 13 +-- tests/qemumonitortestutils.c | 11 +- tests/testutils.c | 10 +- tests/virfilewrapper.c | 9 +- tests/virpcimock.c | 10 +- tools/virsh-domain.c | 3 +- tools/vsh-table.c | 9 +- 75 files changed, 404 insertions(+), 630 deletions(-) -- 2.31.1
The idea of @add was that the insersion/appension macros would allow adding more than one element but this feature was never implemented. 'add' is nowadays used as a dummy variable consuming the result of the VIR_TYPEMATCH compile time check. Make it obvious that we don't use 'add' by renaming it to 'typematchDummy', marking it as unused and replacing all occurences where the value was used by literal '1'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viralloc.c | 31 ++++++++++++++++--------------- src/util/viralloc.h | 14 +------------- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index cd7ae9e7d1..6c76da8537 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -148,7 +148,7 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) * @size: the size of one element in bytes * @at: index within array where new elements should be added, -1 for end * @countptr: variable tracking number of elements currently allocated - * @add: number of elements to add + * @typematchDummy: helper variable to consume results of compile time checks * @newelems: pointer to array of one or more new elements to move into * place (the originals will be zeroed out if successful * and if clearOriginal is true) @@ -160,8 +160,8 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) * already* done that. * * Re-allocate an array of *countptr elements, each sizeof(*ptrptr) bytes - * long, to be *countptr+add elements long, then appropriately move - * the elements starting at ptrptr[at] up by add elements, copy the + * long, to be *countptr elements long, then appropriately move + * the elements starting at ptrptr[at] up by 1 elements, copy the * items from newelems into ptrptr[at], then store the address of * allocated memory in *ptrptr and the new size in *countptr. If * newelems is NULL, the new elements at ptrptr[at] are instead filled @@ -173,22 +173,23 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) int virInsertElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, - size_t add, void *newelems, + size_t typematchDummy G_GNUC_UNUSED, + void *newelems, bool clearOriginal, bool inPlace) { if (at == -1) { at = *countptr; } else if (at > *countptr) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("out of bounds index - count %zu at %zu add %zu"), - *countptr, at, add); + _("out of bounds index - count %zu at %zu"), + *countptr, at); return -1; } if (inPlace) { - *countptr += add; + *countptr += 1; } else { - virExpandN(ptrptr, size, countptr, add); + virExpandN(ptrptr, size, countptr, 1); } /* memory was successfully re-allocated. Move up all elements from @@ -197,19 +198,19 @@ virInsertElementsN(void *ptrptr, size_t size, size_t at, * from their original location. Remember that *countptr has * already been updated with new element count! */ - if (at < *countptr - add) { - memmove(*(char**)ptrptr + (size * (at + add)), + if (at < *countptr - 1) { + memmove(*(char**)ptrptr + (size * (at + 1)), *(char**)ptrptr + (size * at), - size * (*countptr - add - at)); + size * (*countptr - 1 - at)); } if (newelems) { - memcpy(*(char**)ptrptr + (size * at), newelems, size * add); + memcpy(*(char**)ptrptr + (size * at), newelems, size); if (clearOriginal) - memset((char*)newelems, 0, size * add); - } else if (inPlace || (at < *countptr - add)) { + memset((char*)newelems, 0, size); + } else if (inPlace || (at < *countptr - 1)) { /* NB: if inPlace, assume memory at the end wasn't initialized */ - memset(*(char**)ptrptr + (size * at), 0, size * add); + memset(*(char**)ptrptr + (size * at), 0, size); } return 0; diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 72e8f13bef..b637bc2ca4 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -43,7 +43,7 @@ void virResizeN(void *ptrptr, size_t size, size_t *alloc, size_t count, size_t d void virShrinkN(void *ptrptr, size_t size, size_t *count, size_t toremove) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); int virInsertElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, - size_t add, void *newelem, + size_t typematchDummy, void *newelem, bool clearOriginal, bool inPlace) G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, @@ -182,12 +182,6 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, * VIR_INSERT_ELEMENT_INPLACE is identical, but assumes any necessary * memory re-allocation has already been done. * - * Both functions need to send "1" as the "add" argument to - * virInsertElementsN (which has the currently-unused capability of - * inserting multiple items at once). We use this to our advantage by - * replacing it with VIR_TYPECHECK(ptr, &newelem) so that we can be - * assured ptr and &newelem are of compatible types. - * * These macros are safe to use on arguments with side effects. * * Returns -1 on failure (with OOM error reported), 0 on success @@ -224,12 +218,6 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, * VIR_APPEND_ELEMENT_*INPLACE are identical, but assume any * necessary memory re-allocation has already been done. * - * VIR_APPEND_ELEMENT_* all need to send "1" as the "add" argument to - * virInsertElementsN (which has the currently-unused capability of - * inserting multiple items at once). We use this to our advantage by - * replacing it with VIR_TYPECHECK(ptr, &newelem) so that we can be - * assured ptr and &newelem are of compatible types. - * * These macros are safe to use on arguments with side effects. * * Returns -1 on failure (with OOM error reported), 0 on success -- 2.31.1
Split out the code doing the movement of the elements and insertion from the range checks. This will help in adding an error-free version for appension. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viralloc.c | 78 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index 6c76da8537..c1211a5f23 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -148,7 +148,6 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) * @size: the size of one element in bytes * @at: index within array where new elements should be added, -1 for end * @countptr: variable tracking number of elements currently allocated - * @typematchDummy: helper variable to consume results of compile time checks * @newelems: pointer to array of one or more new elements to move into * place (the originals will be zeroed out if successful * and if clearOriginal is true) @@ -168,24 +167,17 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) * with zero. at must be between [0,*countptr], except that -1 is * treated the same as *countptr for convenience. * - * Returns -1 on failure, 0 on success + * Aborts on OOM failure. */ -int -virInsertElementsN(void *ptrptr, size_t size, size_t at, - size_t *countptr, - size_t typematchDummy G_GNUC_UNUSED, - void *newelems, - bool clearOriginal, bool inPlace) +static void +virInsertElementInternal(void *ptrptr, + size_t size, + size_t at, + size_t *countptr, + void *newelems, + bool clearOriginal, + bool inPlace) { - if (at == -1) { - at = *countptr; - } else if (at > *countptr) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("out of bounds index - count %zu at %zu"), - *countptr, at); - return -1; - } - if (inPlace) { *countptr += 1; } else { @@ -212,10 +204,62 @@ virInsertElementsN(void *ptrptr, size_t size, size_t at, /* NB: if inPlace, assume memory at the end wasn't initialized */ memset(*(char**)ptrptr + (size * at), 0, size); } +} + + +/** + * virInsertElementsN: + * @ptrptr: pointer to hold address of allocated memory + * @size: the size of one element in bytes + * @at: index within array where new elements should be added, -1 for end + * @countptr: variable tracking number of elements currently allocated + * @typematchDummy: helper variable to consume results of compile time checks + * @newelems: pointer to array of one or more new elements to move into + * place (the originals will be zeroed out if successful + * and if clearOriginal is true) + * @clearOriginal: false if the new item in the array should be copied + * from the original, and the original left intact. + * true if the original should be 0'd out on success. + * @inPlace: false if we should expand the allocated memory before + * moving, true if we should assume someone else *has + * already* done that. + * + * Re-allocate an array of *countptr elements, each sizeof(*ptrptr) bytes + * long, to be *countptr elements long, then appropriately move + * the elements starting at ptrptr[at] up by 1 elements, copy the + * items from newelems into ptrptr[at], then store the address of + * allocated memory in *ptrptr and the new size in *countptr. If + * newelems is NULL, the new elements at ptrptr[at] are instead filled + * with zero. at must be between [0,*countptr], except that -1 is + * treated the same as *countptr for convenience. + * + * Returns -1 on failure, 0 on success + */ +int +virInsertElementsN(void *ptrptr, + size_t size, + size_t at, + size_t *countptr, + size_t typematchDummy G_GNUC_UNUSED, + void *newelems, + bool clearOriginal, + bool inPlace) +{ + if (at == -1) { + at = *countptr; + } else if (at > *countptr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("out of bounds index - count %zu at %zu"), + *countptr, at); + return -1; + } + + virInsertElementInternal(ptrptr, size, at, countptr, newelems, clearOriginal, inPlace); return 0; } + /** * virDeleteElementsN: * @ptrptr: pointer to hold address of allocated memory -- 2.31.1
The new wrapper calls virInsertElementInternal with the appropriate arguments without any checks which are unnecessary for appension. This allows to have no return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/viralloc.c | 30 ++++++++++++++++++++++++++++++ src/util/viralloc.h | 8 ++++++++ 3 files changed, 39 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6961cdb137..97ff884f7e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1760,6 +1760,7 @@ vir_g_strdup_vprintf; # util/viralloc.h +virAppendElement; virDeleteElementsN; virExpandN; virInsertElementsN; diff --git a/src/util/viralloc.c b/src/util/viralloc.c index c1211a5f23..17ce5f3dbe 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -260,6 +260,36 @@ virInsertElementsN(void *ptrptr, } +/** + * virAppendElement: + * @ptrptr: pointer to hold address of allocated memory + * @size: the size of one element in bytes + * @countptr: variable tracking number of elements currently allocated + * @typematchDummy: helper variable to consume results of compile time checks + * @newelem: pointer to a new element to append to @ptrptr + * (the original will be zeroed out if clearOriginal is true) + * @clearOriginal: false if the new item in the array should be copied + * from the original, and the original left intact. + * true if the original should be 0'd out on success. + * @inPlace: false if we should expand the allocated memory before + * moving, true if we should assume someone else *has + * already* done that. + * + * Re-allocate @ptrptr to fit an extra element and place @newelem at the end. + */ +void +virAppendElement(void *ptrptr, + size_t size, + size_t *countptr, + size_t typematchDummy G_GNUC_UNUSED, + void *newelem, + bool clearOriginal, + bool inPlace) +{ + virInsertElementInternal(ptrptr, size, *countptr, countptr, newelem, clearOriginal, inPlace); +} + + /** * virDeleteElementsN: * @ptrptr: pointer to hold address of allocated memory diff --git a/src/util/viralloc.h b/src/util/viralloc.h index b637bc2ca4..7669b12e89 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -46,6 +46,14 @@ int virInsertElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, size_t typematchDummy, void *newelem, bool clearOriginal, bool inPlace) G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virAppendElement(void *ptrptr, + size_t size, + size_t *countptr, + size_t typematchDummy, + void *newelem, + bool clearOriginal, + bool inPlace) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, size_t toremove, bool inPlace) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); -- 2.31.1
VIR_APPEND_ELEMENT_INPLACE and VIR_APPEND_ELEMENT_COPY_INPLACE already ignore the return value from 'virInsertElementsN' which allows a trivial conversion to virAppendElement without the need for 'ignore_value'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viralloc.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 7669b12e89..7397ee3771 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -237,13 +237,13 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) #define VIR_APPEND_ELEMENT_INPLACE(ptr, count, newelem) \ - ignore_value(virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), \ - &(newelem), true, true)) + virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ + VIR_TYPEMATCH(ptr, &(newelem)), \ + &(newelem), true, true) #define VIR_APPEND_ELEMENT_COPY_INPLACE(ptr, count, newelem) \ - ignore_value(virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), \ - &(newelem), false, true)) + virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ + VIR_TYPEMATCH(ptr, &(newelem)), \ + &(newelem), false, true) /* Quiet version of macros above */ #define VIR_APPEND_ELEMENT_QUIET(ptr, count, newelem) \ -- 2.31.1
Use virAppendElement instead of virInsertElementsN to implement VIR_APPEND_ELEMENT_COPY which allows us to remove error handling as the only relevant errors were removed when switching to aborting memory allocation functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 29 +++++------------------------ src/conf/nwfilter_conf.c | 12 +++--------- src/conf/virnwfilterobj.c | 8 ++------ src/cpu/cpu_x86.c | 12 ++++-------- src/logging/log_handler.c | 6 ++---- src/openvz/openvz_conf.c | 5 ++--- src/qemu/qemu_domain.c | 6 +----- src/qemu/qemu_hotplug.c | 6 ++---- src/util/viralloc.h | 4 ++-- src/util/virfirewall.c | 8 ++------ tests/nwfilterxml2firewalltest.c | 7 +------ 11 files changed, 26 insertions(+), 77 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 09da4ab952..8d13e2de8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16573,8 +16573,7 @@ virDomainMemoryInsert(virDomainDef *def, return -1; } - if (VIR_APPEND_ELEMENT_COPY(def->mems, def->nmems, mem) < 0) - return -1; + VIR_APPEND_ELEMENT_COPY(def->mems, def->nmems, mem); virDomainDefSetMemoryTotal(def, memory + mem->size); @@ -17141,10 +17140,7 @@ virDomainDefAddController(virDomainDef *def, int type, int idx, int model) cont->idx = idx; cont->model = model; - if (VIR_APPEND_ELEMENT_COPY(def->controllers, def->ncontrollers, cont) < 0) { - VIR_FREE(cont); - return NULL; - } + VIR_APPEND_ELEMENT_COPY(def->controllers, def->ncontrollers, cont); return cont; } @@ -22780,15 +22776,9 @@ virDomainIOThreadIDAdd(virDomainDef *def, iothrid->iothread_id = iothread_id; - if (VIR_APPEND_ELEMENT_COPY(def->iothreadids, def->niothreadids, - iothrid) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(def->iothreadids, def->niothreadids, iothrid); return iothrid; - - error: - virDomainIOThreadIDDefFree(iothrid); - return NULL; } @@ -29304,13 +29294,9 @@ virDomainGraphicsListenAppendAddress(virDomainGraphicsDef *def, glisten.address = g_strdup(address); - if (VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, glisten) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, glisten); return 0; - error: - VIR_FREE(glisten.address); - return -1; } @@ -29326,14 +29312,9 @@ virDomainGraphicsListenAppendSocket(virDomainGraphicsDef *def, glisten.socket = g_strdup(socketPath); - if (VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, glisten) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(def->listens, def->nListens, glisten); return 0; - - error: - VIR_FREE(glisten.socket); - return -1; } diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 6fced96865..fc81fd97ea 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -394,11 +394,9 @@ virNWFilterRuleDefAddString(virNWFilterRuleDef *nwf, const char *string, size_t maxstrlen) { - char *tmp; + char *tmp = g_strndup(string, maxstrlen); - tmp = g_strndup(string, maxstrlen); - if (VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0) - VIR_FREE(tmp); + VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp); return tmp; } @@ -2696,11 +2694,7 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) } if (entry->rule || entry->include) { - if (VIR_APPEND_ELEMENT_COPY(ret->filterEntries, - ret->nentries, entry) < 0) { - virNWFilterEntryFree(entry); - goto cleanup; - } + VIR_APPEND_ELEMENT_COPY(ret->filterEntries, ret->nentries, entry); } else { virNWFilterEntryFree(entry); } diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index b64fdb9670..c3b2eb048c 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -371,12 +371,8 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfilters, if (!(obj = virNWFilterObjNew())) return NULL; - if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, - nwfilters->count, obj) < 0) { - virNWFilterObjUnlock(obj); - virNWFilterObjFree(obj); - return NULL; - } + VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj); + obj->def = def; return obj; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index a4792c21da..0238cef8a3 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -514,9 +514,8 @@ virCPUx86DataAddItem(virCPUx86Data *data, if ((existing = virCPUx86DataGet(data, item))) { virCPUx86DataItemSetBits(existing, item); } else { - if (VIR_APPEND_ELEMENT_COPY(data->items, data->len, - *((virCPUx86DataItem *)item)) < 0) - return -1; + VIR_APPEND_ELEMENT_COPY(data->items, data->len, + *((virCPUx86DataItem *)item)); qsort(data->items, data->len, sizeof(virCPUx86DataItem), virCPUx86DataSorter); @@ -1143,11 +1142,8 @@ x86FeatureParse(xmlXPathContextPtr ctxt, return -1; } - if (!feature->migratable && - VIR_APPEND_ELEMENT_COPY(map->migrate_blockers, - map->nblockers, - feature) < 0) - return -1; + if (!feature->migratable) + VIR_APPEND_ELEMENT_COPY(map->migrate_blockers, map->nblockers, feature); if (VIR_APPEND_ELEMENT(map->features, map->nfeatures, feature) < 0) return -1; diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index dde3506fe9..5c3df37415 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -318,8 +318,7 @@ virLogHandlerNewPostExecRestart(virJSONValue *object, if (!(file = virLogHandlerLogFilePostExecRestart(handler, child))) goto error; - if (VIR_APPEND_ELEMENT_COPY(handler->files, handler->nfiles, file) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(handler->files, handler->nfiles, file); if ((file->watch = virEventAddHandle(file->pipefd, VIR_EVENT_HANDLE_READABLE, @@ -401,8 +400,7 @@ virLogHandlerDomainOpenLogFile(virLogHandler *handler, DEFAULT_MODE)) == NULL) goto error; - if (VIR_APPEND_ELEMENT_COPY(handler->files, handler->nfiles, file) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(handler->files, handler->nfiles, file); if ((file->watch = virEventAddHandle(file->pipefd, VIR_EVENT_HANDLE_READABLE, diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 2a794801ae..07c2cddd92 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -206,8 +206,7 @@ openvzReadNetworkConf(virDomainDef *def, if (virDomainNetAppendIPAddress(net, token, AF_UNSPEC, 0) < 0) goto error; - if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net); token = strtok_r(NULL, " ", &saveptr); } @@ -275,7 +274,7 @@ openvzReadNetworkConf(virDomainDef *def, } } - ignore_value(VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net)); + VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net); } } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7e2efc8168..004bfb5d83 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4001,11 +4001,7 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, if (j == def->npanics) { virDomainPanicDef *panic = g_new0(virDomainPanicDef, 1); - if (VIR_APPEND_ELEMENT_COPY(def->panics, - def->npanics, panic) < 0) { - VIR_FREE(panic); - return -1; - } + VIR_APPEND_ELEMENT_COPY(def->panics, def->npanics, panic); } } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6743a8a742..c00e8a7852 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1236,8 +1236,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, virDomainNetGetActualHostdev(net)) < 0) { goto cleanup; } - if (VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net) < 0) - goto cleanup; + VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net); /* the rest of the setup doesn't apply to hostdev interfaces, so * we can skip straight to the cleanup (nothing there applies to @@ -1272,8 +1271,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, * locked memory limit). This means we will need to remove it if * there is a failure. */ - if (VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net) < 0) - goto cleanup; + VIR_APPEND_ELEMENT_COPY(vm->def->nets, vm->def->nnets, net); switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 7397ee3771..70cd721bc7 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -234,8 +234,8 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) #define VIR_APPEND_ELEMENT_COPY(ptr, count, newelem) \ - virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) + virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) #define VIR_APPEND_ELEMENT_INPLACE(ptr, count, newelem) \ virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), \ diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index e79fe52ac8..9594192a7b 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -313,13 +313,9 @@ virFirewallAddRuleFullV(virFirewall *firewall, ADD_ARG(rule, str); if (group->addingRollback) { - ignore_value(VIR_APPEND_ELEMENT_COPY(group->rollback, - group->nrollback, - rule)); + VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, rule); } else { - ignore_value(VIR_APPEND_ELEMENT_COPY(group->action, - group->naction, - rule)); + VIR_APPEND_ELEMENT_COPY(group->action, group->naction, rule); } diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 33ba8b9932..8c82efa060 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -269,12 +269,7 @@ virNWFilterDefToInst(const char *xml, if (!def) return -1; - if (VIR_APPEND_ELEMENT_COPY(inst->filters, - inst->nfilters, - def) < 0) { - virNWFilterDefFree(def); - goto cleanup; - } + VIR_APPEND_ELEMENT_COPY(inst->filters, inst->nfilters, def); for (i = 0; i < def->nentries; i++) { if (def->filterEntries[i]->rule) { -- 2.31.1
On 8/4/21 1:02 PM, Peter Krempa wrote:
Use virAppendElement instead of virInsertElementsN to implement VIR_APPEND_ELEMENT_COPY which allows us to remove error handling as the only relevant errors were removed when switching to aborting memory allocation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 29 +++++------------------------ src/conf/nwfilter_conf.c | 12 +++--------- src/conf/virnwfilterobj.c | 8 ++------ src/cpu/cpu_x86.c | 12 ++++-------- src/logging/log_handler.c | 6 ++---- src/openvz/openvz_conf.c | 5 ++--- src/qemu/qemu_domain.c | 6 +----- src/qemu/qemu_hotplug.c | 6 ++---- src/util/viralloc.h | 4 ++-- src/util/virfirewall.c | 8 ++------ tests/nwfilterxml2firewalltest.c | 7 +------ 11 files changed, 26 insertions(+), 77 deletions(-)
Squash this in please: diff --git i/src/vz/vz_sdk.c w/src/vz/vz_sdk.c index e09950812d..8a027a50b8 100644 --- i/src/vz/vz_sdk.c +++ w/src/vz/vz_sdk.c @@ -578,8 +578,7 @@ prlsdkAddDomainVideoInfoVm(PRL_HANDLE sdkdom, virDomainDef *def) video = g_new0(virDomainVideoDef, 1); accel = g_new0(virDomainVideoAccelDef, 1); - if (VIR_APPEND_ELEMENT_COPY(def->videos, def->nvideos, video) < 0) - goto error; + VIR_APPEND_ELEMENT_COPY(def->videos, def->nvideos, video); video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; video->vram = videoRam << 10; /* from mbibytes to kbibytes */ Michal
For now it was an alias to VIR_APPEND_ELEMENT. Use virAppendElement directly until VIR_APPEND_ELEMENT is refactored too and we'll be able to get rid of VIR_APPEND_ELEMENT_QUIET completely. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viralloc.h | 3 ++- tests/virfilewrapper.c | 9 ++------- tests/virpcimock.c | 10 +++------- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 70cd721bc7..c5ca8e9929 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -247,7 +247,8 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, /* Quiet version of macros above */ #define VIR_APPEND_ELEMENT_QUIET(ptr, count, newelem) \ - VIR_APPEND_ELEMENT(ptr, count, newelem) + virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) /** * VIR_DELETE_ELEMENT: diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c index 30c63ecf56..3085a190fb 100644 --- a/tests/virfilewrapper.c +++ b/tests/virfilewrapper.c @@ -78,13 +78,8 @@ virFileWrapperAddPrefix(const char *prefix, init_syms(); - if (VIR_APPEND_ELEMENT_QUIET(prefixes, nprefixes, prefix) < 0 || - VIR_APPEND_ELEMENT_QUIET(overrides, noverrides, override) < 0) { - VIR_FREE(prefixes); - VIR_FREE(overrides); - fprintf(stderr, "Unable to add path override for '%s'\n", prefix); - abort(); - } + VIR_APPEND_ELEMENT_QUIET(prefixes, nprefixes, prefix); + VIR_APPEND_ELEMENT_QUIET(overrides, noverrides, override); } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index be513ad918..de39fc65a4 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -428,9 +428,7 @@ pci_device_create_iommu(const struct pciDevice *dev, iommuGroup->iommu = dev->iommuGroup; iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default */ - if (VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, - iommuGroup) < 0) - ABORT_OOM(); + VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, iommuGroup); } @@ -544,8 +542,7 @@ pci_device_new_from_stub(const struct pciDevice *data) if (pci_device_autobind(dev) < 0) ABORT("Unable to bind: %s", devid); - if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0) - ABORT_OOM(); + VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev); } static struct pciDevice * @@ -716,8 +713,7 @@ pci_driver_new(const char *name, ...) make_file(driverpath, "bind", NULL, -1); make_file(driverpath, "unbind", NULL, -1); - if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver) < 0) - ABORT_OOM(); + VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver); } static struct pciDriver * -- 2.31.1
Use virAppendElement instead of virInsertElementsN to implement VIR_APPEND_ELEMENT which allows us to remove error handling as the only relevant errors were removed when switching to aborting memory allocation functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_parse_command.c | 18 ++----- src/conf/capabilities.c | 11 +--- src/conf/domain_conf.c | 66 +++++++++-------------- src/conf/network_conf.c | 9 ++-- src/conf/node_device_conf.c | 24 ++++----- src/conf/object_event.c | 8 ++- src/conf/storage_conf.c | 6 +-- src/conf/virdomainobjlist.c | 6 +-- src/cpu/cpu_arm.c | 6 +-- src/cpu/cpu_ppc64.c | 6 +-- src/cpu/cpu_x86.c | 9 ++-- src/esx/esx_driver.c | 7 +-- src/esx/esx_network_driver.c | 3 +- src/hyperv/hyperv_driver.c | 22 +++----- src/hyperv/hyperv_network_driver.c | 3 +- src/libxl/xen_common.c | 17 ++---- src/libxl/xen_xl.c | 21 ++------ src/libxl/xen_xm.c | 12 +---- src/lxc/lxc_container.c | 5 +- src/lxc/lxc_controller.c | 3 +- src/lxc/lxc_native.c | 6 +-- src/network/bridge_driver.c | 4 +- src/node_device/node_device_driver.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 3 +- src/nwfilter/nwfilter_gentech_driver.c | 10 +--- src/openvz/openvz_conf.c | 3 +- src/qemu/qemu_block.c | 9 ++-- src/qemu/qemu_command.c | 14 +++-- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_driver.c | 15 ++---- src/qemu/qemu_firmware.c | 3 +- src/qemu/qemu_monitor_json.c | 3 +- src/qemu/qemu_namespace.c | 5 +- src/qemu/qemu_process.c | 3 +- src/remote/remote_daemon_dispatch.c | 56 +++++++++---------- src/rpc/virnetdaemon.c | 4 +- src/rpc/virnetmessage.c | 3 +- src/rpc/virnetserver.c | 6 +-- src/security/security_dac.c | 3 +- src/security/security_manager.c | 5 +- src/security/security_selinux.c | 10 +--- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/viralloc.h | 6 +-- src/util/viriscsi.c | 3 +- src/util/virjson.c | 5 +- src/util/virlog.c | 10 +--- src/util/virmdev.c | 7 +-- src/util/virnetdev.c | 3 +- src/util/virnvme.c | 7 ++- src/util/virpci.c | 13 +++-- src/util/virprocess.c | 3 +- src/util/virresctrl.c | 6 +-- src/util/virscsi.c | 7 +-- src/util/virscsivhost.c | 4 +- src/util/virusb.c | 4 +- src/util/virutil.c | 9 +--- src/vbox/vbox_common.c | 13 ++--- src/vmx/vmx.c | 3 +- src/vz/vz_sdk.c | 23 +++----- tests/nwfilterxml2firewalltest.c | 5 +- tests/qemumonitortestutils.c | 11 +--- tests/testutils.c | 10 ++-- tools/virsh-domain.c | 3 +- tools/vsh-table.c | 9 ++-- 65 files changed, 206 insertions(+), 399 deletions(-) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 9b97c262ec..04fd88872c 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -328,10 +328,7 @@ bhyveParseBhyveLPCArg(virDomainDef *def, break; } - if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { - virDomainChrDefFree(chr); - goto error; - } + VIR_APPEND_ELEMENT(def->serials, def->nserials, chr); } VIR_FREE(type); @@ -459,8 +456,7 @@ bhyveParsePCIDisk(virDomainDef *def, disk->dst[2] = 'a' + idx; - if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) - goto error; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); return 0; @@ -536,8 +532,7 @@ bhyveParsePCINet(virDomainDef *def, if (!mac) virDomainNetGenerateMAC(xmlopt, &net->mac); - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) - goto error; + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); return 0; error: @@ -642,11 +637,8 @@ bhyveParsePCIFbuf(virDomainDef *def, } cleanup: - if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0) - goto error; - - if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0) - goto error; + VIR_APPEND_ELEMENT(def->videos, def->nvideos, video); + VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics); return 0; diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 15c76d791a..72d4146ac3 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2124,10 +2124,7 @@ virCapabilitiesInitResctrlMemory(virCaps *caps) node->id = bank->id; node->cpus = virBitmapNewCopy(bank->cpus); - if (VIR_APPEND_ELEMENT(caps->host.memBW.nodes, - caps->host.memBW.nnodes, node) < 0) { - goto cleanup; - } + VIR_APPEND_ELEMENT(caps->host.memBW.nodes, caps->host.memBW.nnodes, node); } virCapsHostMemBWNodeFree(node); node = NULL; @@ -2250,11 +2247,7 @@ virCapabilitiesInitCaches(virCaps *caps) &bank->controls) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(caps->host.cache.banks, - caps->host.cache.nbanks, - bank) < 0) { - goto cleanup; - } + VIR_APPEND_ELEMENT(caps->host.cache.banks, caps->host.cache.nbanks, bank); } virCapsHostCacheBankFree(bank); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d13e2de8e..3415e28b95 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4820,10 +4820,7 @@ virDomainDefAddConsoleCompat(virDomainDef *def) /* create the serial port definition from the console definition */ if (def->nserials == 0) { - if (VIR_APPEND_ELEMENT(def->serials, - def->nserials, - def->consoles[0]) < 0) - return -1; + VIR_APPEND_ELEMENT(def->serials, def->nserials, def->consoles[0]); /* modify it to be a serial port */ def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; @@ -7475,9 +7472,10 @@ virDomainNetIPInfoParseXML(const char *source, goto cleanup; for (i = 0; i < nnodes; i++) { - if (!(ip = virDomainNetIPParseXML(nodes[i])) || - VIR_APPEND_ELEMENT(def->ips, def->nips, ip) < 0) + if (!(ip = virDomainNetIPParseXML(nodes[i]))) goto cleanup; + + VIR_APPEND_ELEMENT(def->ips, def->nips, ip); } VIR_FREE(nodes); @@ -7485,9 +7483,10 @@ virDomainNetIPInfoParseXML(const char *source, goto cleanup; for (i = 0; i < nnodes; i++) { - if (!(route = virNetDevIPRouteParseXML(source, nodes[i], ctxt)) || - VIR_APPEND_ELEMENT(def->routes, def->nroutes, route) < 0) + if (!(route = virNetDevIPRouteParseXML(source, nodes[i], ctxt))) goto cleanup; + + VIR_APPEND_ELEMENT(def->routes, def->nroutes, route); } ret = 0; @@ -10120,8 +10119,7 @@ virDomainNetAppendIPAddress(virDomainNetDef *def, goto error; ipDef->prefix = prefix; - if (VIR_APPEND_ELEMENT(def->guestIP.ips, def->guestIP.nips, ipDef) < 0) - goto error; + VIR_APPEND_ELEMENT(def->guestIP.ips, def->guestIP.nips, ipDef); return 0; @@ -12464,8 +12462,7 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDef *def, /* If no <listen/> element was found add a new one created by parsing * <graphics/> element. */ if (def->nListens == 0) { - if (VIR_APPEND_ELEMENT(def->listens, def->nListens, newListen) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->listens, def->nListens, newListen); } else { virDomainGraphicsListenDef *glisten = &def->listens[0]; @@ -15216,7 +15213,9 @@ virDomainChrTargetTypeToString(int deviceType, int virDomainHostdevInsert(virDomainDef *def, virDomainHostdevDef *hostdev) { - return VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); + + return 0; } virDomainHostdevDef * @@ -15631,13 +15630,7 @@ int virDomainNetInsert(virDomainDef *def, virDomainNetDef *net) virDomainHostdevInsert(def, &net->data.hostdev.def) < 0) return -1; - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) { - /* virDomainHostdevInsert just appends new hostdevs, so we are sure - * that the hostdev we've added a few lines above is at the end of - * array. Although, devices are indexed from zero ... */ - virDomainHostdevRemove(def, def->nhostdevs - 1); - return -1; - } + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); return 0; } @@ -15835,8 +15828,7 @@ virDomainNetUpdate(virDomainDef *def, } } else if (newhostdev) { /* add newhostdev to end of def->hostdevs */ - if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, newhostdev) < 0) - return -1; + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, newhostdev); } def->nets[netidx] = newnet; @@ -15952,8 +15944,7 @@ virDomainNetARPInterfaces(virDomainDef *def, iface->addrs->addr = g_strdup(entry.ipaddr); - if (VIR_APPEND_ELEMENT(ifaces_ret, ifaces_count, iface) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(ifaces_ret, ifaces_count, iface); } } } @@ -16649,7 +16640,9 @@ int virDomainShmemDefInsert(virDomainDef *def, virDomainShmemDef *shmem) { - return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); + VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); + + return 0; } @@ -17237,10 +17230,7 @@ virDomainDefMaybeAddInput(virDomainDef *def, input->type = type; input->bus = bus; - if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) { - VIR_FREE(input); - return -1; - } + VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input); return 0; } @@ -18581,10 +18571,7 @@ virDomainResctrlMonDefParse(virDomainDef *def, if (virResctrlMonitorSetID(domresmon->instance, id) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(resctrl->monitors, - resctrl->nmonitors, - domresmon) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(resctrl->monitors, resctrl->nmonitors, domresmon); VIR_FREE(id); VIR_FREE(tmp); @@ -18697,8 +18684,7 @@ virDomainCachetuneDefParse(virDomainDef *def, goto cleanup; } - if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, resctrl) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, resctrl); ret = 0; cleanup: @@ -19063,8 +19049,7 @@ virDomainMemorytuneDefParse(virDomainDef *def, * only append the new @newresctrl object to domain if any of them is * not zero. */ if (newresctrl && (nmons || n)) { - if (VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, newresctrl) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, newresctrl); } ret = 0; @@ -22728,8 +22713,7 @@ virDomainDefAddImplicitVideo(virDomainDef *def, virDomainXMLOption *xmlopt) if (!(video = virDomainVideoDefNew(xmlopt))) return -1; video->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT; - if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0) - return -1; + VIR_APPEND_ELEMENT(def->videos, def->nvideos, video); return 0; } @@ -28694,7 +28678,9 @@ virDiskNameToBusDeviceIndex(virDomainDiskDef *disk, int virDomainFSInsert(virDomainDef *def, virDomainFSDef *fs) { - return VIR_APPEND_ELEMENT(def->fss, def->nfss, fs); + VIR_APPEND_ELEMENT(def->fss, def->nfss, fs); + + return 0; } virDomainFSDef * diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index fe9919d4ce..e34ac52a68 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -643,8 +643,7 @@ virNetworkDHCPDefParseXML(const char *networkName, if (virNetworkDHCPRangeDefParseXML(networkName, def, cur, &range) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(def->ranges, def->nranges, range) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->ranges, def->nranges, range); } else if (cur->type == XML_ELEMENT_NODE && virXMLNodeNameEqual(cur, "host")) { @@ -652,8 +651,7 @@ virNetworkDHCPDefParseXML(const char *networkName, if (virNetworkDHCPHostDefParseXML(networkName, def, cur, &host, false) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(def->hosts, def->nhosts, host) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->hosts, def->nhosts, host); } else if (VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) && cur->type == XML_ELEMENT_NODE && virXMLNodeNameEqual(cur, "bootp")) { @@ -726,8 +724,7 @@ virNetworkDNSHostDefParseXML(const char *networkName, networkName); goto error; } - if (VIR_APPEND_ELEMENT(def->names, def->nnames, name) < 0) - goto error; + VIR_APPEND_ELEMENT(def->names, def->nnames, name); } } cur = cur->next; diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 332b12f997..d12d97f077 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -849,9 +849,7 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, type->name = virXPathString("string(./name)", ctxt); - if (VIR_APPEND_ELEMENT(*mdev_types, - *nmdev_types, type) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*mdev_types, *nmdev_types, type); } ret = 0; @@ -1574,10 +1572,9 @@ virNodeDevCapPCIDevIommuGroupParseXML(xmlXPathContextPtr ctxt, if (virPCIDeviceAddressParseXML(addrNodes[i], pciAddr) < 0) return -1; - if (VIR_APPEND_ELEMENT(pci_dev->iommuGroupDevices, - pci_dev->nIommuGroupDevices, - pciAddr) < 0) - return -1; + VIR_APPEND_ELEMENT(pci_dev->iommuGroupDevices, + pci_dev->nIommuGroupDevices, + pciAddr); } return 0; @@ -1684,10 +1681,9 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt, if (virPCIDeviceAddressParseXML(addresses[i], addr) < 0) return -1; - if (VIR_APPEND_ELEMENT(pci_dev->virtual_functions, - pci_dev->num_virtual_functions, - addr) < 0) - return -1; + VIR_APPEND_ELEMENT(pci_dev->virtual_functions, + pci_dev->num_virtual_functions, + addr); } pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; @@ -1896,9 +1892,9 @@ virNodeDevCapMdevAttributeParseXML(xmlXPathContextPtr ctxt, return -1; } - return VIR_APPEND_ELEMENT(mdev->attributes, - mdev->nattributes, - attr); + VIR_APPEND_ELEMENT(mdev->attributes, mdev->nattributes, attr); + + return 0; } static int diff --git a/src/conf/object_event.c b/src/conf/object_event.c index 4b571facc8..866d438f89 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -440,8 +440,7 @@ virObjectEventCallbackListAddID(virConnectPtr conn, cb->filter_opaque = filter_opaque; cb->legacy = legacy; - if (VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, cb) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, cb); /* When additional filtering is being done, every client callback * is matched to exactly one server callback. */ @@ -454,7 +453,6 @@ virObjectEventCallbackListAddID(virConnectPtr conn, ret++; } - cleanup: virObjectEventCallbackFree(cb); return ret; } @@ -642,8 +640,8 @@ virObjectEventQueuePush(virObjectEventQueue *evtQueue, if (!evtQueue) return -1; - if (VIR_APPEND_ELEMENT(evtQueue->events, evtQueue->count, event) < 0) - return -1; + VIR_APPEND_ELEMENT(evtQueue->events, evtQueue->count, event); + return 0; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 2aa9a3d8f9..ad2eb66417 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -609,11 +609,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, return -1; } - if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) { - virStoragePoolSourceDeviceClear(&dev); - return -1; - } - + VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev); } sourcedir = virXPathString("string(./dir/@path)", ctxt); diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 43d09692a9..5c40d697dc 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -1027,11 +1027,7 @@ virDomainObjListConvert(virDomainObjList *domlist, virObjectRef(vm); - if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0) { - virObjectRWUnlock(domlist); - virObjectUnref(vm); - goto error; - } + VIR_APPEND_ELEMENT(*vms, *nvms, vm); } virObjectRWUnlock(domlist); diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 1e5ac558b2..09ade1d422 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -282,8 +282,7 @@ virCPUarmVendorParse(xmlXPathContextPtr ctxt, return -1; } - if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) - return -1; + VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor); return 0; } @@ -368,8 +367,7 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt, return -1; } - if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) - return -1; + VIR_APPEND_ELEMENT(map->models, map->nmodels, model); return 0; } diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index c68a0f3656..4909f61ff1 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -285,8 +285,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, return -1; } - if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) - return -1; + VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor); return 0; } @@ -361,8 +360,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, model->data.pvr[i].mask = pvr; } - if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) - return -1; + VIR_APPEND_ELEMENT(map->models, map->nmodels, model); return 0; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 0238cef8a3..1b829e5658 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -937,8 +937,7 @@ x86VendorParse(xmlXPathContextPtr ctxt, if (virCPUx86VendorToData(string, &vendor->data) < 0) return -1; - if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) - return -1; + VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor); return 0; } @@ -1145,8 +1144,7 @@ x86FeatureParse(xmlXPathContextPtr ctxt, if (!feature->migratable) VIR_APPEND_ELEMENT_COPY(map->migrate_blockers, map->nblockers, feature); - if (VIR_APPEND_ELEMENT(map->features, map->nfeatures, feature) < 0) - return -1; + VIR_APPEND_ELEMENT(map->features, map->nfeatures, feature); return 0; } @@ -1679,8 +1677,7 @@ x86ModelParse(xmlXPathContextPtr ctxt, if (x86ModelParseFeatures(model, ctxt, map) < 0) return -1; - if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) - return -1; + VIR_APPEND_ELEMENT(map->models, map->nmodels, model); return 0; } diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 75eee49775..5d9687733f 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -5166,8 +5166,7 @@ esxDomainInterfaceAddresses(virDomainPtr domain, else if (ret == 0) continue; - if (VIR_APPEND_ELEMENT(iface->addrs, addrs_count, ip_addr) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(iface->addrs, addrs_count, ip_addr); } } else { esxVI_String *str; @@ -5182,9 +5181,7 @@ esxDomainInterfaceAddresses(virDomainPtr domain, else if (ret == 0) continue; - if (VIR_APPEND_ELEMENT(iface->addrs, addrs_count, ip_addr) < 0) - goto cleanup; - + VIR_APPEND_ELEMENT(iface->addrs, addrs_count, ip_addr); } } diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 0a0a3dda89..a87ec6a377 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -900,8 +900,7 @@ esxConnectListAllNetworks(virConnectPtr conn, virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch); if (!net) goto cleanup; - if (VIR_APPEND_ELEMENT(*nets, count, net) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*nets, count, net); } else { ++count; } diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index ff20d5548b..8996394291 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1203,8 +1203,7 @@ hypervDomainDefAppendController(virDomainDef *def, controller->idx = idx; - if (VIR_APPEND_ELEMENT(def->controllers, def->ncontrollers, controller) < 0) - return -1; + VIR_APPEND_ELEMENT(def->controllers, def->ncontrollers, controller); return 0; } @@ -1270,8 +1269,7 @@ hypervDomainDefAppendDisk(virDomainDef *def, disk->info.addr.drive.target = 0; disk->info.addr.drive.unit = addr; - if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) - return -1; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); return 0; } @@ -1283,8 +1281,7 @@ hypervDomainDefParseFloppyStorageExtent(virDomainDef *def, virDomainDiskDef *dis disk->bus = VIR_DOMAIN_DISK_BUS_FDC; disk->dst = g_strdup("fda"); - if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) - return -1; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); return 0; } @@ -1456,8 +1453,7 @@ hypervDomainDefParsePhysicalDisk(hypervPrivate *priv, disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); result = 0; @@ -1579,10 +1575,7 @@ hypervDomainDefParseSerial(virDomainDef *def, Msvm_ResourceAllocationSettingData serial->source->data.file.path = g_strdup(srcPath); - if (VIR_APPEND_ELEMENT(def->serials, def->nserials, serial) < 0) { - virDomainChrDefFree(serial); - return -1; - } + VIR_APPEND_ELEMENT(def->serials, def->nserials, serial); } return 0; @@ -1661,10 +1654,7 @@ hypervDomainDefParseEthernetAdapter(virDomainDef *def, /* get bridge name */ ndef->data.bridge.brname = g_strdup(vSwitch->data->Name); - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, ndef) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not append definition to domain")); - return -1; - } + VIR_APPEND_ELEMENT(def->nets, def->nnets, ndef); return 0; } diff --git a/src/hyperv/hyperv_network_driver.c b/src/hyperv/hyperv_network_driver.c index 93ef01c9aa..a794a681b5 100644 --- a/src/hyperv/hyperv_network_driver.c +++ b/src/hyperv/hyperv_network_driver.c @@ -134,8 +134,7 @@ hypervConnectListAllNetworks(virConnectPtr conn, virNetworkPtr net = hypervMsvmVirtualSwitchToNetwork(conn, entry); if (!net) goto cleanup; - if (VIR_APPEND_ELEMENT(*nets, count, net) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*nets, count, net); } else { ++count; } diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 284d63a6aa..87f090f979 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -495,10 +495,7 @@ xenParsePCIList(virConf *conf, virDomainDef *def) if (!(hostdev = xenParsePCI(entry))) return -1; - if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { - virDomainHostdevDefFree(hostdev); - return -1; - } + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); } return 0; @@ -983,8 +980,7 @@ xenParseCharDev(virConf *conf, virDomainDef *def, const char *nativeFormat) goto cleanup; chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; chr->target.port = portnum; - if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->serials, def->nserials, chr); } } else { g_autofree char *serial = NULL; @@ -1265,7 +1261,6 @@ xenParseVifList(virConf *conf, virDomainDef *def, const char *vif_typename) for (list = list->list; list; list = list->next) { virDomainNetDef *net = NULL; - int rc; if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) continue; @@ -1273,11 +1268,7 @@ xenParseVifList(virConf *conf, virDomainDef *def, const char *vif_typename) if (!(net = xenParseVif(list->str, vif_typename))) return -1; - rc = VIR_APPEND_ELEMENT(def->nets, def->nnets, net); - if (rc < 0) { - virDomainNetDefFree(net); - return -1; - } + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); } return 0; @@ -1339,7 +1330,7 @@ xenParseSxprSound(virDomainDef *def, snddef = g_new0(virDomainSoundDef, 1); snddef->model = model; - ignore_value(VIR_APPEND_ELEMENT(def->sounds, def->nsounds, snddef)); + VIR_APPEND_ELEMENT(def->sounds, def->nsounds, snddef); if (!next) break; diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index c0905b08d8..5d4a273ff1 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -796,8 +796,7 @@ xenParseXLDisk(virConf *conf, virDomainDef *def) else disk->bus = VIR_DOMAIN_DISK_BUS_IDE; - if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) - goto fail; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); libxl_device_disk_dispose(libxldisk); @@ -855,10 +854,7 @@ xenParseXLInputDevs(virConf *conf, virDomainDef *def) input->type = VIR_DOMAIN_INPUT_TYPE_TABLET; else if (STREQ(str, "keyboard")) input->type = VIR_DOMAIN_INPUT_TYPE_KBD; - if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) { - virDomainInputDefFree(input); - return -1; - } + VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input); } val = val->next; } @@ -928,10 +924,7 @@ xenParseXLUSBController(virConf *conf, virDomainDef *def) controller->model = usbctrl_type; controller->opts.usbopts.ports = usbctrl_ports; - if (VIR_APPEND_ELEMENT(def->controllers, def->ncontrollers, controller) < 0) { - virDomainControllerDefFree(controller); - return -1; - } + VIR_APPEND_ELEMENT(def->controllers, def->ncontrollers, controller); skipusbctrl: list = list->next; @@ -995,10 +988,7 @@ xenParseXLUSB(virConf *conf, virDomainDef *def) hostdev->source.subsys.u.usb.bus = busNum; hostdev->source.subsys.u.usb.device = devNum; - if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev) < 0) { - virDomainHostdevDefFree(hostdev); - return -1; - } + VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, hostdev); skipusb: list = list->next; @@ -1073,8 +1063,7 @@ xenParseXLChannel(virConf *conf, virDomainDef *def) channel->targetType = VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN; channel->target.name = g_steal_pointer(&name); - if (VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel); skipchannel: list = list->next; diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index feec25b9f4..ac86ddf9b7 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -255,16 +255,11 @@ xenParseXMDiskList(virConf *conf, virDomainDef *def) continue; /* Maintain list in sorted order according to target device name */ - rc = VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); - virDomainDiskDefFree(disk); - - if (rc < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); } ret = 0; - cleanup: g_strfreev(disks); return ret; } @@ -402,10 +397,7 @@ xenParseXMInputDevs(virConf *conf, virDomainDef *def) input->type = VIR_DOMAIN_INPUT_TYPE_TABLET; else if (STREQ(str, "keyboard")) input->type = VIR_DOMAIN_INPUT_TYPE_KBD; - if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) { - virDomainInputDefFree(input); - return -1; - } + VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input); } } return 0; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1ff3159fd2..1cadfe70e0 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -794,10 +794,7 @@ static int lxcContainerSetReadOnly(void) tmp = g_strdup(mntent.mnt_dir); - if (VIR_APPEND_ELEMENT(mounts, nmounts, tmp) < 0) { - g_free(tmp); - goto cleanup; - } + VIR_APPEND_ELEMENT(mounts, nmounts, tmp); } if (!mounts) { diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 066e013ed4..8953e0c904 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -597,8 +597,7 @@ static int virLXCControllerAppendNBDPids(virLXCController *ctrl, return -1; for (i = 0; i < npids; i++) { - if (VIR_APPEND_ELEMENT(ctrl->nbdpids, ctrl->nnbdpids, pids[i]) < 0) - return -1; + VIR_APPEND_ELEMENT(ctrl->nbdpids, ctrl->nnbdpids, pids[i]); } return 0; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 4bdd960e23..9bf079bbb2 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -448,8 +448,7 @@ lxcAddNetworkRouteDefinition(const char *address, 0, false))) return -1; - if (VIR_APPEND_ELEMENT(*routes, *nroutes, route) < 0) - return -1; + VIR_APPEND_ELEMENT(*routes, *nroutes, route); return 0; } @@ -569,8 +568,7 @@ lxcNetworkParseDataIPs(const char *name, g_strfreev(ipparts); - if (VIR_APPEND_ELEMENT(parseData->ips, parseData->nips, ip) < 0) - return -1; + VIR_APPEND_ELEMENT(parseData->ips, parseData->nips, ip); return 0; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a711b34c48..c9ee4b1a4e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4227,9 +4227,7 @@ networkGetDHCPLeases(virNetworkPtr net, lease->clientid = g_strdup(virJSONValueObjectGetString(lease_tmp, "client-id")); lease->hostname = g_strdup(virJSONValueObjectGetString(lease_tmp, "hostname")); - if (VIR_APPEND_ELEMENT(leases_ret, nleases, lease) < 0) - goto cleanup; - + VIR_APPEND_ELEMENT(leases_ret, nleases, lease); } else { nleases++; } diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 6f8968f1f0..65c163f519 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1191,8 +1191,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring, goto error; } - if (VIR_APPEND_ELEMENT(outdevs, noutdevs, child) < 0) - goto error; + VIR_APPEND_ELEMENT(outdevs, noutdevs, child); } } diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index ec17d43c4e..63ba69f794 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3315,8 +3315,7 @@ ebtablesGetSubChainInsts(GHashTable *chains, inst->protoidx = idx; inst->filtername = filter_names[i].key; - if (VIR_APPEND_ELEMENT(*insts, *ninsts, inst) < 0) - return -1; + VIR_APPEND_ELEMENT(*insts, *ninsts, inst); } return 0; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 43d2a61e55..4ea0f6f0d6 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -266,10 +266,7 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(inst->rules, - inst->nrules, - ruleinst) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(inst->rules, inst->nrules, ruleinst); ret = 0; cleanup: @@ -316,10 +313,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver, break; } - if (VIR_APPEND_ELEMENT(inst->filters, - inst->nfilters, - obj) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(inst->filters, inst->nfilters, obj); obj = NULL; if (virNWFilterDefToInst(driver, diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 07c2cddd92..694b048e2b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -355,8 +355,7 @@ openvzReadFSConf(virDomainDef *def, } } - if (VIR_APPEND_ELEMENT(def->fss, def->nfss, fs) < 0) - goto error; + VIR_APPEND_ELEMENT(def->fss, def->nfss, fs); VIR_FREE(temp); diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4691dff4f7..e09c7e66c8 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1962,8 +1962,7 @@ qemuBlockStorageSourceChainDetachPrepareBlockdev(virStorageSource *src) if (!(backend = qemuBlockStorageSourceDetachPrepare(n, NULL))) return NULL; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) - return NULL; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend); } return g_steal_pointer(&data); @@ -1990,8 +1989,7 @@ qemuBlockStorageSourceChainDetachPrepareDrive(virStorageSource *src, if (!(backend = qemuBlockStorageSourceDetachPrepare(src, driveAlias))) return NULL; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) - return NULL; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend); return g_steal_pointer(&data); } @@ -2016,8 +2014,7 @@ qemuBlockStorageSourceChainDetachPrepareChardev(char *chardevAlias) backend->chardevAlias = chardevAlias; backend->chardevAdded = true; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend) < 0) - return NULL; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, backend); return g_steal_pointer(&data); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 156af4caee..4381ea7d8b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8591,9 +8591,10 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, */ if (driver->privileged && nicindexes && nnicindexes && net->ifname) { - if (virNetDevGetIndex(net->ifname, &nicindex) < 0 || - VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex) < 0) + if (virNetDevGetIndex(net->ifname, &nicindex) < 0) goto cleanup; + + VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex); } break; } @@ -11005,8 +11006,7 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDef *disk, if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, elem, qemuCaps) < 0) return NULL; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) - return NULL; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem); return g_steal_pointer(&data); } @@ -11030,8 +11030,7 @@ qemuBuildStorageSourceChainAttachPrepareChardev(virDomainDiskDef *disk) if (!(elem = qemuBuildStorageSourceAttachPrepareChardev(disk))) return NULL; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) - return NULL; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem); return g_steal_pointer(&data); } @@ -11051,8 +11050,7 @@ qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainD if (qemuBuildStorageSourceAttachPrepareCommon(src, elem, qemuCaps) < 0) return -1; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) - return -1; + VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem); return 0; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 6cd0cb8c84..40e84cce9c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2969,12 +2969,10 @@ qemuDomainUSBAddressAddHubs(virDomainDef *def) hub = g_new0(virDomainHubDef, 1); hub->type = VIR_DOMAIN_HUB_TYPE_USB; - if (VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub); } ret = 0; - cleanup: VIR_FREE(hub); return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d8c431b41a..e53f167575 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7245,8 +7245,7 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, case VIR_DOMAIN_DEVICE_SOUND: sound = dev->data.sound; - if (VIR_APPEND_ELEMENT(vmdef->sounds, vmdef->nsounds, sound) < 0) - return -1; + VIR_APPEND_ELEMENT(vmdef->sounds, vmdef->nsounds, sound); dev->data.sound = NULL; break; @@ -7319,8 +7318,7 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, return -1; } - if (VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, dev->data.rng) < 0) - return -1; + VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, dev->data.rng); dev->data.rng = NULL; break; @@ -7342,8 +7340,7 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, case VIR_DOMAIN_DEVICE_REDIRDEV: redirdev = dev->data.redirdev; - if (VIR_APPEND_ELEMENT(vmdef->redirdevs, vmdef->nredirdevs, redirdev) < 0) - return -1; + VIR_APPEND_ELEMENT(vmdef->redirdevs, vmdef->nredirdevs, redirdev); dev->data.redirdev = NULL; break; @@ -7369,8 +7366,7 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, break; case VIR_DOMAIN_DEVICE_INPUT: - if (VIR_APPEND_ELEMENT(vmdef->inputs, vmdef->ninputs, dev->data.input) < 0) - return -1; + VIR_APPEND_ELEMENT(vmdef->inputs, vmdef->ninputs, dev->data.input); break; case VIR_DOMAIN_DEVICE_VSOCK: @@ -17665,8 +17661,7 @@ qemuDomainGetResctrlMonData(virQEMUDriver *driver, &res->stats, &res->nstats) < 0) goto error; - if (VIR_APPEND_ELEMENT(*resdata, *nresdata, res) < 0) - goto error; + VIR_APPEND_ELEMENT(*resdata, *nresdata, res); } } diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 5b9926f2bc..17380b7573 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1484,8 +1484,7 @@ qemuFirmwareGetSupported(const char *machine, tmp->name = g_strdup(fwpath); tmp->nvram = g_strdup(nvrampath); - if (VIR_APPEND_ELEMENT(*fws, *nfws, tmp) < 0) - return -1; + VIR_APPEND_ELEMENT(*fws, *nfws, tmp); } } } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ca2c3bb6cf..14dca96afc 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9253,8 +9253,7 @@ qemuMonitorJSONGetJobInfo(qemuMonitor *mon, if (!(job = qemuMonitorJSONGetJobInfoOne(virJSONValueArrayGet(data, i)))) return -1; - if (VIR_APPEND_ELEMENT(*jobs, *njobs, job) < 0) - return -1; + VIR_APPEND_ELEMENT(*jobs, *njobs, job); } return 0; diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index e7716d1944..ca59b5d95b 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -1226,9 +1226,8 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodData *data, g_free(next); next = g_strdup(item.target); - if (addToData && - VIR_APPEND_ELEMENT(data->items, data->nitems, item) < 0) - return -1; + if (addToData) + VIR_APPEND_ELEMENT(data->items, data->nitems, item); if (!isLink) break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 521fda57da..4730d52f01 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6001,8 +6001,7 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriver *driver, vcpupriv->vcpus != 0) { vcpupriv->alias = g_strdup_printf("vcpu%zu", i); - if (VIR_APPEND_ELEMENT(bootHotplug, nbootHotplug, vcpu) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(bootHotplug, nbootHotplug, vcpu); } } diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 65aa20f7d1..99ae25c5f7 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -4120,10 +4120,9 @@ remoteDispatchConnectDomainEventRegister(virNetServer *server G_GNUC_UNUSED, callback->callbackID = -1; callback->legacy = true; ref = callback; - if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks, - priv->ndomainEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->domainEventCallbacks, + priv->ndomainEventCallbacks, + callback); if ((callbackID = virConnectDomainEventRegisterAny(conn, NULL, @@ -4348,10 +4347,9 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServer *server G_GNUC_UNUSED, callback->callbackID = -1; callback->legacy = true; ref = callback; - if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks, - priv->ndomainEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->domainEventCallbacks, + priv->ndomainEventCallbacks, + callback); if ((callbackID = virConnectDomainEventRegisterAny(conn, NULL, @@ -4422,10 +4420,9 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServer *server G_GNUC_ callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks, - priv->ndomainEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->domainEventCallbacks, + priv->ndomainEventCallbacks, + callback); if ((callbackID = virConnectDomainEventRegisterAny(conn, dom, @@ -5898,10 +5895,9 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServer *server G_GNUC_UNUSED, callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->networkEventCallbacks, - priv->nnetworkEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->networkEventCallbacks, + priv->nnetworkEventCallbacks, + callback); if ((callbackID = virConnectNetworkEventRegisterAny(conn, net, @@ -6018,10 +6014,9 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServer *server G_GNUC_UNU callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->storageEventCallbacks, - priv->nstorageEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->storageEventCallbacks, + priv->nstorageEventCallbacks, + callback); if ((callbackID = virConnectStoragePoolEventRegisterAny(conn, pool, @@ -6137,10 +6132,9 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServer *server G_GNUC_UNUS callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->nodeDeviceEventCallbacks, - priv->nnodeDeviceEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->nodeDeviceEventCallbacks, + priv->nnodeDeviceEventCallbacks, + callback); if ((callbackID = virConnectNodeDeviceEventRegisterAny(conn, dev, @@ -6256,10 +6250,9 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServer *server G_GNUC_UNUSED, callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->secretEventCallbacks, - priv->nsecretEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->secretEventCallbacks, + priv->nsecretEventCallbacks, + callback); if ((callbackID = virConnectSecretEventRegisterAny(conn, secret, @@ -6370,10 +6363,9 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServer *server G_GNUC_UNUSED callback->eventID = -1; callback->callbackID = -1; ref = callback; - if (VIR_APPEND_ELEMENT(priv->qemuEventCallbacks, - priv->nqemuEventCallbacks, - callback) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(priv->qemuEventCallbacks, + priv->nqemuEventCallbacks, + callback); if ((callbackID = virConnectDomainQemuMonitorEventRegister(conn, dom, diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index b6f3233f64..444fe3dbe7 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -237,7 +237,9 @@ collectServers(void *payload, if (!srv) return -1; - return VIR_APPEND_ELEMENT(*data->servers, data->nservers, srv); + VIR_APPEND_ELEMENT(*data->servers, data->nservers, srv); + + return 0; } diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index e0fbd607bf..ca11f1688e 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -593,8 +593,7 @@ int virNetMessageAddFD(virNetMessage *msg, newfd); goto error; } - if (VIR_APPEND_ELEMENT(msg->fds, msg->nfds, newfd) < 0) - goto error; + VIR_APPEND_ELEMENT(msg->fds, msg->nfds, newfd); return 0; error: VIR_FORCE_CLOSE(newfd); diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index cfb65f8b5f..bb1a96b65e 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1086,16 +1086,12 @@ virNetServerGetClients(virNetServer *srv, for (i = 0; i < srv->nclients; i++) { virNetServerClient *client = virObjectRef(srv->clients[i]); - if (VIR_APPEND_ELEMENT(list, nclients, client) < 0) { - virObjectUnref(client); - goto cleanup; - } + VIR_APPEND_ELEMENT(list, nclients, client); } *clts = g_steal_pointer(&list); ret = nclients; - cleanup: virObjectListFreeCount(list, nclients); virObjectUnlock(srv); return ret; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index b874dd4ab6..04b9ecf028 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -121,8 +121,7 @@ virSecurityDACChownListAppend(virSecurityDACChownList *list, item->remember = remember; item->restore = restore; - if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0) - return -1; + VIR_APPEND_ELEMENT(list->items, list->nItems, item); return 0; } diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 0af581bc8b..9906c1691d 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -684,9 +684,8 @@ virSecurityManagerGenLabel(virSecurityManager *mgr, } else { /* The seclabel must be added to @vm prior calling domainGenSecurityLabel * which may require seclabel to be presented already */ - if (generated && - VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0) - goto cleanup; + if (generated) + VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel); if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) { if (VIR_DELETE_ELEMENT(vm->seclabels, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5f98d4d47a..30fb6028aa 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -125,11 +125,9 @@ virSecuritySELinuxContextListAppend(virSecuritySELinuxContextList *list, item->remember = remember; item->restore = restore; - if (VIR_APPEND_ELEMENT(list->items, list->nItems, item) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(list->items, list->nItems, item); ret = 0; - cleanup: virSecuritySELinuxContextItemFree(item); return ret; } @@ -1894,11 +1892,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, if (!disk_seclabel) return -1; disk_seclabel->labelskip = true; - if (VIR_APPEND_ELEMENT(src->seclabels, src->nseclabels, - disk_seclabel) < 0) { - virSecurityDeviceLabelDefFree(disk_seclabel); - return -1; - } + VIR_APPEND_ELEMENT(src->seclabels, src->nseclabels, disk_seclabel); ret = 0; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 02ede74aeb..c23d0060ed 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -201,9 +201,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, extent.start = offset * size; extent.end = (offset * size) + length; - if (VIR_APPEND_ELEMENT(vol->source.extents, vol->source.nextent, - extent) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(vol->source.extents, vol->source.nextent, extent); } ret = 0; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index a4e8115dc4..28b4b7fae6 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -639,8 +639,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) namedup = g_strdup(name); - if (VIR_APPEND_ELEMENT(names, nnames, namedup) < 0) - return NULL; + VIR_APPEND_ELEMENT(names, nnames, namedup); name += strlen(name) + 1; } diff --git a/src/util/viralloc.h b/src/util/viralloc.h index c5ca8e9929..f02d634eea 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -227,12 +227,10 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, * necessary memory re-allocation has already been done. * * These macros are safe to use on arguments with side effects. - * - * Returns -1 on failure (with OOM error reported), 0 on success */ #define VIR_APPEND_ELEMENT(ptr, count, newelem) \ - virInsertElementsN(&(ptr), sizeof(*(ptr)), -1, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) + virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) #define VIR_APPEND_ELEMENT_COPY(ptr, count, newelem) \ virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index ac48527dde..ab4363a5ab 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -369,8 +369,7 @@ virISCSIGetTargets(char **const groups, target = g_strdup(groups[1]); - if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) - return -1; + VIR_APPEND_ELEMENT(list->targets, list->ntargets, target); return 0; } diff --git a/src/util/virjson.c b/src/util/virjson.c index b5f553a724..9adcea4fff 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -599,8 +599,9 @@ virJSONValueObjectInsert(virJSONValue *object, ret = VIR_INSERT_ELEMENT(object->data.object.pairs, 0, object->data.object.npairs, pair); } else { - ret = VIR_APPEND_ELEMENT(object->data.object.pairs, - object->data.object.npairs, pair); + VIR_APPEND_ELEMENT(object->data.object.pairs, + object->data.object.npairs, pair); + ret = 0; } if (ret == 0) diff --git a/src/util/virlog.c b/src/util/virlog.c index 3ad043d98a..ac35e36148 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1638,10 +1638,7 @@ virLogParseOutputs(const char *src, virLogOutput ***outputs) * lose the old entry */ at = virLogFindOutput(list, noutputs, output->dest, output->name); - if (VIR_APPEND_ELEMENT(list, noutputs, output) < 0) { - virLogOutputFree(output); - return -1; - } + VIR_APPEND_ELEMENT(list, noutputs, output); if (at >= 0) { virLogOutputFree(list[at]); VIR_DELETE_ELEMENT(list, at, noutputs); @@ -1687,10 +1684,7 @@ virLogParseFilters(const char *src, virLogFilter ***filters) if (!(filter = virLogParseFilter(*next))) return -1; - if (VIR_APPEND_ELEMENT(list, nfilters, filter)) { - virLogFilterFree(filter); - return -1; - } + VIR_APPEND_ELEMENT(list, nfilters, filter); } *filters = g_steal_pointer(&list); diff --git a/src/util/virmdev.c b/src/util/virmdev.c index fb89b80da4..d12c7b87a0 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -303,7 +303,9 @@ virMediatedDeviceListAdd(virMediatedDeviceList *list, _("device %s is already in use"), (*dev)->path); return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, *dev); + VIR_APPEND_ELEMENT(list->devs, list->count, *dev); + + return 0; } @@ -554,8 +556,7 @@ virMediatedDeviceGetMdevTypes(const char *sysfspath, if (virMediatedDeviceTypeReadAttrs(tmppath, &mdev_type) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(mdev_types, nmdev_types, mdev_type) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(mdev_types, nmdev_types, mdev_type); } if (dirret < 0) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 407270271f..fe531a3260 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2799,8 +2799,7 @@ static int virNetDevGetMcastList(const char *ifname, /* Only return global multicast MAC addresses for * specified interface */ if (entry->global && STREQ(ifname, entry->name)) { - if (VIR_APPEND_ELEMENT(mcast->entries, mcast->nentries, entry)) - return -1; + VIR_APPEND_ELEMENT(mcast->entries, mcast->nentries, entry); } else { memset(entry, 0, sizeof(virNetDevMcastEntry)); } diff --git a/src/util/virnvme.c b/src/util/virnvme.c index 11da76e760..3ce02b5000 100644 --- a/src/util/virnvme.c +++ b/src/util/virnvme.c @@ -191,11 +191,10 @@ virNVMeDeviceListAdd(virNVMeDeviceList *list, return -1; } - if (!(tmp = virNVMeDeviceCopy(dev)) || - VIR_APPEND_ELEMENT(list->devs, list->count, tmp) < 0) { - virNVMeDeviceFree(tmp); + if (!(tmp = virNVMeDeviceCopy(dev))) return -1; - } + + VIR_APPEND_ELEMENT(list->devs, list->count, tmp); return 0; } diff --git a/src/util/virpci.c b/src/util/virpci.c index bf42bcf73c..67c3896dd5 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1688,7 +1688,9 @@ virPCIDeviceListAdd(virPCIDeviceList *list, _("Device %s is already in use"), dev->name); return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, dev); + VIR_APPEND_ELEMENT(list->devs, list->count, dev); + + return 0; } @@ -1948,9 +1950,8 @@ virPCIGetIOMMUGroupAddressesAddOne(virPCIDeviceAddress *newDevAddr, void *opaque *copyAddr = *newDevAddr; - if (VIR_APPEND_ELEMENT(*addrList->iommuGroupDevices, - *addrList->nIommuGroupDevices, copyAddr) < 0) - return -1; + VIR_APPEND_ELEMENT(*addrList->iommuGroupDevices, + *addrList->nIommuGroupDevices, copyAddr); return 0; } @@ -2363,9 +2364,7 @@ virPCIGetVirtualFunctions(const char *sysfs_path, goto error; } - if (VIR_APPEND_ELEMENT(*virtual_functions, *num_virtual_functions, - config_addr) < 0) - goto error; + VIR_APPEND_ELEMENT(*virtual_functions, *num_virtual_functions, config_addr); } while (1); VIR_DEBUG("Found %zu virtual functions for %s", diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 67dd599b3e..a391e7f3c6 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -632,8 +632,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) goto cleanup; tmp_pid = tmp; - if (VIR_APPEND_ELEMENT(*pids, *npids, tmp_pid) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*pids, *npids, tmp_pid); } if (value < 0) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 1c41d1d356..edbf078654 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2634,14 +2634,12 @@ virResctrlMonitorGetStats(virResctrlMonitor *monitor, if (rv < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val); stat->features[i] = g_strdup(resources[i]); } - if (VIR_APPEND_ELEMENT(*stats, *nstats, stat) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*stats, *nstats, stat); } /* Sort in id's ascending order */ diff --git a/src/util/virscsi.c b/src/util/virscsi.c index b1f202eef1..2488b0e5f8 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -254,8 +254,7 @@ virSCSIDeviceSetUsedBy(virSCSIDevice *dev, copy->drvname = g_strdup(drvname); copy->domname = g_strdup(domname); - if (VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy) < 0) - return -1; + VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy); return 0; } @@ -359,7 +358,9 @@ virSCSIDeviceListAdd(virSCSIDeviceList *list, return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, dev); + VIR_APPEND_ELEMENT(list->devs, list->count, dev); + + return 0; } virSCSIDevice * diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index 396f10d708..afbfddb0fb 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -183,7 +183,9 @@ virSCSIVHostDeviceListAdd(virSCSIVHostDeviceList *list, _("Device %s is already in use"), dev->name); return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, dev); + VIR_APPEND_ELEMENT(list->devs, list->count, dev); + + return 0; } diff --git a/src/util/virusb.c b/src/util/virusb.c index 183b8a9792..9e6ec9c9cf 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -447,7 +447,9 @@ virUSBDeviceListAdd(virUSBDeviceList *list, (*dev)->name); return -1; } - return VIR_APPEND_ELEMENT(list->devs, list->count, *dev); + VIR_APPEND_ELEMENT(list->devs, list->count, *dev); + + return 0; } virUSBDevice * diff --git a/src/util/virutil.c b/src/util/virutil.c index 00cd56e2b2..c9de043c40 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -971,13 +971,8 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list) if ((*list)[i] == gid) goto cleanup; } - if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) { - ret = -1; - VIR_FREE(*list); - goto cleanup; - } else { - ret = i; - } + VIR_APPEND_ELEMENT(*list, i, gid); + ret = i; } cleanup: diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1ca521321c..6cb5ba8928 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3535,9 +3535,8 @@ vboxDumpDisplay(virDomainDef *def, struct _vboxDriver *data, IMachine *machine) graphics->data.desktop.display = g_strdup(getenv("DISPLAY")); } - if (graphics && - VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0) - goto cleanup; + if (graphics) + VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics); gVBoxAPI.UIMachine.GetVRDEServer(machine, &VRDEServer); if (VRDEServer) @@ -3574,8 +3573,7 @@ vboxDumpDisplay(virDomainDef *def, struct _vboxDriver *data, IMachine *machine) if (reuseSingleConnection) graphics->data.rdp.replaceUser = true; - if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics); } ret = 0; @@ -3758,10 +3756,7 @@ vboxDumpNetworks(virDomainDef *def, struct _vboxDriver *data, IMachine *machine, if (enabled) { net = vboxDumpNetwork(data, adapter); - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) { - VBOX_RELEASE(adapter); - return -1; - } + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); } VBOX_RELEASE(adapter); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 04eabff18a..de86eb1f80 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1893,8 +1893,7 @@ virVMXParseConfig(virVMXContext *ctx, if (!net) continue; - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); } /* def:inputs */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index e09950812d..1cc9eb8caf 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -553,8 +553,7 @@ prlsdkAddDomainVideoInfoCt(virDomainDef *def, video->type = VIR_DOMAIN_VIDEO_TYPE_PARALLELS; video->vram = 0; - if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(def->videos, def->nvideos, video); ret = 0; cleanup: @@ -951,10 +950,7 @@ prlsdkGetNetAddresses(PRL_HANDLE sdknet, virDomainNetDef *net) if (!(ip = prlsdkParseNetAddress(addr))) continue; - if (VIR_APPEND_ELEMENT(net->guestIP.ips, net->guestIP.nips, ip) < 0) { - VIR_FREE(ip); - goto cleanup; - } + VIR_APPEND_ELEMENT(net->guestIP.ips, net->guestIP.nips, ip); } ret = 0; @@ -985,8 +981,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDef *net) NULL, gw, 0, true, 0, false))) goto cleanup; - if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route); } if (*gw6 != '\0') { @@ -995,8 +990,7 @@ prlsdkGetRoutes(PRL_HANDLE sdknet, virDomainNetDef *net) NULL, gw6, 0, true, 0, false))) goto cleanup; - if (VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(net->guestIP.routes, net->guestIP.nroutes, route); } ret = 0; @@ -1132,8 +1126,7 @@ prlsdkAddDomainNetInfo(PRL_HANDLE sdkdom, virDomainDef *def) PrlHandle_Free(netAdapter); netAdapter = PRL_INVALID_HANDLE; - if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) - goto error; + VIR_APPEND_ELEMENT(def->nets, def->nnets, net); } return 0; @@ -1248,8 +1241,7 @@ prlsdkAddSerialInfo(PRL_HANDLE sdkdom, PrlHandle_Free(serialPort); serialPort = PRL_INVALID_HANDLE; - if (VIR_APPEND_ELEMENT(*serials, *nserials, chr) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(*serials, *nserials, chr); } return 0; @@ -1333,8 +1325,7 @@ prlsdkAddVNCInfo(PRL_HANDLE sdkdom, virDomainDef *def) gr->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, gr) < 0) - goto error; + VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, gr); return 0; diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 8c82efa060..33648a5077 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -213,10 +213,7 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(inst->rules, - inst->nrules, - ruleinst) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(inst->rules, inst->nrules, ruleinst); ruleinst = NULL; ret = 0; diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 143dd954e6..30d475ebe4 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -465,19 +465,10 @@ qemuMonitorTestAddHandler(qemuMonitorTest *test, item->opaque = opaque; virMutexLock(&test->lock); - if (VIR_APPEND_ELEMENT(test->items, test->nitems, item) < 0) { - virMutexUnlock(&test->lock); - goto error; - } + VIR_APPEND_ELEMENT(test->items, test->nitems, item); virMutexUnlock(&test->lock); return 0; - - error: - if (freecb) - (freecb)(opaque); - VIR_FREE(item); - return -1; } void * diff --git a/tests/testutils.c b/tests/testutils.c index 58d3300998..682fa142b5 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -812,10 +812,12 @@ int virTestMain(int argc, if (!getenv("LIBVIRT_DEBUG") && !virLogGetNbOutputs()) { if (!(output = virLogOutputNew(virtTestLogOutput, virtTestLogClose, &testLog, VIR_LOG_DEBUG, - VIR_LOG_TO_STDERR, NULL)) || - VIR_APPEND_ELEMENT(outputs, noutputs, output) < 0 || - virLogDefineOutputs(outputs, noutputs) < 0) { - virLogOutputFree(output); + VIR_LOG_TO_STDERR, NULL))) + return EXIT_FAILURE; + + VIR_APPEND_ELEMENT(outputs, noutputs, output); + + if (virLogDefineOutputs(outputs, noutputs) < 0) { virLogOutputListFree(outputs, noutputs); return EXIT_FAILURE; } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f9962f0515..81f3c82094 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3852,8 +3852,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) vol.source = g_steal_pointer(&source); vol.target = g_steal_pointer(&target); - if (VIR_APPEND_ELEMENT(vols, nvols, vol) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(vols, nvols, vol); } /* print volumes specified by user that were not found in domain definition */ diff --git a/tools/vsh-table.c b/tools/vsh-table.c index 78d1cb27f1..7fb0f13ee6 100644 --- a/tools/vsh-table.c +++ b/tools/vsh-table.c @@ -102,8 +102,7 @@ vshTableRowNew(const char *arg, va_list ap) tmp = g_strdup(arg); - if (VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp) < 0) - goto error; + VIR_APPEND_ELEMENT(row->cells, row->ncells, tmp); arg = va_arg(ap, const char *); } @@ -140,8 +139,7 @@ vshTableNew(const char *arg, ...) if (!header) goto error; - if (VIR_APPEND_ELEMENT(table->rows, table->nrows, header) < 0) - goto error; + VIR_APPEND_ELEMENT(table->rows, table->nrows, header); return table; error: @@ -182,8 +180,7 @@ vshTableRowAppend(vshTable *table, const char *arg, ...) goto cleanup; } - if (VIR_APPEND_ELEMENT(table->rows, table->nrows, row) < 0) - goto cleanup; + VIR_APPEND_ELEMENT(table->rows, table->nrows, row); ret = 0; cleanup: -- 2.31.1
VIR_APPEND_ELEMENT doesn't report any errors now so we can remove VIR_APPEND_ELEMENT_QUIET and replace all uses by VIR_APPEND_ELEMENT Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viralloc.h | 5 ----- tests/virfilewrapper.c | 4 ++-- tests/virpcimock.c | 6 +++--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index f02d634eea..3a09af65c5 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -243,11 +243,6 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, VIR_TYPEMATCH(ptr, &(newelem)), \ &(newelem), false, true) -/* Quiet version of macros above */ -#define VIR_APPEND_ELEMENT_QUIET(ptr, count, newelem) \ - virAppendElement(&(ptr), sizeof(*(ptr)), &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) - /** * VIR_DELETE_ELEMENT: * @ptr: pointer to array of objects (*not* ptr to ptr) diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c index 3085a190fb..aaff39866d 100644 --- a/tests/virfilewrapper.c +++ b/tests/virfilewrapper.c @@ -78,8 +78,8 @@ virFileWrapperAddPrefix(const char *prefix, init_syms(); - VIR_APPEND_ELEMENT_QUIET(prefixes, nprefixes, prefix); - VIR_APPEND_ELEMENT_QUIET(overrides, noverrides, override); + VIR_APPEND_ELEMENT(prefixes, nprefixes, prefix); + VIR_APPEND_ELEMENT(overrides, noverrides, override); } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index de39fc65a4..ead810d412 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -428,7 +428,7 @@ pci_device_create_iommu(const struct pciDevice *dev, iommuGroup->iommu = dev->iommuGroup; iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default */ - VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, iommuGroup); + VIR_APPEND_ELEMENT(pciIommuGroups, npciIommuGroups, iommuGroup); } @@ -542,7 +542,7 @@ pci_device_new_from_stub(const struct pciDevice *data) if (pci_device_autobind(dev) < 0) ABORT("Unable to bind: %s", devid); - VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev); + VIR_APPEND_ELEMENT(pciDevices, nPCIDevices, dev); } static struct pciDevice * @@ -713,7 +713,7 @@ pci_driver_new(const char *name, ...) make_file(driverpath, "bind", NULL, -1); make_file(driverpath, "unbind", NULL, -1); - VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver); + VIR_APPEND_ELEMENT(pciDrivers, nPCIDrivers, driver); } static struct pciDriver * -- 2.31.1
'video' will only ever be NULL after the 'cleanup' label thus there's no need to use 'virDomainVideoDefFree'. In fact we can fully remove the cleanup section and 'ret' variable by returning directly from failure points. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/vz/vz_sdk.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 1cc9eb8caf..4f0aae91a2 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -542,24 +542,19 @@ prlsdkAddDomainVideoInfoCt(virDomainDef *def, virDomainXMLOption *xmlopt) { virDomainVideoDef *video = NULL; - int ret = -1; if (def->ngraphics == 0) return 0; if (!(video = virDomainVideoDefNew(xmlopt))) - goto cleanup; + return -1; video->type = VIR_DOMAIN_VIDEO_TYPE_PARALLELS; video->vram = 0; VIR_APPEND_ELEMENT(def->videos, def->nvideos, video); - ret = 0; - cleanup: - virDomainVideoDefFree(video); - - return ret; + return 0; } static int -- 2.31.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/xen_xm.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index ac86ddf9b7..f978b94f93 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -238,9 +238,9 @@ xenParseXMDisk(char *entry, int hvm) static int xenParseXMDiskList(virConf *conf, virDomainDef *def) { - char **disks = NULL, **entries; + g_auto(GStrv) disks = NULL; + GStrv entries; int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM; - int ret = -1; int rc; rc = virConfGetValueStringList(conf, "disk", false, &disks); @@ -258,10 +258,7 @@ xenParseXMDiskList(virConf *conf, virDomainDef *def) VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk); } - ret = 0; - - g_strfreev(disks); - return ret; + return 0; } -- 2.31.1
'hub' doesn't need to be freed any more because it's always consumed and NULLed-out by VIR_APPEND element. This also makes the 'ret' variable obsolete. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain_address.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 40e84cce9c..392368bd38 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2942,7 +2942,6 @@ qemuDomainUSBAddressAddHubs(virDomainDef *def) virDomainHubDef *hub = NULL; size_t available_ports; size_t hubs_needed = 0; - int ret = -1; size_t i; available_ports = virDomainUSBAddressCountAllPorts(def); @@ -2972,9 +2971,7 @@ qemuDomainUSBAddressAddHubs(virDomainDef *def) VIR_APPEND_ELEMENT(def->hubs, def->nhubs, hub); } - ret = 0; - VIR_FREE(hub); - return ret; + return 0; } -- 2.31.1
'item' is always NULLed-out by VIR_APPEND_ELEMENT and 'ret' variable is always 0 when used so both can be removed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/security/security_selinux.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 30fb6028aa..9ff35a7be5 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -114,7 +114,6 @@ virSecuritySELinuxContextListAppend(virSecuritySELinuxContextList *list, bool remember, bool restore) { - int ret = -1; virSecuritySELinuxContextItem *item = NULL; item = g_new0(virSecuritySELinuxContextItem, 1); @@ -127,9 +126,7 @@ virSecuritySELinuxContextListAppend(virSecuritySELinuxContextList *list, VIR_APPEND_ELEMENT(list->items, list->nItems, item); - ret = 0; - virSecuritySELinuxContextItemFree(item); - return ret; + return 0; } static void -- 2.31.1
'dev->data.rng' is NULLed by VIR_APPEND_ELEMENT Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e53f167575..a7d76dd00f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7319,8 +7319,6 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, } VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, dev->data.rng); - dev->data.rng = NULL; - break; case VIR_DOMAIN_DEVICE_MEMORY: -- 2.31.1
'ruleinst' is NULLed by VIR_APPEND_ELEMENT Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 33648a5077..7fefd3ce75 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -214,7 +214,6 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, goto cleanup; VIR_APPEND_ELEMENT(inst->rules, inst->nrules, ruleinst); - ruleinst = NULL; ret = 0; cleanup: -- 2.31.1
'cb' is always NULL when 'virObjectEventCallbackListAddID' is called. Remove the call. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/object_event.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/conf/object_event.c b/src/conf/object_event.c index 866d438f89..9ea9ee3496 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -453,7 +453,6 @@ virObjectEventCallbackListAddID(virConnectPtr conn, ret++; } - virObjectEventCallbackFree(cb); return ret; } -- 2.31.1
Construct the 'ruleinst->vars' hash table separately in a temporary variable so that 'ruleinst' can be allocated on success. This allows us to get rid of the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 4ea0f6f0d6..c9ffa30839 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -252,8 +252,11 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, GHashTable *vars, virNWFilterInst *inst) { + g_autoptr(GHashTable) tmpvars = virHashNew(virNWFilterVarValueHashFree); virNWFilterRuleInst *ruleinst; - int ret = -1; + + if (virNWFilterHashTablePutAll(vars, tmpvars) < 0) + return -1; ruleinst = g_new0(virNWFilterRuleInst, 1); @@ -261,17 +264,11 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, ruleinst->chainPriority = def->chainPriority; ruleinst->def = rule; ruleinst->priority = rule->priority; - ruleinst->vars = virHashNew(virNWFilterVarValueHashFree); - - if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) - goto cleanup; + ruleinst->vars = g_steal_pointer(&tmpvars); VIR_APPEND_ELEMENT(inst->rules, inst->nrules, ruleinst); - ret = 0; - cleanup: - virNWFilterRuleInstFree(ruleinst); - return ret; + return 0; } -- 2.31.1
Use automatic memory freeing for 'tmpvars' and move the allocation of tmpvars earlier so that we are guaranteed that 'obj' will always be appended to 'inst->filters' and thus don't need cleanup for it. By moving the reset of 'inst' to the block when virNWFilterDefToInst fails we can get rid of the rest of the cleanup section and remove the 'ret' variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 32 +++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index c9ffa30839..ecba16d55c 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -281,20 +281,20 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver, virNWFilterInst *inst) { virNWFilterObj *obj; - GHashTable *tmpvars = NULL; + g_autoptr(GHashTable) tmpvars = NULL; virNWFilterDef *childdef; virNWFilterDef *newChilddef; - int ret = -1; VIR_DEBUG("Instantiating filter %s", inc->filterref); - if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters, - inc->filterref))) - goto cleanup; /* create a temporary hashmap for depth-first tree traversal */ - if (!(tmpvars = virNWFilterCreateVarsFrom(inc->params, - vars))) - goto cleanup; + if (!(tmpvars = virNWFilterCreateVarsFrom(inc->params, vars))) + return -1; + + /* 'obj' is always appended to 'inst->filters' thus we don't unlock it */ + if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters, + inc->filterref))) + return -1; childdef = virNWFilterObjGetDef(obj); @@ -311,24 +311,18 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver, } VIR_APPEND_ELEMENT(inst->filters, inst->nfilters, obj); - obj = NULL; if (virNWFilterDefToInst(driver, childdef, tmpvars, useNewFilter, foundNewFilter, - inst) < 0) - goto cleanup; - - ret = 0; - cleanup: - if (ret < 0) + inst) < 0) { virNWFilterInstReset(inst); - virHashFree(tmpvars); - if (obj) - virNWFilterObjUnlock(obj); - return ret; + return -1; + } + + return 0; } -- 2.31.1
'bootHotplug' can be auto-freed when terminating the function and moving the declaration of 'vcpuprops' to the loop which uses it along with automatic freeing allows us to simplify cleanup in certain cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4730d52f01..68d9de798d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5985,12 +5985,11 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriver *driver, qemuCgroupEmulatorAllNodesData *emulatorCgroup = NULL; virDomainVcpuDef *vcpu; qemuDomainVcpuPrivate *vcpupriv; - virJSONValue *vcpuprops = NULL; size_t i; int ret = -1; int rc; - virDomainVcpuDef **bootHotplug = NULL; + g_autofree virDomainVcpuDef **bootHotplug = NULL; size_t nbootHotplug = 0; for (i = 0; i < maxvcpus; i++) { @@ -6005,10 +6004,8 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriver *driver, } } - if (nbootHotplug == 0) { - ret = 0; - goto cleanup; - } + if (nbootHotplug == 0) + return 0; qsort(bootHotplug, nbootHotplug, sizeof(*bootHotplug), qemuProcessVcpusSortOrder); @@ -6017,6 +6014,7 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriver *driver, goto cleanup; for (i = 0; i < nbootHotplug; i++) { + g_autoptr(virJSONValue) vcpuprops = NULL; vcpu = bootHotplug[i]; if (!(vcpuprops = qemuBuildHotpluggableCPUProps(vcpu))) @@ -6033,16 +6031,12 @@ qemuProcessSetupHotpluggableVcpus(virQEMUDriver *driver, if (rc < 0) goto cleanup; - - virJSONValueFree(vcpuprops); } ret = 0; cleanup: qemuCgroupEmulatorAllNodesRestore(emulatorCgroup); - VIR_FREE(bootHotplug); - virJSONValueFree(vcpuprops); return ret; } -- 2.31.1
'list' will always be NULL when reaching 'virObjectListFreeCount' thus we can remove the call as well as the 'ret' variable which was only ever equal to 'nclients' at the point when we returned the value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/rpc/virnetserver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index bb1a96b65e..b3214883ee 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1077,7 +1077,6 @@ int virNetServerGetClients(virNetServer *srv, virNetServerClient ***clts) { - int ret = -1; size_t i; size_t nclients = 0; virNetServerClient **list = NULL; @@ -1090,11 +1089,10 @@ virNetServerGetClients(virNetServer *srv, } *clts = g_steal_pointer(&list); - ret = nclients; - virObjectListFreeCount(list, nclients); virObjectUnlock(srv); - return ret; + + return nclients; } virNetServerClient * -- 2.31.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_native.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 9bf079bbb2..347d5f4139 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -548,7 +548,7 @@ lxcNetworkParseDataIPs(const char *name, lxcNetworkParseData *parseData) { int family = AF_INET; - char **ipparts = NULL; + g_auto(GStrv) ipparts = NULL; g_autofree virNetDevIPAddr *ip = g_new0(virNetDevIPAddr, 1); if (STREQ(name, "ipv6") || STREQ(name, "ipv6.address")) @@ -561,13 +561,9 @@ lxcNetworkParseDataIPs(const char *name, virReportError(VIR_ERR_INVALID_ARG, _("Invalid CIDR address: '%s'"), value->str); - - g_strfreev(ipparts); return -1; } - g_strfreev(ipparts); - VIR_APPEND_ELEMENT(parseData->ips, parseData->nips, ip); return 0; -- 2.31.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_logical.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index c23d0060ed..cffa2c8849 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -121,10 +121,13 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, g_autoptr(GRegex) re = NULL; g_autoptr(GError) err = NULL; g_autoptr(GMatchInfo) info = NULL; - int nextents, ret = -1; + int nextents; + int ret = -1; const char *regex_unit = "(\\S+)\\((\\S+)\\)"; size_t i; - unsigned long long offset, size, length; + unsigned long long offset; + unsigned long long size; + unsigned long long length; virStorageVolSourceExtent extent; g_autofree char *regex = NULL; -- 2.31.1
It's used only inside the loop filling the extents, move it there and restructure the code so that 'extent.path' doesn't have to be freed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_logical.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index cffa2c8849..90e7c9e41d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -128,11 +128,8 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, unsigned long long offset; unsigned long long size; unsigned long long length; - virStorageVolSourceExtent extent; g_autofree char *regex = NULL; - memset(&extent, 0, sizeof(extent)); - /* Assume 1 extent (the regex for 'devices' is "(\\S+)") and only * check the 'stripes' field if we have a striped, mirror, or one of * the raid (raid1, raid4, raid5*, raid6*, or raid10) segtypes in which @@ -189,11 +186,12 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, * is the whole matched string. */ for (i = 0; i < nextents; i++) { - size_t j; g_autofree char *offset_str = NULL; + virStorageVolSourceExtent extent; + size_t j = (i * 2) + 1; + + memset(&extent, 0, sizeof(extent)); - j = (i * 2) + 1; - extent.path = g_match_info_fetch(info, j); offset_str = g_match_info_fetch(info, j + 1); if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { @@ -201,6 +199,8 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, _("malformed volume extent offset value")); goto cleanup; } + + extent.path = g_match_info_fetch(info, j); extent.start = offset * size; extent.end = (offset * size) + length; @@ -210,7 +210,6 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, ret = 0; cleanup: - VIR_FREE(extent.path); return ret; } -- 2.31.1
The function was inconsistently using 'return -1' and 'goto cleanup;' unify it by removing the cleanup label and 'ret' variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_logical.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 90e7c9e41d..ed490b0201 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -122,7 +122,6 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, g_autoptr(GError) err = NULL; g_autoptr(GMatchInfo) info = NULL; int nextents; - int ret = -1; const char *regex_unit = "(\\S+)\\((\\S+)\\)"; size_t i; unsigned long long offset; @@ -144,20 +143,20 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, if (virStrToLong_i(groups[5], NULL, 10, &nextents) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent stripes value")); - goto cleanup; + return -1; } } if (virStrToLong_ull(groups[6], NULL, 10, &length) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent length value")); - goto cleanup; + return -1; } if (virStrToLong_ull(groups[7], NULL, 10, &size) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent size value")); - goto cleanup; + return -1; } /* Allocate space for 'nextents' regex_unit strings plus a comma for each */ @@ -179,7 +178,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, if (!g_regex_match(re, groups[3], 0, &info)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent devices value")); - goto cleanup; + return -1; } /* Each extent has a "path:offset" pair, and match #0 @@ -197,7 +196,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent offset value")); - goto cleanup; + return -1; } extent.path = g_match_info_fetch(info, j); @@ -207,10 +206,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol, VIR_APPEND_ELEMENT(vol->source.extents, vol->source.nextent, extent); } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.31.1
On 8/4/21 1:02 PM, Peter Krempa wrote:
Since this touches all kind of conditionally compiled code you can find the pipeline build results here:
https://gitlab.com/pipo.sk/libvirt/-/pipelines/347734973
Peter Krempa (23): virInsertElementsN: Rename 'add' argument virInsertElementsN: Split out actual insertion code util: alloc: Introduce virAppendElement helper util: alloc: Reimplement VIR_APPEND_ELEMENT_(COPY_)INPLACE using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT_COPY using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT_QUIET using virAppendElement util: alloc: Reimplement VIR_APPEND_ELEMENT using virAppendElement util: alloc: Completely replace VIR_APPEND_ELEMENT_QUIET by VIR_APPEND_ELEMENT prlsdkAddDomainVideoInfoCt: Remove pointless cleanup section xenParseXMDisk: Use automatic memory clearing and remove 'ret' variable qemuDomainUSBAddressAddHubs: Refactor cleanup virSecuritySELinuxContextListAppend: Remove unreachable cleanup qemuDomainAttachDeviceConfig: Remove pointless assignment virNWFilterRuleDefToRuleInst: Remove pointless assignment virObjectEventCallbackListAddID: Remove pointless cleanup of 'cb' virNWFilterRuleDefToRuleInst: Restructure code to avoid cleanup virNWFilterIncludeDefToRuleInst: Refactor cleanup qemuProcessSetupHotpluggableVcpus: Use automatic memory freeing virNetServerGetClients: Remove pointless cleanup lxcNetworkParseDataIPs: Automatically free string list virStorageBackendLogicalParseVolExtents: Declare one variable per line virStorageBackendLogicalParseVolExtents: Move 'extents' inside the loop virStorageBackendLogicalParseVolExtents: Remove 'cleanup' and 'ret'
75 files changed, 404 insertions(+), 630 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa