Move the 'path' and 'type' variables down to the appropriate block and use automatic freeing for them as well as the temporary virBitmap. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index cea4803fc8..34295ec4d7 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2155,11 +2155,9 @@ int virCapabilitiesInitCaches(virCaps *caps) { size_t i = 0; - virBitmap *cpus = NULL; + g_autoptr(virBitmap) cpus = NULL; ssize_t pos = -1; int ret = -1; - char *path = NULL; - char *type = NULL; struct dirent *ent = NULL; virCapsHostCacheBank *bank = NULL; const virResctrlMonitorType montype = VIR_RESCTRL_MONITOR_TYPE_CACHE; @@ -2180,9 +2178,7 @@ virCapabilitiesInitCaches(virCaps *caps) while ((pos = virBitmapNextSetBit(cpus, pos)) >= 0) { int rv = -1; g_autoptr(DIR) dirp = NULL; - - VIR_FREE(path); - path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos); + g_autofree char *path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos); rv = virDirOpenIfExists(&dirp, path); if (rv < 0) @@ -2192,6 +2188,7 @@ virCapabilitiesInitCaches(virCaps *caps) continue; while ((rv = virDirRead(dirp, &ent, path)) > 0) { + g_autofree char *type = NULL; int kernel_type; unsigned int level; @@ -2242,7 +2239,6 @@ virCapabilitiesInitCaches(virCaps *caps) } bank->type = kernel_type; - VIR_FREE(type); for (i = 0; i < caps->host.cache.nbanks; i++) { if (virCapsHostCacheBankEquals(bank, caps->host.cache.banks[i])) @@ -2284,10 +2280,7 @@ virCapabilitiesInitCaches(virCaps *caps) ret = 0; cleanup: - VIR_FREE(type); - VIR_FREE(path); virCapsHostCacheBankFree(bank); - virBitmapFree(cpus); return ret; } -- 2.31.1

Use automatic freeing where possible and use g_clear_pointer instead of manual NULL-ing. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 34295ec4d7..ef594d7241 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -104,10 +104,8 @@ virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPU *cpus, if (!cpus) return; - for (i = 0; i < ncpus; i++) { - virBitmapFree(cpus[i].siblings); - cpus[i].siblings = NULL; - } + for (i = 0; i < ncpus; i++) + g_clear_pointer(&cpus[i].siblings, virBitmapFree); } static void @@ -1416,20 +1414,18 @@ virBitmap * virCapabilitiesHostNUMAGetCpus(virCapsHostNUMA *caps, virBitmap *nodemask) { - virBitmap *ret = NULL; + g_autoptr(virBitmap) ret = NULL; unsigned int maxcpu = virCapabilitiesHostNUMAGetMaxcpu(caps); ssize_t node = -1; ret = virBitmapNew(maxcpu + 1); while ((node = virBitmapNextSetBit(nodemask, node)) >= 0) { - if (virCapabilitiesHostNUMAGetCellCpus(caps, node, ret) < 0) { - virBitmapFree(ret); + if (virCapabilitiesHostNUMAGetCellCpus(caps, node, ret) < 0) return NULL; - } } - return ret; + return g_steal_pointer(&ret); } -- 2.31.1

Automatically free the 'ret' temporary bitmap and get rid of the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 107d2a4f5d..c634e7dd41 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17977,34 +17977,30 @@ virDomainSchedulerParse(xmlNodePtr node, virProcessSchedPolicy *policy, int *priority) { - virBitmap *ret = NULL; + g_autoptr(virBitmap) ret = NULL; g_autofree char *tmp = NULL; if (!(tmp = virXMLPropString(node, attributeName))) { virReportError(VIR_ERR_XML_ERROR, _("Missing attribute '%s' in element '%s'"), attributeName, elementName); - goto error; + return NULL; } if (virBitmapParse(tmp, &ret, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto error; + return NULL; if (virBitmapIsAllClear(ret)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("'%s' scheduler bitmap '%s' is empty"), attributeName, tmp); - goto error; + return NULL; } if (virDomainSchedulerParseCommonAttrs(node, policy, priority) < 0) - goto error; - - return ret; + return NULL; - error: - virBitmapFree(ret); - return NULL; + return g_steal_pointer(&ret); } -- 2.31.1

Use automatic memory clearing for the temporary strings and bitmap and remove the cleanup section. There are multiple temporary strings added so that we don't reuse one. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 52 +++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 9a9b5f4b60..3bc1f63a5d 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -224,22 +224,23 @@ virDomainNumatuneParseXML(virDomainNuma *numa, bool placement_static, xmlXPathContextPtr ctxt) { - char *tmp = NULL; + g_autofree char *modestr = NULL; int mode = -1; int n = 0; + g_autofree char *placementstr = NULL; int placement = -1; - int ret = -1; - virBitmap *nodeset = NULL; + g_autofree char *nodesetstr = NULL; + g_autoptr(virBitmap) nodeset = NULL; xmlNodePtr node = NULL; if (virXPathInt("count(./numatune)", ctxt, &n) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot extract numatune nodes")); - goto cleanup; + return -1; } else if (n > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only one numatune is supported")); - goto cleanup; + return -1; } node = virXPathNode("./numatune/memory[1]", ctxt); @@ -248,34 +249,29 @@ virDomainNumatuneParseXML(virDomainNuma *numa, placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO; if (node) { - if ((tmp = virXMLPropString(node, "mode")) && - (mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) { + if ((modestr = virXMLPropString(node, "mode")) && + (mode = virDomainNumatuneMemModeTypeFromString(modestr)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported NUMA memory tuning mode '%s'"), tmp); - goto cleanup; + _("Unsupported NUMA memory tuning mode '%s'"), modestr); + return -1; } - VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "placement")) && - (placement = virDomainNumatunePlacementTypeFromString(tmp)) < 0) { + if ((placementstr = virXMLPropString(node, "placement")) && + (placement = virDomainNumatunePlacementTypeFromString(placementstr)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unsupported NUMA memory placement mode '%s'"), tmp); - goto cleanup; + _("Unsupported NUMA memory placement mode '%s'"), placementstr); + return -1; } - VIR_FREE(tmp); - tmp = virXMLPropString(node, "nodeset"); - if (tmp) { - if (virBitmapParse(tmp, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; + if ((nodesetstr = virXMLPropString(node, "nodeset"))) { + if (virBitmapParse(nodesetstr, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + return -1; if (virBitmapIsAllClear(nodeset)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid value of 'nodeset': %s"), tmp); - goto cleanup; + _("Invalid value of 'nodeset': %s"), nodesetstr); + return -1; } - - VIR_FREE(tmp); } } @@ -284,16 +280,12 @@ virDomainNumatuneParseXML(virDomainNuma *numa, placement, mode, nodeset) < 0) - goto cleanup; + return -1; if (virDomainNumatuneNodeParseXML(numa, ctxt) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virBitmapFree(nodeset); - VIR_FREE(tmp); - return ret; + return 0; } int -- 2.31.1

Use g_autoptr for the temp bitmap. To achieve this the variable must be moved down to the appropriate scope. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/hypervisor/domain_driver.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 31737b0f4a..2083f06287 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -592,8 +592,6 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, unsigned int bitmap_size) { virDomainIOThreadInfoPtr *info_ret = NULL; - virBitmap *bitmap = NULL; - virBitmap *cpumask = NULL; size_t i; int ret = -1; @@ -603,6 +601,8 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, info_ret = g_new0(virDomainIOThreadInfoPtr, targetDef->niothreadids); for (i = 0; i < targetDef->niothreadids; i++) { + g_autoptr(virBitmap) bitmap = NULL; + virBitmap *cpumask = NULL; info_ret[i] = g_new0(virDomainIOThreadInfo, 1); /* IOThread ID's are taken from the iothreadids list */ @@ -627,8 +627,6 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, if (virBitmapToData(cpumask, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen) < 0) goto cleanup; - virBitmapFree(bitmap); - bitmap = NULL; } *info = g_steal_pointer(&info_ret); @@ -640,7 +638,6 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, virDomainIOThreadInfoFree(info_ret[i]); VIR_FREE(info_ret); } - virBitmapFree(bitmap); return ret; } -- 2.31.1

