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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

On 8/4/21 1:02 PM, Peter Krempa wrote: 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

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(a)redhat.com&gt; --- 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

'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(a)redhat.com&gt; --- 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

'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(a)redhat.com&gt; --- 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

'dev->data.rng' is NULLed by VIR_APPEND_ELEMENT Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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

'cb' is always NULL when 'virObjectEventCallbackListAddID' is called. Remove the call. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

'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(a)redhat.com&gt; --- 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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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