Automatically free the 'slotmap' bitmap and get rid of the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain_address.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index f4cddc6176..18fc34d049 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -3086,8 +3086,7 @@ qemuDomainReleaseMemoryDeviceSlot(virDomainObj *vm, static int qemuDomainAssignMemorySlots(virDomainDef *def) { - virBitmap *slotmap = NULL; - int ret = -1; + g_autoptr(virBitmap) slotmap = NULL; size_t i; if (!virDomainDefHasMemoryHotplug(def)) @@ -3103,7 +3102,7 @@ qemuDomainAssignMemorySlots(virDomainDef *def) case VIR_DOMAIN_MEMORY_MODEL_DIMM: case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: if (qemuAssignMemoryDeviceSlot(def->mems[i], slotmap) < 0) - goto cleanup; + return -1; break; case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: @@ -3116,12 +3115,7 @@ qemuDomainAssignMemorySlots(virDomainDef *def) } } - ret = 0; - - cleanup: - virBitmapFree(slotmap); - return ret; - + return 0; } -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 50 ++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8093b8f69b..79cfbc401c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1480,13 +1480,12 @@ qemuDomainHelperGetVcpus(virDomainObj *vm, if (cpumaps) { unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, ncpuinfo); - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; if (!(map = virProcessGetAffinity(vcpupid))) return -1; virBitmapToDataBuf(map, cpumap, maplen); - virBitmapFree(map); } if (cpuwait) { @@ -4501,7 +4500,7 @@ qemuDomainPinVcpuLive(virDomainObj *vm, virQEMUDriver *driver, virBitmap *cpumap) { - virBitmap *tmpmap = NULL; + g_autoptr(virBitmap) tmpmap = NULL; virDomainVcpuDef *vcpuinfo; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virCgroup) cgroup_vcpu = NULL; @@ -4547,8 +4546,7 @@ qemuDomainPinVcpuLive(virDomainObj *vm, } virBitmapFree(vcpuinfo->cpumask); - vcpuinfo->cpumask = tmpmap; - tmpmap = NULL; + vcpuinfo->cpumask = g_steal_pointer(&tmpmap); qemuDomainSaveStatus(vm); @@ -4566,7 +4564,6 @@ qemuDomainPinVcpuLive(virDomainObj *vm, ret = 0; cleanup: - virBitmapFree(tmpmap); virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -4584,7 +4581,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virDomainDef *def; virDomainDef *persistentDef; int ret = -1; - virBitmap *pcpumap = NULL; + g_autoptr(virBitmap) pcpumap = NULL; virDomainVcpuDef *vcpuinfo = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; @@ -4628,8 +4625,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (persistentDef) { virBitmapFree(vcpuinfo->cpumask); - vcpuinfo->cpumask = pcpumap; - pcpumap = NULL; + vcpuinfo->cpumask = g_steal_pointer(&pcpumap); ret = virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir); goto endjob; @@ -4642,7 +4638,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virBitmapFree(pcpumap); return ret; } @@ -4708,7 +4703,7 @@ qemuDomainPinEmulator(virDomainPtr dom, virDomainDef *persistentDef; int ret = -1; qemuDomainObjPrivate *priv; - virBitmap *pcpumap = NULL; + g_autoptr(virBitmap) pcpumap = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; virObjectEvent *event = NULL; g_autofree char *str = NULL; @@ -4791,7 +4786,6 @@ qemuDomainPinEmulator(virDomainPtr dom, cleanup: virObjectEventStateQueue(driver->domainEventState, event); - virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; } @@ -5017,7 +5011,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriver *driver, info_ret = g_new0(virDomainIOThreadInfoPtr, niothreads); for (i = 0; i < niothreads; i++) { - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; info_ret[i] = g_new0(virDomainIOThreadInfo, 1); info_ret[i]->iothread_id = iothreads[i]->iothread_id; @@ -5025,12 +5019,8 @@ qemuDomainGetIOThreadsLive(virQEMUDriver *driver, if (!(map = virProcessGetAffinity(iothreads[i]->thread_id))) goto endjob; - if (virBitmapToData(map, &info_ret[i]->cpumap, - &info_ret[i]->cpumaplen) < 0) { - virBitmapFree(map); + if (virBitmapToData(map, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen) < 0) goto endjob; - } - virBitmapFree(map); } *info = g_steal_pointer(&info_ret); @@ -5100,7 +5090,7 @@ qemuDomainPinIOThread(virDomainPtr dom, virDomainObj *vm; virDomainDef *def; virDomainDef *persistentDef; - virBitmap *pcpumap = NULL; + g_autoptr(virBitmap) pcpumap = NULL; qemuDomainObjPrivate *priv; g_autoptr(virCgroup) cgroup_iothread = NULL; virObjectEvent *event = NULL; @@ -5214,7 +5204,6 @@ qemuDomainPinIOThread(virDomainPtr dom, cleanup: virObjectEventStateQueue(driver->domainEventState, event); - virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; } @@ -8846,7 +8835,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, int ret = -1; g_autoptr(virQEMUDriverConfig) cfg = NULL; qemuDomainObjPrivate *priv; - virBitmap *nodeset = NULL; + g_autoptr(virBitmap) nodeset = NULL; virDomainNumatuneMemMode config_mode; int mode = -1; @@ -8953,7 +8942,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - virBitmapFree(nodeset); virDomainObjEndAPI(&vm); return ret; } @@ -16579,7 +16567,7 @@ qemuDomainGetCPUStats(virDomainPtr domain, virDomainObj *vm = NULL; int ret = -1; qemuDomainObjPrivate *priv; - virBitmap *guestvcpus = NULL; + g_autoptr(virBitmap) guestvcpus = NULL; virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); @@ -16611,7 +16599,6 @@ qemuDomainGetCPUStats(virDomainPtr domain, ret = virCgroupGetPercpuStats(priv->cgroup, params, nparams, start_cpu, ncpus, guestvcpus); cleanup: - virBitmapFree(guestvcpus); virDomainObjEndAPI(&vm); return ret; } @@ -19385,9 +19372,9 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, virTypedParameterPtr par = NULL; int npar = 0; int maxpar = 0; - virBitmap *vcpus = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); - virBitmap *online = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); - virBitmap *offlinable = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); + g_autoptr(virBitmap) vcpus = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); + g_autoptr(virBitmap) online = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); + g_autoptr(virBitmap) offlinable = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); g_autofree char *tmp = NULL; size_t i; int ret = -1; @@ -19424,9 +19411,6 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, ret = 0; cleanup: - virBitmapFree(vcpus); - virBitmapFree(online); - virBitmapFree(offlinable); virTypedParamsFree(par, npar); return ret; } @@ -19489,7 +19473,7 @@ qemuDomainSetGuestVcpus(virDomainPtr dom, { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; qemuAgentCPUInfo *info = NULL; qemuAgent *agent; int ninfo = 0; @@ -19562,7 +19546,6 @@ qemuDomainSetGuestVcpus(virDomainPtr dom, cleanup: VIR_FREE(info); - virBitmapFree(map); virDomainObjEndAPI(&vm); return ret; } @@ -19578,7 +19561,7 @@ qemuDomainSetVcpu(virDomainPtr dom, virDomainObj *vm = NULL; virDomainDef *def = NULL; virDomainDef *persistentDef = NULL; - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; ssize_t lastvcpu; int ret = -1; @@ -19635,7 +19618,6 @@ qemuDomainSetVcpu(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - virBitmapFree(map); virDomainObjEndAPI(&vm); return ret; } -- 2.31.1

Use automatic memory freeing for the 'ret' bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2e1d18c633..f307420ac1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6473,7 +6473,7 @@ qemuDomainSelectHotplugVcpuEntities(virDomainDef *def, unsigned int nvcpus, bool *enable) { - virBitmap *ret = NULL; + g_autoptr(virBitmap) ret = NULL; virDomainVcpuDef *vcpu; qemuDomainVcpuPrivate *vcpupriv; unsigned int maxvcpus = virDomainDefGetVcpusMax(def); @@ -6501,7 +6501,7 @@ qemuDomainSelectHotplugVcpuEntities(virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("target vm vcpu granularity does not allow the " "desired vcpu count")); - goto error; + return NULL; } ignore_value(virBitmapSetBit(ret, i)); @@ -6528,7 +6528,7 @@ qemuDomainSelectHotplugVcpuEntities(virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("target vm vcpu granularity does not allow the " "desired vcpu count")); - goto error; + return NULL; } ignore_value(virBitmapSetBit(ret, i)); @@ -6539,14 +6539,10 @@ qemuDomainSelectHotplugVcpuEntities(virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("failed to find appropriate hotpluggable vcpus to " "reach the desired target vcpu count")); - goto error; + return NULL; } - return ret; - - error: - virBitmapFree(ret); - return NULL; + return g_steal_pointer(&ret); } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f307420ac1..b4c8536b47 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6665,16 +6665,15 @@ qemuDomainSetVcpusInternal(virQEMUDriver *driver, bool hotpluggable) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virBitmap *vcpumap = NULL; + g_autoptr(virBitmap) vcpumap = NULL; bool enable; - int ret = -1; if (def && nvcpus > virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" " vcpus for the live domain: %u > %u"), nvcpus, virDomainDefGetVcpusMax(def)); - goto cleanup; + return -1; } if (persistentDef && nvcpus > virDomainDefGetVcpusMax(persistentDef)) { @@ -6682,30 +6681,26 @@ qemuDomainSetVcpusInternal(virQEMUDriver *driver, _("requested vcpus is greater than max allowable" " vcpus for the persistent domain: %u > %u"), nvcpus, virDomainDefGetVcpusMax(persistentDef)); - goto cleanup; + return -1; } if (def) { if (!(vcpumap = qemuDomainSelectHotplugVcpuEntities(vm->def, nvcpus, &enable))) - goto cleanup; + return -1; if (qemuDomainSetVcpusLive(driver, cfg, vm, vcpumap, enable) < 0) - goto cleanup; + return -1; } if (persistentDef) { qemuDomainSetVcpusConfig(persistentDef, nvcpus, hotpluggable); if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virBitmapFree(vcpumap); - return ret; + return 0; } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b4c8536b47..32728a196a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6845,19 +6845,18 @@ qemuDomainSetVcpuInternal(virQEMUDriver *driver, bool state) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virBitmap *livevcpus = NULL; - int ret = -1; + g_autoptr(virBitmap) livevcpus = NULL; if (def) { if (!qemuDomainSupportsNewVcpuHotplug(vm)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("this qemu version does not support specific " "vCPU hotplug")); - goto cleanup; + return -1; } if (!(livevcpus = qemuDomainFilterHotplugVcpuEntities(def, map, state))) - goto cleanup; + return -1; /* Make sure that only one hotpluggable entity is selected. * qemuDomainSetVcpusLive allows setting more at once but error @@ -6866,31 +6865,27 @@ qemuDomainSetVcpuInternal(virQEMUDriver *driver, if (virBitmapCountBits(livevcpus) != 1) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("only one hotpluggable entity can be selected")); - goto cleanup; + return -1; } } if (persistentDef) { if (qemuDomainVcpuValidateConfig(persistentDef, map) < 0) - goto cleanup; + return -1; } if (livevcpus && qemuDomainSetVcpusLive(driver, cfg, vm, livevcpus, state) < 0) - goto cleanup; + return -1; if (persistentDef) { qemuDomainSetVcpuConfig(persistentDef, map, state); if (virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virBitmapFree(livevcpus); - return ret; + return 0; } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6b83a571b9..804205131a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5954,8 +5954,7 @@ qemuProcessValidateHotpluggableVcpus(virDomainDef *def) unsigned int maxvcpus = virDomainDefGetVcpusMax(def); size_t i = 0; size_t j; - virBitmap *ordermap = virBitmapNew(maxvcpus + 1); - int ret = -1; + g_autoptr(virBitmap) ordermap = virBitmapNew(maxvcpus + 1); /* validate: * - all hotpluggable entities to be hotplugged have the correct data @@ -5974,14 +5973,14 @@ qemuProcessValidateHotpluggableVcpus(virDomainDef *def) if (virBitmapIsBitSet(ordermap, vcpu->order)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("duplicate vcpu order '%u'"), vcpu->order); - goto cleanup; + return -1; } if (virBitmapSetBit(ordermap, vcpu->order)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("vcpu order '%u' exceeds vcpu count"), vcpu->order); - goto cleanup; + return -1; } } @@ -5993,7 +5992,7 @@ qemuProcessValidateHotpluggableVcpus(virDomainDef *def) virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("vcpus '%zu' and '%zu' are in the same hotplug " "group but differ in configuration"), i, j); - goto cleanup; + return -1; } } @@ -6003,15 +6002,12 @@ qemuProcessValidateHotpluggableVcpus(virDomainDef *def) !vcpupriv->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("vcpu '%zu' is missing hotplug data"), i); - goto cleanup; + return -1; } } } - ret = 0; - cleanup: - virBitmapFree(ordermap); - return ret; + return 0; } -- 2.31.1

Automatically free 'cmd' and 'created' by moving them to the appropriate scopes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 1a32b15f51..9a5d3e60aa 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -181,23 +181,21 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, size_t i; virDomainSnapshotDiskDef *snapdisk; virDomainDiskDef *defdisk; - virCommand *cmd = NULL; const char *qemuImgPath; - virBitmap *created = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int ret = -1; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap); + g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks); if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) goto cleanup; - created = virBitmapNew(snapdef->ndisks); - /* If reuse is true, then qemuSnapshotPrepare already * ensured that the new files exist, and it was up to the user to * create them correctly. */ for (i = 0; i < snapdef->ndisks && !reuse; i++) { + g_autoptr(virCommand) cmd = NULL; snapdisk = &(snapdef->disks[i]); defdisk = vm->def->disks[i]; if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) @@ -234,9 +232,6 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, if (virCommandRun(cmd, NULL) < 0) goto cleanup; - - virCommandFree(cmd); - cmd = NULL; } /* update disk definitions */ @@ -272,8 +267,6 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, ret = 0; cleanup: - virCommandFree(cmd); - /* unlink images if creation has failed */ if (ret < 0 && created) { ssize_t bit = -1; @@ -284,7 +277,6 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, snapdisk->src->path); } } - virBitmapFree(created); return ret; } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 67b6910626..ad75eb5843 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -271,17 +271,12 @@ virHostCPUGetSiblingsList(unsigned int cpu) static unsigned long virHostCPUCountThreadSiblings(unsigned int cpu) { - virBitmap *siblings_map; - unsigned long ret = 0; + g_autoptr(virBitmap) siblings_map = NULL; if (!(siblings_map = virHostCPUGetSiblingsList(cpu))) - goto cleanup; - - ret = virBitmapCountBits(siblings_map); + return 0; - cleanup: - virBitmapFree(siblings_map); - return ret; + return virBitmapCountBits(siblings_map); } /* parses a node entry, returning number of processors in the node and -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index ad75eb5843..a8d8b34a39 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -450,31 +450,25 @@ virHostCPUParseNode(const char *node, static bool virHostCPUHasValidSubcoreConfiguration(int threads_per_subcore) { - virBitmap *online_cpus = NULL; + g_autoptr(virBitmap) online_cpus = NULL; int cpu = -1; - bool ret = false; /* No point in checking if subcores are not in use */ if (threads_per_subcore <= 0) - goto cleanup; + return false; if (!(online_cpus = virHostCPUGetOnlineBitmap())) - goto cleanup; + return false; while ((cpu = virBitmapNextSetBit(online_cpus, cpu)) >= 0) { /* A single online secondary thread is enough to * make the configuration invalid */ if (cpu % threads_per_subcore != 0) - goto cleanup; + return false; } - ret = true; - - cleanup: - virBitmapFree(online_cpus); - - return ret; + return true; } -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index a8d8b34a39..1920184f61 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -300,10 +300,10 @@ virHostCPUParseNode(const char *node, int processors = 0; g_autoptr(DIR) cpudir = NULL; struct dirent *cpudirent = NULL; - virBitmap *node_cpus_map = NULL; - virBitmap *sockets_map = NULL; + g_autoptr(virBitmap) sockets_map = virBitmapNew(0); virBitmap **cores_maps = NULL; int npresent_cpus = virBitmapSize(present_cpus_map); + g_autoptr(virBitmap) node_cpus_map = virBitmapNew(npresent_cpus); unsigned int sock_max = 0; unsigned int sock; unsigned int core; @@ -319,12 +319,6 @@ virHostCPUParseNode(const char *node, if (virDirOpen(&cpudir, node) < 0) goto cleanup; - /* Keep track of the CPUs that belong to the current node */ - node_cpus_map = virBitmapNew(npresent_cpus); - - /* enumerate sockets in the node */ - sockets_map = virBitmapNew(0); - while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) { if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) continue; @@ -437,8 +431,6 @@ virHostCPUParseNode(const char *node, for (i = 0; i < sock_max; i++) virBitmapFree(cores_maps[i]); VIR_FREE(cores_maps); - virBitmapFree(sockets_map); - virBitmapFree(node_cpus_map); return ret; } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/vircgroup.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 51da6e6a2f..bf8db47ad4 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2347,7 +2347,6 @@ virCgroupGetPercpuStats(virCgroup *group, unsigned int ncpus, virBitmap *guestvcpus) { - int ret = -1; size_t i; int need_cpus, total_cpus; char *pos; @@ -2356,7 +2355,7 @@ virCgroupGetPercpuStats(virCgroup *group, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; - virBitmap *cpumap = NULL; + g_autoptr(virBitmap) cpumap = NULL; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { @@ -2373,21 +2372,19 @@ virCgroupGetPercpuStats(virCgroup *group, total_cpus = virBitmapSize(cpumap); /* return total number of cpus */ - if (ncpus == 0) { - ret = total_cpus; - goto cleanup; - } + if (ncpus == 0) + return total_cpus; if (start_cpu >= total_cpus) { virReportError(VIR_ERR_INVALID_ARG, _("start_cpu %d larger than maximum of %d"), start_cpu, total_cpus - 1); - goto cleanup; + return -1; } /* we get percpu cputime accounting info. */ if (virCgroupGetCpuacctPercpuUsage(group, &buf)) - goto cleanup; + return -1; pos = buf; /* return percpu cputime in index 0 */ @@ -2402,14 +2399,14 @@ virCgroupGetPercpuStats(virCgroup *group, } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); - goto cleanup; + return -1; } if (i < start_cpu) continue; ent = ¶ms[(i - start_cpu) * nparams + param_idx]; if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) - goto cleanup; + return -1; } /* return percpu vcputime in index 1 */ @@ -2419,7 +2416,7 @@ virCgroupGetPercpuStats(virCgroup *group, sum_cpu_time = g_new0(unsigned long long, need_cpus); if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time, need_cpus, cpumap) < 0) - goto cleanup; + return -1; for (i = start_cpu; i < need_cpus; i++) { int idx = (i - start_cpu) * nparams + param_idx; @@ -2427,17 +2424,13 @@ virCgroupGetPercpuStats(virCgroup *group, VIR_DOMAIN_CPU_STATS_VCPUTIME, VIR_TYPED_PARAM_ULLONG, sum_cpu_time[i]) < 0) - goto cleanup; + return -1; } param_idx++; } - ret = param_idx; - - cleanup: - virBitmapFree(cpumap); - return ret; + return param_idx; } -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8379f9f135..2338d6522a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6983,7 +6983,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen, const char *cpulist, int maxcpu) { unsigned char *cpumap = NULL; - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; if (cpulist[0] == 'r') { map = virBitmapNew(maxcpu); @@ -6994,21 +6994,19 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen, if (virBitmapParse(cpulist, &map, 1024) < 0 || virBitmapIsAllClear(map)) { vshError(ctl, _("Invalid cpulist '%s'"), cpulist); - goto cleanup; + return NULL; } lastcpu = virBitmapLastSetBit(map); if (lastcpu >= maxcpu) { vshError(ctl, _("CPU %d in cpulist '%s' exceed the maxcpu %d"), lastcpu, cpulist, maxcpu); - goto cleanup; + return NULL; } } if (virBitmapToData(map, &cpumap, cpumaplen) < 0) - goto cleanup; + return NULL; - cleanup: - virBitmapFree(map); return cpumap; } -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virnumamock.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/virnumamock.c b/tests/virnumamock.c index 19c26d1e37..7f2653cb53 100644 --- a/tests/virnumamock.c +++ b/tests/virnumamock.c @@ -63,29 +63,23 @@ virNumaIsAvailable(void) int virNumaGetMaxNode(void) { - int ret = -1; - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; if (virFileReadValueBitmap(&map, "%s/node/online", SYSFS_SYSTEM_PATH) < 0) return -1; - ret = virBitmapLastSetBit(map); - virBitmapFree(map); - return ret; + return virBitmapLastSetBit(map); } bool virNumaNodeIsAvailable(int node) { - bool ret = false; - virBitmap *map = NULL; + g_autoptr(virBitmap) map = NULL; if (virFileReadValueBitmap(&map, "%s/node/online", SYSFS_SYSTEM_PATH) < 0) return false; - ret = virBitmapIsBitSet(map, node); - virBitmapFree(map); - return ret; + return virBitmapIsBitSet(map, node); } int -- 2.31.1

Use automatic memory freeing for the temporary bitmap and remove the pointless 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/vircapstest.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/tests/vircapstest.c b/tests/vircapstest.c index d1b5033844..697803fdc9 100644 --- a/tests/vircapstest.c +++ b/tests/vircapstest.c @@ -33,28 +33,21 @@ static int test_virCapabilitiesGetCpusForNodemask(const void *data G_GNUC_UNUSED) { const char *nodestr = "3,4,5,6"; - virBitmap *nodemask = NULL; - virBitmap *cpumap = NULL; + g_autoptr(virBitmap) nodemask = NULL; + g_autoptr(virBitmap) cpumap = NULL; g_autoptr(virCapsHostNUMA) caps = NULL; int mask_size = 8; - int ret = -1; if (!(caps = virTestCapsBuildNUMATopology(3))) - goto error; + return -1; if (virBitmapParse(nodestr, &nodemask, mask_size) < 0) - goto error; + return -1; if (!(cpumap = virCapabilitiesHostNUMAGetCpus(caps, nodemask))) - goto error; - - ret = 0; - - error: - virBitmapFree(nodemask); - virBitmapFree(cpumap); - return ret; + return -1; + return 0; } -- 2.31.1

The function isn't used besides tests. Since the separator parsing capability is trivial we can keep it in place and just unexport it for now. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 5 ----- tests/virbitmaptest.c | 2 +- 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7be5b51100..ac0a704750 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1851,7 +1851,6 @@ virBitmapNextClearBit; virBitmapNextSetBit; virBitmapOverlaps; virBitmapParse; -virBitmapParseSeparator; virBitmapParseUnlimited; virBitmapSetAll; virBitmapSetBit; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ee76fec8cd..5f14f1e5e0 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -397,7 +397,7 @@ virBitmapFormat(virBitmap *bitmap) * * Returns 0 on success, or -1 in case of error. */ -int +static int virBitmapParseSeparator(const char *str, char terminator, virBitmap **bitmap, diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 026659544e..48074d1ae6 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -81,11 +81,6 @@ int virBitmapParse(const char *str, virBitmap **bitmap, size_t bitmapSize) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int -virBitmapParseSeparator(const char *str, - char terminator, - virBitmap **bitmap, - size_t bitmapSize); virBitmap * virBitmapParseUnlimited(const char *str); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 02241c4c20..1794f73f34 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -497,7 +497,7 @@ test10(const void *opaque G_GNUC_UNUSED) g_autoptr(virBitmap) b3 = NULL; g_autoptr(virBitmap) b4 = NULL; - if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 || + if (virBitmapParse("0-3,5-8,11-15", &b1, 20) < 0 || virBitmapParse("4,9,10,16-19", &b2, 20) < 0 || virBitmapParse("15", &b3, 20) < 0 || virBitmapParse("0,^0", &b4, 20) < 0) -- 2.31.1

There's nothing that can fail in the function. Remove the return value and adjust callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5f14f1e5e0..2b885803fd 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -128,10 +128,8 @@ virBitmapSetBit(virBitmap *bitmap, * * Resizes the bitmap so that bit @b will fit into it. This shall be called only * if @b would not fit into the map. - * - * Returns 0 on success, -1 on error. */ -static int +static void virBitmapExpand(virBitmap *map, size_t b) { @@ -145,8 +143,6 @@ virBitmapExpand(virBitmap *map, map->nbits = b + 1; map->map_len = new_len; - - return 0; } @@ -164,8 +160,8 @@ int virBitmapSetBitExpand(virBitmap *bitmap, size_t b) { - if (bitmap->nbits <= b && virBitmapExpand(bitmap, b) < 0) - return -1; + if (bitmap->nbits <= b) + virBitmapExpand(bitmap, b); bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] |= VIR_BITMAP_BIT(b); return 0; @@ -208,8 +204,7 @@ virBitmapClearBitExpand(virBitmap *bitmap, size_t b) { if (bitmap->nbits <= b) { - if (virBitmapExpand(bitmap, b) < 0) - return -1; + virBitmapExpand(bitmap, b); } else { bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] &= ~VIR_BITMAP_BIT(b); } @@ -1178,10 +1173,8 @@ virBitmapUnion(virBitmap *a, { size_t i; - if (a->nbits < b->nbits && - virBitmapExpand(a, b->nbits - 1) < 0) { - return -1; - } + if (a->nbits < b->nbits) + virBitmapExpand(a, b->nbits - 1); for (i = 0; i < b->map_len; i++) a->map[i] |= b->map[i]; -- 2.31.1

The function can't fail at this point. Remove the return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 6 +----- src/util/virbitmap.h | 4 ++-- src/util/virnuma.c | 3 +-- tests/virbitmaptest.c | 3 +-- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 2b885803fd..c067b1f738 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -1164,10 +1164,8 @@ virBitmapIntersect(virBitmap *a, * @b: other bitmap * * Performs union of two bitmaps: a = union(a, b) - * - * Returns 0 on success, <0 on failure. */ -int +void virBitmapUnion(virBitmap *a, const virBitmap *b) { @@ -1178,8 +1176,6 @@ virBitmapUnion(virBitmap *a, for (i = 0; i < b->map_len; i++) a->map[i] |= b->map[i]; - - return 0; } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 48074d1ae6..23242b44ff 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -133,8 +133,8 @@ bool virBitmapOverlaps(virBitmap *b1, void virBitmapIntersect(virBitmap *a, virBitmap *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virBitmapUnion(virBitmap *a, - const virBitmap *b) +void virBitmapUnion(virBitmap *a, + const virBitmap *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); void virBitmapSubtract(virBitmap *a, virBitmap *b) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 31f65d8902..7c892d6267 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -333,8 +333,7 @@ virNumaNodesetToCPUset(virBitmap *nodeset, return -1; } - if (virBitmapUnion(allNodesCPUs, nodeCPUs) < 0) - return -1; + virBitmapUnion(allNodesCPUs, nodeCPUs); } *cpuset = g_steal_pointer(&allNodesCPUs); diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1794f73f34..4c525679f6 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -665,8 +665,7 @@ test15(const void *opaque) return -1; } - if (virBitmapUnion(amap, bmap) < 0) - return -1; + virBitmapUnion(amap, bmap); if (!virBitmapEqual(amap, resmap)) { fprintf(stderr, -- 2.31.1

The function can't fail at this point. Remove the return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 9 ++------- src/util/virbitmap.h | 4 ++-- tests/virbitmaptest.c | 3 +-- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index c067b1f738..ecc4b96a57 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -196,10 +196,8 @@ virBitmapClearBit(virBitmap *bitmap, * * Clear bit position @b in @bitmap. Expands the bitmap as necessary so that * @b is included in the map. - * - * Returns 0 on if bit is successfully cleared, -1 on error. */ -int +void virBitmapClearBitExpand(virBitmap *bitmap, size_t b) { @@ -208,8 +206,6 @@ virBitmapClearBitExpand(virBitmap *bitmap, } else { bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] &= ~VIR_BITMAP_BIT(b); } - - return 0; } @@ -573,8 +569,7 @@ virBitmapParseUnlimited(const char *str) if (*cur == ',' || *cur == 0) { if (neg) { - if (virBitmapClearBitExpand(bitmap, start) < 0) - goto error; + virBitmapClearBitExpand(bitmap, start); } else { if (virBitmapSetBitExpand(bitmap, start) < 0) goto error; diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 23242b44ff..08ada97aa1 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -54,8 +54,8 @@ int virBitmapSetBitExpand(virBitmap *bitmap, size_t b) int virBitmapClearBit(virBitmap *bitmap, size_t b) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -int virBitmapClearBitExpand(virBitmap *bitmap, size_t b) - ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; +void virBitmapClearBitExpand(virBitmap *bitmap, size_t b) + ATTRIBUTE_NONNULL(1); /* * Get bit @b in @bitmap. Returns false if b is out of range. diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 4c525679f6..f16df4d2b2 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -564,8 +564,7 @@ test12a(const void *opaque G_GNUC_UNUSED) if (checkBitmap(map, "128", 129) < 0) return -1; - if (virBitmapClearBitExpand(map, 150) < 0) - return -1; + virBitmapClearBitExpand(map, 150); if (checkBitmap(map, "128", 151) < 0) return -1; -- 2.31.1

The function can't fail at this point. Remove the return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 3 +-- src/network/bridge_driver.c | 3 +-- src/util/virbitmap.c | 14 ++++---------- src/util/virbitmap.h | 4 ++-- src/util/virhostcpu.c | 6 ++---- src/util/virqemu.c | 3 +-- src/util/virtpm.c | 3 +-- tests/testutils.c | 2 +- tests/virbitmaptest.c | 7 +++---- 9 files changed, 16 insertions(+), 29 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 3bc1f63a5d..32821bf6a3 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1149,8 +1149,7 @@ virDomainNumaDefValidate(const virDomainNuma *def) return -1; } - if (virBitmapSetBitExpand(levelsSeen, cache->level)) - return -1; + virBitmapSetBitExpand(levelsSeen, cache->level); } } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7e5fab630b..40dccf2c15 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4966,8 +4966,7 @@ networkNextClassID(virNetworkObj *obj) if ((ret = virBitmapNextClearBit(classIdMap, -1)) < 0) ret = virBitmapSize(classIdMap); - if (virBitmapSetBitExpand(classIdMap, ret) < 0) - return -1; + virBitmapSetBitExpand(classIdMap, ret); return ret; } diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ecc4b96a57..a3f674eb19 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -153,10 +153,8 @@ virBitmapExpand(virBitmap *map, * * Set bit position @b in @bitmap. Expands the bitmap as necessary so that @b is * included in the map. - * - * Returns 0 on if bit is successfully set, -1 on error. */ -int +void virBitmapSetBitExpand(virBitmap *bitmap, size_t b) { @@ -164,7 +162,6 @@ virBitmapSetBitExpand(virBitmap *bitmap, virBitmapExpand(bitmap, b); bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] |= VIR_BITMAP_BIT(b); - return 0; } @@ -571,8 +568,7 @@ virBitmapParseUnlimited(const char *str) if (neg) { virBitmapClearBitExpand(bitmap, start); } else { - if (virBitmapSetBitExpand(bitmap, start) < 0) - goto error; + virBitmapSetBitExpand(bitmap, start); } } else if (*cur == '-') { if (neg) @@ -588,10 +584,8 @@ virBitmapParseUnlimited(const char *str) cur = tmp; - for (i = start; i <= last; i++) { - if (virBitmapSetBitExpand(bitmap, i) < 0) - goto error; - } + for (i = start; i <= last; i++) + virBitmapSetBitExpand(bitmap, i); virSkipSpaces(&cur); } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 08ada97aa1..e2314904b0 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -44,8 +44,8 @@ void virBitmapFree(virBitmap *bitmap); int virBitmapSetBit(virBitmap *bitmap, size_t b) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -int virBitmapSetBitExpand(virBitmap *bitmap, size_t b) - ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; +void virBitmapSetBitExpand(virBitmap *bitmap, size_t b) + ATTRIBUTE_NONNULL(1); /* diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 1920184f61..83e3a5d0f1 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -336,8 +336,7 @@ virHostCPUParseNode(const char *node, if (virHostCPUGetSocket(cpu, &sock) < 0) goto cleanup; - if (virBitmapSetBitExpand(sockets_map, sock) < 0) - goto cleanup; + virBitmapSetBitExpand(sockets_map, sock); if (sock > sock_max) sock_max = sock; @@ -396,8 +395,7 @@ virHostCPUParseNode(const char *node, goto cleanup; } - if (virBitmapSetBitExpand(cores_maps[sock], core) < 0) - goto cleanup; + virBitmapSetBitExpand(cores_maps[sock], core); if (!(siblings = virHostCPUCountThreadSiblings(cpu))) goto cleanup; diff --git a/src/util/virqemu.c b/src/util/virqemu.c index aef6006dd7..8ff61b22b0 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -70,8 +70,7 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key, if (virJSONValueGetNumberUlong(member, &value) < 0) return -1; - if (virBitmapSetBitExpand(bitmap, value) < 0) - return -1; + virBitmapSetBitExpand(bitmap, value); } while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 63579b8e69..cf4c7c458f 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -251,8 +251,7 @@ virTPMExecGetCaps(virCommand *cmd, if (typ < 0) continue; - if (virBitmapSetBitExpand(bitmap, typ) < 0) - return bitmap; + virBitmapSetBitExpand(bitmap, typ); } return bitmap; diff --git a/tests/testutils.c b/tests/testutils.c index e460c9ecc5..65f02e0231 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -173,7 +173,7 @@ virTestRun(const char *title, } if (ret != 0 && ret != EXIT_AM_SKIP) - ignore_value(virBitmapSetBitExpand(failedTests, testCounter)); + virBitmapSetBitExpand(failedTests, testCounter); g_unsetenv("VIR_TEST_MOCK_TESTNAME"); return ret; diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index f16df4d2b2..89dc702da2 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -558,8 +558,7 @@ test12a(const void *opaque G_GNUC_UNUSED) if (checkBitmap(map, "", 0) < 0) return -1; - if (virBitmapSetBitExpand(map, 128) < 0) - return -1; + virBitmapSetBitExpand(map, 128); if (checkBitmap(map, "128", 129) < 0) return -1; @@ -692,8 +691,8 @@ test16(const void *opaque G_GNUC_UNUSED) return -1; } - ignore_value(virBitmapSetBitExpand(map, 2)); - ignore_value(virBitmapSetBitExpand(map, 11)); + virBitmapSetBitExpand(map, 2); + virBitmapSetBitExpand(map, 11); if (!(res_set = virBitmapToString(map)) || STRNEQ_NULLABLE(res_set, "804")) { -- 2.31.1

Since the feature is not needed remove it and remove the function to virBitmapParseInternal. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index a3f674eb19..1aaefba133 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -364,9 +364,8 @@ virBitmapFormat(virBitmap *bitmap) /** - * virBitmapParseSeparator: + * virBitmapParseInternal: * @str: points to a string representing a human-readable bitmap - * @terminator: character separating the bitmap to parse * @bitmap: a bitmap created from @str * @bitmapSize: the upper limit of num of bits in created bitmap * @@ -377,19 +376,12 @@ virBitmapFormat(virBitmap *bitmap) * to set, and ^N, which means to unset the bit, and N-M for ranges of bits * to set. * - * To allow parsing of bitmaps within larger strings it is possible to set - * a termination character in the argument @terminator. When the character - * in @terminator is encountered in @str, the parsing of the bitmap stops. - * Pass 0 as @terminator if it is not needed. Whitespace characters may not - * be used as terminators. - * * Returns 0 on success, or -1 in case of error. */ static int -virBitmapParseSeparator(const char *str, - char terminator, - virBitmap **bitmap, - size_t bitmapSize) +virBitmapParseInternal(const char *str, + virBitmap **bitmap, + size_t bitmapSize) { bool neg = false; const char *cur = str; @@ -407,7 +399,7 @@ virBitmapParseSeparator(const char *str, if (*cur == '\0') goto error; - while (*cur != 0 && *cur != terminator) { + while (*cur != 0) { /* * 3 constructs are allowed: * - N : a single CPU number @@ -431,7 +423,7 @@ virBitmapParseSeparator(const char *str, virSkipSpaces(&cur); - if (*cur == ',' || *cur == 0 || *cur == terminator) { + if (*cur == ',' || *cur == 0) { if (neg) { if (virBitmapClearBit(*bitmap, start) < 0) goto error; @@ -465,7 +457,7 @@ virBitmapParseSeparator(const char *str, cur++; virSkipSpaces(&cur); neg = false; - } else if (*cur == 0 || *cur == terminator) { + } else if (*cur == 0) { break; } else { goto error; @@ -503,7 +495,7 @@ virBitmapParse(const char *str, virBitmap **bitmap, size_t bitmapSize) { - return virBitmapParseSeparator(str, '\0', bitmap, bitmapSize); + return virBitmapParseInternal(str, bitmap, bitmapSize); } -- 2.31.1

In order to prepare for reuse of the function, move the allocation of the bitmap to the caller. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1aaefba133..4047870e04 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -366,8 +366,7 @@ virBitmapFormat(virBitmap *bitmap) /** * virBitmapParseInternal: * @str: points to a string representing a human-readable bitmap - * @bitmap: a bitmap created from @str - * @bitmapSize: the upper limit of num of bits in created bitmap + * @bitmap: a bitmap populated from @str * * This function is the counterpart of virBitmapFormat. This function creates * a bitmap, in which bits are set according to the content of @str. @@ -380,8 +379,7 @@ virBitmapFormat(virBitmap *bitmap) */ static int virBitmapParseInternal(const char *str, - virBitmap **bitmap, - size_t bitmapSize) + virBitmap *bitmap) { bool neg = false; const char *cur = str; @@ -389,8 +387,6 @@ virBitmapParseInternal(const char *str, size_t i; int start, last; - *bitmap = virBitmapNew(bitmapSize); - if (!str) goto error; @@ -425,10 +421,10 @@ virBitmapParseInternal(const char *str, if (*cur == ',' || *cur == 0) { if (neg) { - if (virBitmapClearBit(*bitmap, start) < 0) + if (virBitmapClearBit(bitmap, start) < 0) goto error; } else { - if (virBitmapSetBit(*bitmap, start) < 0) + if (virBitmapSetBit(bitmap, start) < 0) goto error; } } else if (*cur == '-') { @@ -446,7 +442,7 @@ virBitmapParseInternal(const char *str, cur = tmp; for (i = start; i <= last; i++) { - if (virBitmapSetBit(*bitmap, i) < 0) + if (virBitmapSetBit(bitmap, i) < 0) goto error; } @@ -469,8 +465,6 @@ virBitmapParseInternal(const char *str, error: virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse bitmap '%s'"), str); - virBitmapFree(*bitmap); - *bitmap = NULL; return -1; } @@ -495,7 +489,14 @@ virBitmapParse(const char *str, virBitmap **bitmap, size_t bitmapSize) { - return virBitmapParseInternal(str, bitmap, bitmapSize); + g_autoptr(virBitmap) tmp = virBitmapNew(bitmapSize); + + if (virBitmapParseInternal(str, tmp) < 0) + return -1; + + *bitmap = g_steal_pointer(&tmp); + + return 0; } -- 2.31.1

There were two separate instances of string->virBitmap code: virBitmapParseInternal and virBitmapParseUnlimited. By adding a flag to switch to expanding APIs we can merge the two implementations into one. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 115 ++++++++++--------------------------------- 1 file changed, 26 insertions(+), 89 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 4047870e04..5b9204cbd7 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -367,6 +367,7 @@ virBitmapFormat(virBitmap *bitmap) * virBitmapParseInternal: * @str: points to a string representing a human-readable bitmap * @bitmap: a bitmap populated from @str + * @limited: Don't use self-expanding APIs, report error if bit exceeds bitmap size * * This function is the counterpart of virBitmapFormat. This function creates * a bitmap, in which bits are set according to the content of @str. @@ -379,7 +380,8 @@ virBitmapFormat(virBitmap *bitmap) */ static int virBitmapParseInternal(const char *str, - virBitmap *bitmap) + virBitmap *bitmap, + bool limited) { bool neg = false; const char *cur = str; @@ -421,11 +423,19 @@ virBitmapParseInternal(const char *str, if (*cur == ',' || *cur == 0) { if (neg) { - if (virBitmapClearBit(bitmap, start) < 0) - goto error; + if (limited) { + if (virBitmapClearBit(bitmap, start) < 0) + goto error; + } else { + virBitmapClearBitExpand(bitmap, start); + } } else { - if (virBitmapSetBit(bitmap, start) < 0) - goto error; + if (limited) { + if (virBitmapSetBit(bitmap, start) < 0) + goto error; + } else { + virBitmapSetBitExpand(bitmap, start); + } } } else if (*cur == '-') { if (neg) @@ -442,8 +452,12 @@ virBitmapParseInternal(const char *str, cur = tmp; for (i = start; i <= last; i++) { - if (virBitmapSetBit(bitmap, i) < 0) - goto error; + if (limited) { + if (virBitmapSetBit(bitmap, i) < 0) + goto error; + } else { + virBitmapSetBitExpand(bitmap, i); + } } virSkipSpaces(&cur); @@ -491,7 +505,7 @@ virBitmapParse(const char *str, { g_autoptr(virBitmap) tmp = virBitmapNew(bitmapSize); - if (virBitmapParseInternal(str, tmp) < 0) + if (virBitmapParseInternal(str, tmp, true) < 0) return -1; *bitmap = g_steal_pointer(&tmp); @@ -518,89 +532,12 @@ virBitmapParse(const char *str, virBitmap * virBitmapParseUnlimited(const char *str) { - virBitmap *bitmap = virBitmapNew(0); - bool neg = false; - const char *cur = str; - char *tmp; - size_t i; - int start, last; - - if (!str) - goto error; - - virSkipSpaces(&cur); - - if (*cur == '\0') - goto error; - - while (*cur != 0) { - /* - * 3 constructs are allowed: - * - N : a single CPU number - * - N-M : a range of CPU numbers with N < M - * - ^N : remove a single CPU number from the current set - */ - if (*cur == '^') { - cur++; - neg = true; - } - - if (!g_ascii_isdigit(*cur)) - goto error; - - if (virStrToLong_i(cur, &tmp, 10, &start) < 0) - goto error; - if (start < 0) - goto error; - - cur = tmp; - - virSkipSpaces(&cur); + g_autoptr(virBitmap) tmp = virBitmapNew(0); - if (*cur == ',' || *cur == 0) { - if (neg) { - virBitmapClearBitExpand(bitmap, start); - } else { - virBitmapSetBitExpand(bitmap, start); - } - } else if (*cur == '-') { - if (neg) - goto error; - - cur++; - virSkipSpaces(&cur); - - if (virStrToLong_i(cur, &tmp, 10, &last) < 0) - goto error; - if (last < start) - goto error; - - cur = tmp; - - for (i = start; i <= last; i++) - virBitmapSetBitExpand(bitmap, i); - - virSkipSpaces(&cur); - } - - if (*cur == ',') { - cur++; - virSkipSpaces(&cur); - neg = false; - } else if (*cur == 0) { - break; - } else { - goto error; - } - } - - return bitmap; + if (virBitmapParseInternal(str, tmp, false) < 0) + return NULL; - error: - virReportError(VIR_ERR_INVALID_ARG, - _("Failed to parse bitmap '%s'"), NULLSTR(str)); - virBitmapFree(bitmap); - return NULL; + return g_steal_pointer(&tmp); } -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7ad6e15402..995e63b004 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2826,7 +2826,7 @@ testDomainPinEmulator(virDomainPtr dom, { virDomainObj *vm = NULL; virDomainDef *def = NULL; - virBitmap *pcpumap = NULL; + g_autoptr(virBitmap) pcpumap = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -2848,11 +2848,10 @@ testDomainPinEmulator(virDomainPtr dom, } virBitmapFree(def->cputune.emulatorpin); - def->cputune.emulatorpin = virBitmapNewCopy(pcpumap); + def->cputune.emulatorpin = g_steal_pointer(&pcpumap); ret = 0; cleanup: - virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); return ret; } @@ -2868,7 +2867,7 @@ testDomainGetEmulatorPinInfo(virDomainPtr dom, virDomainObj *vm = NULL; virDomainDef *def = NULL; virBitmap *cpumask = NULL; - virBitmap *bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; int hostcpus; int ret = -1; @@ -2898,7 +2897,6 @@ testDomainGetEmulatorPinInfo(virDomainPtr dom, ret = 1; cleanup: virDomainObjEndAPI(&vm); - virBitmapFree(bitmap); return ret; } @@ -3049,7 +3047,7 @@ static int testDomainGetVcpus(virDomainPtr domain, int hostcpus; int ret = -1; unsigned long long statbase; - virBitmap *allcpumap = NULL; + g_autoptr(virBitmap) allcpumap = NULL; if (!(privdom = testDomObjFromDomain(domain))) return -1; @@ -3102,7 +3100,6 @@ static int testDomainGetVcpus(virDomainPtr domain, ret = maxinfo; cleanup: - virBitmapFree(allcpumap); virDomainObjEndAPI(&privdom); return ret; } @@ -3523,7 +3520,7 @@ testDomainSetNumaParameters(virDomainPtr dom, { virDomainObj *vm = NULL; virDomainDef *def = NULL; - virBitmap *nodeset = NULL; + g_autoptr(virBitmap) nodeset = NULL; virDomainNumatuneMemMode config_mode; bool live; size_t i; @@ -3590,7 +3587,6 @@ testDomainSetNumaParameters(virDomainPtr dom, ret = 0; cleanup: - virBitmapFree(nodeset); virDomainObjEndAPI(&vm); return ret; } @@ -9669,7 +9665,7 @@ testDomainPinIOThread(virDomainPtr dom, virDomainObj *vm; virDomainDef *def; virDomainIOThreadIDDef *iothrid; - virBitmap *cpumask = NULL; + g_autoptr(virBitmap) cpumask = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -9702,7 +9698,6 @@ testDomainPinIOThread(virDomainPtr dom, ret = 0; cleanup: - virBitmapFree(cpumask); virDomainObjEndAPI(&vm); return ret; } -- 2.31.1

Use automatic memory clearing for virBitmap and remove a reuse of a temporary string. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_native.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 972e9cdc15..e9b58cbd1a 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -926,32 +926,28 @@ lxcSetCpuTune(virDomainDef *def, virConf *properties) static int lxcSetCpusetTune(virDomainDef *def, virConf *properties) { - g_autofree char *value = NULL; - virBitmap *nodeset = NULL; + g_autofree char *cpus = NULL; + g_autofree char *mems = NULL; + g_autoptr(virBitmap) nodeset = NULL; if (virConfGetValueString(properties, "lxc.cgroup.cpuset.cpus", - &value) > 0) { - if (virBitmapParse(value, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + &cpus) > 0) { + if (virBitmapParse(cpus, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC; - g_free(value); - value = NULL; } if (virConfGetValueString(properties, "lxc.cgroup.cpuset.mems", - &value) > 0) { - if (virBitmapParse(value, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) + &mems) > 0) { + if (virBitmapParse(mems, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) return -1; if (virDomainNumatuneSet(def->numa, def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC, VIR_DOMAIN_NUMATUNE_MEM_STRICT, - nodeset) < 0) { - virBitmapFree(nodeset); + nodeset) < 0) return -1; - } - virBitmapFree(nodeset); } return 0; -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/libxl_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0b14b5093e..ef31f2cdcf 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2416,7 +2416,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, libxlDriverPrivate *driver = dom->conn->privateData; libxlDriverConfig *cfg = libxlDriverConfigGet(driver); virDomainDef *targetDef = NULL; - virBitmap *pcpumap = NULL; + g_autoptr(virBitmap) pcpumap = NULL; virDomainVcpuDef *vcpuinfo; virDomainObj *vm; int ret = -1; @@ -2477,7 +2477,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, cleanup: virDomainObjEndAPI(&vm); - virBitmapFree(pcpumap); virObjectUnref(cfg); return ret; } @@ -4808,7 +4807,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom, libxlDriverConfig *cfg = libxlDriverConfigGet(driver); virDomainObj *vm; libxl_bitmap nodemap; - virBitmap *nodes = NULL; + g_autoptr(virBitmap) nodes = NULL; int rc, ret = -1; size_t i, j; @@ -4907,7 +4906,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom, ret = 0; cleanup: - virBitmapFree(nodes); libxl_bitmap_dispose(&nodemap); virDomainObjEndAPI(&vm); virObjectUnref(cfg); -- 2.31.1

The bitmap is allocated just above the explicit clear, so it's already empty. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libxl/libxl_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ef31f2cdcf..bc8598ea96 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -4879,7 +4879,6 @@ libxlDomainGetNumaParameters(virDomainPtr dom, /* First, we convert libxl_bitmap into virBitmap. After that, * we format virBitmap as a string that can be returned. */ - virBitmapClearAll(nodes); libxl_for_each_set_bit(j, nodemap) { if (virBitmapSetBit(nodes, j)) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_controller.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 039efcd7c7..ae1f090077 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -731,7 +731,7 @@ static int virLXCControllerSetupLoopDevices(virLXCController *ctrl) static int virLXCControllerSetupCpuAffinity(virLXCController *ctrl) { int hostcpus, maxcpu = CPU_SETSIZE; - virBitmap *cpumap; + g_autoptr(virBitmap) cpumap = NULL; virBitmap *cpumapToSet; VIR_DEBUG("Setting CPU affinity"); @@ -761,11 +761,8 @@ static int virLXCControllerSetupCpuAffinity(virLXCController *ctrl) * so use '0' to indicate our own process ID. No threads are * running at this point */ - if (virProcessSetAffinity(0 /* Self */, cpumapToSet, false) < 0) { - virBitmapFree(cpumap); + if (virProcessSetAffinity(0 /* Self */, cpumapToSet, false) < 0) return -1; - } - virBitmapFree(cpumap); return 0; } @@ -810,7 +807,7 @@ static int virLXCControllerGetNumadAdvice(virLXCController *ctrl, */ static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) { - virBitmap *auto_nodeset = NULL; + g_autoptr(virBitmap) auto_nodeset = NULL; int ret = -1; virBitmap *nodeset = NULL; virDomainNumatuneMemMode mode; @@ -841,7 +838,6 @@ static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) ret = 0; cleanup: - virBitmapFree(auto_nodeset); return ret; } @@ -852,7 +848,7 @@ static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) */ static int virLXCControllerSetupCgroupLimits(virLXCController *ctrl) { - virBitmap *auto_nodeset = NULL; + g_autoptr(virBitmap) auto_nodeset = NULL; int ret = -1; virBitmap *nodeset = NULL; size_t i; @@ -884,7 +880,6 @@ static int virLXCControllerSetupCgroupLimits(virLXCController *ctrl) ret = 0; cleanup: - virBitmapFree(auto_nodeset); return ret; } -- 2.31.1

Remove the pointless cleanup sections. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_controller.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index ae1f090077..c577d6a427 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -808,7 +808,6 @@ static int virLXCControllerGetNumadAdvice(virLXCController *ctrl, static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) { g_autoptr(virBitmap) auto_nodeset = NULL; - int ret = -1; virBitmap *nodeset = NULL; virDomainNumatuneMemMode mode; @@ -824,21 +823,19 @@ static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) VIR_DEBUG("Setting up process resource limits"); if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0) - goto cleanup; + return -1; nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1); if (virNumaSetupMemoryPolicy(mode, nodeset) < 0) - goto cleanup; + return -1; } } if (virLXCControllerSetupCpuAffinity(ctrl) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } @@ -849,14 +846,13 @@ static int virLXCControllerSetupResourceLimits(virLXCController *ctrl) static int virLXCControllerSetupCgroupLimits(virLXCController *ctrl) { g_autoptr(virBitmap) auto_nodeset = NULL; - int ret = -1; virBitmap *nodeset = NULL; size_t i; VIR_DEBUG("Setting up cgroup resource limits"); if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0) - goto cleanup; + return -1; nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1); @@ -864,23 +860,21 @@ static int virLXCControllerSetupCgroupLimits(virLXCController *ctrl) getpid(), ctrl->nnicindexes, ctrl->nicindexes))) - goto cleanup; + return -1; if (virCgroupAddMachineProcess(ctrl->cgroup, ctrl->initpid) < 0) - goto cleanup; + return -1; /* Add all qemu-nbd tasks to the cgroup */ for (i = 0; i < ctrl->nnbdpids; i++) { if (virCgroupAddMachineProcess(ctrl->cgroup, ctrl->nbdpids[i]) < 0) - goto cleanup; + return -1; } if (virLXCCgroupSetup(ctrl->def, ctrl->cgroup, nodeset) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virt-host-validate-common.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index aba1bdf6cf..647ed7b48b 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -330,7 +330,7 @@ int virHostValidateCGroupControllers(const char *hvname G_GNUC_UNUSED, int virHostValidateIOMMU(const char *hvname, virHostValidateLevel level) { - virBitmap *flags; + g_autoptr(virBitmap) flags = NULL; struct stat sb; const char *bootarg = NULL; bool isAMD = false, isIntel = false; @@ -347,8 +347,6 @@ int virHostValidateIOMMU(const char *hvname, else if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SVM)) isAMD = true; - virBitmapFree(flags); - if (isIntel) { if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) { virHostMsgPass(); @@ -445,7 +443,7 @@ bool virHostKernelModuleIsLoaded(const char *module) int virHostValidateSecureGuests(const char *hvname, virHostValidateLevel level) { - virBitmap *flags; + g_autoptr(virBitmap) flags = NULL; bool hasFac158 = false; bool hasAMDSev = false; virArch arch = virArchFromHost(); @@ -460,8 +458,6 @@ int virHostValidateSecureGuests(const char *hvname, else if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SEV)) hasAMDSev = true; - virBitmapFree(flags); - virHostMsgCheck(hvname, "%s", _("for secure guest support")); if (ARCH_IS_S390(arch)) { if (hasFac158) { -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virt-host-validate-qemu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index d225a7e5c8..46ff1d2494 100644 --- a/tools/virt-host-validate-qemu.c +++ b/tools/virt-host-validate-qemu.c @@ -29,7 +29,7 @@ int virHostValidateQEMU(void) { - virBitmap *flags; + g_autoptr(virBitmap) flags = NULL; int ret = 0; bool hasHwVirt = false; bool hasVirtFlag = false; @@ -98,8 +98,6 @@ int virHostValidateQEMU(void) virHostMsgPass(); } - virBitmapFree(flags); - if (virHostValidateDeviceExists("QEMU", "/dev/vhost-net", VIR_HOST_VALIDATE_WARN, _("Load the 'vhost_net' module to improve performance " -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 83e3a5d0f1..54e2462a95 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -582,8 +582,8 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, unsigned int *cores, unsigned int *threads) { - virBitmap *present_cpus_map = NULL; - virBitmap *online_cpus_map = NULL; + g_autoptr(virBitmap) present_cpus_map = NULL; + g_autoptr(virBitmap) online_cpus_map = NULL; g_autoptr(DIR) nodedir = NULL; struct dirent *nodedirent = NULL; int nodecpus, nodecores, nodesockets, nodethreads, offline = 0; @@ -745,8 +745,6 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo, ret = 0; cleanup: - virBitmapFree(present_cpus_map); - virBitmapFree(online_cpus_map); VIR_FREE(sysfs_nodedir); VIR_FREE(sysfs_cpudir); return ret; -- 2.31.1