Modify virCgroupFree function signature to take a value of type virCgroupPtr instead of virCgroupPtr * as the parameter. Change the argument type in all calls to virCgroupFree function from virCgroupPtr * to virCgroupPtr. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/libvirt-lxc.c | 4 ++-- src/lxc/lxc_cgroup.c | 4 ++-- src/lxc/lxc_container.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_domain.c | 2 +- src/lxc/lxc_process.c | 10 ++++----- src/qemu/qemu_cgroup.c | 16 +++++++------- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 34 ++++++++++++++--------------- src/qemu/qemu_process.c | 2 +- src/util/vircgroup.c | 56 ++++++++++++++++++++++++------------------------ src/util/vircgroup.h | 2 +- tests/vircgrouptest.c | 42 ++++++++++++++++++------------------ 13 files changed, 88 insertions(+), 90 deletions(-) diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index c9f2146..12be893 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -309,12 +309,12 @@ int virDomainLxcEnterCGroup(virDomainPtr domain, if (virCgroupAddTask(cgroup, getpid()) < 0) goto error; - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return 0; error: virDispatchError(NULL); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return -1; } diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8e937ec..873c843 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -306,7 +306,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -515,7 +515,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, def->idmap.uidmap[0].target, def->idmap.gidmap[0].target, (1 << VIR_CGROUP_CONTROLLER_SYSTEMD)) < 0) { - virCgroupFree(&cgroup); + virCgroupFree(cgroup); cgroup = NULL; goto cleanup; } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 3a1b2d6..407214f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1815,7 +1815,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, cleanup: VIR_FREE(stateDir); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); VIR_FREE(sec_mount_options); return ret; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4e84391..7be45f8 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -296,7 +296,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) VIR_FREE(ctrl->nbdpids); VIR_FREE(ctrl->nsFDs); - virCgroupFree(&ctrl->cgroup); + virCgroupFree(ctrl->cgroup); /* This must always be the last thing to be closed */ VIR_FORCE_CLOSE(ctrl->handshakeFd); diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index b197f9d..eb0071d 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -172,7 +172,7 @@ virLXCDomainObjPrivateFree(void *data) { virLXCDomainObjPrivatePtr priv = data; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); virLXCDomainObjFreeJob(priv); VIR_FREE(priv); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 14502e1..8534051 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -220,7 +220,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, if (priv->cgroup) { virCgroupRemove(priv->cgroup); - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); } /* Get machined to terminate the machine as it may not have cleaned it @@ -1203,26 +1203,26 @@ int virLXCProcessStart(virConnectPtr conn, if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'cpuacct' cgroups controller mount")); return -1; } if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'devices' cgroups controller mount")); return -1; } if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'memory' cgroups controller mount")); return -1; } - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); if (vm->def->nconsoles == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 43e17d7..8a00ffc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -841,7 +841,7 @@ qemuSetupCpusetMems(virDomainObjPtr vm) ret = 0; cleanup: VIR_FREE(mem_mask); - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -920,7 +920,7 @@ qemuInitCgroup(virDomainObjPtr vm, if (!virCgroupAvailable()) goto done; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); if (!vm->def->resource) { virDomainResourceDefPtr res; @@ -1008,7 +1008,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto cleanup; VIR_FREE(nodeset); - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } for (i = 0; i < vm->def->niothreadids; i++) { @@ -1021,7 +1021,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto cleanup; VIR_FREE(nodeset); - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, @@ -1035,7 +1035,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) VIR_FREE(mem_mask); VIR_FREE(nodeset); virBitmapFree(all_nodes); - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return; error: @@ -1057,7 +1057,7 @@ qemuConnectCgroup(virDomainObjPtr vm) if (!virCgroupAvailable()) goto done; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); if (virCgroupNewDetectMachine(vm->def->name, "qemu", @@ -1203,7 +1203,7 @@ qemuSetupCgroupForExtDevices(virDomainObjPtr vm, ret = qemuExtDevicesSetupCgroup(driver, vm->def, cgroup_temp); cleanup: - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -1281,7 +1281,7 @@ qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data) if (!data) return; - virCgroupFree(&data->emulatorCgroup); + virCgroupFree(data->emulatorCgroup); VIR_FREE(data->emulatorMemMask); VIR_FREE(data); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9188302..26eeb8f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1920,7 +1920,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) virStringListFree(priv->qemuDevices); priv->qemuDevices = NULL; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); virPerfFree(priv->perf); priv->perf = NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8cdb04e..606fd72 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5075,7 +5075,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, cleanup: virBitmapFree(tmpmap); - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); VIR_FREE(str); virObjectEventStateQueue(driver->domainEventState, event); return ret; @@ -5312,8 +5312,7 @@ qemuDomainPinEmulator(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - if (cgroup_emulator) - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); virObjectEventStateQueue(driver->domainEventState, event); VIR_FREE(str); virBitmapFree(pcpumap); @@ -5794,8 +5793,7 @@ qemuDomainPinIOThread(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - if (cgroup_iothread) - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); virObjectEventStateQueue(driver->domainEventState, event); VIR_FREE(str); virBitmapFree(pcpumap); @@ -9855,7 +9853,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); @@ -9867,7 +9865,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } for (i = 0; i < vm->def->niothreadids; i++) { @@ -9876,13 +9874,13 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } ret = 0; cleanup: VIR_FREE(nodeset_str); - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -10304,13 +10302,13 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); } return 0; cleanup: - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); return -1; } @@ -10331,11 +10329,11 @@ qemuSetEmulatorBandwidthLive(virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return 0; cleanup: - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return -1; } @@ -10362,13 +10360,13 @@ qemuSetIOThreadsBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); } return 0; cleanup: - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); return -1; } @@ -10754,7 +10752,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, ret = 0; cleanup: - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); return ret; } @@ -10777,7 +10775,7 @@ qemuGetEmulatorBandwidthLive(virCgroupPtr cgroup, ret = 0; cleanup: - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return ret; } @@ -10813,7 +10811,7 @@ qemuGetIOThreadsBWLive(virDomainObjPtr vm, ret = 0; cleanup: - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a4b1f97..b3abaae 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2532,7 +2532,7 @@ qemuProcessSetupPid(virDomainObjPtr vm, if (cgroup) { if (ret < 0) virCgroupRemove(cgroup); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); } return ret; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index e810a3d..140b016 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1178,7 +1178,7 @@ virCgroupNew(pid_t pid, return 0; error: - virCgroupFree(group); + virCgroupFree(*group); *group = NULL; return -1; @@ -1379,8 +1379,8 @@ virCgroupNewPartition(const char *path, ret = 0; cleanup: if (ret != 0) - virCgroupFree(group); - virCgroupFree(&parent); + virCgroupFree(*group); + virCgroupFree(parent); VIR_FREE(parentPath); VIR_FREE(newPath); return ret; @@ -1447,7 +1447,7 @@ virCgroupNewDomainPartition(virCgroupPtr partition, if (virCgroupMakeGroup(partition, *group, create, VIR_CGROUP_MEM_HIERACHY) < 0) { virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); goto cleanup; } @@ -1509,7 +1509,7 @@ virCgroupNewThread(virCgroupPtr domain, if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); goto cleanup; } @@ -1550,7 +1550,7 @@ virCgroupNewDetectMachine(const char *name, true, machinename)) { VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", name, drivername); - virCgroupFree(group); + virCgroupFree(*group); return 0; } @@ -1603,7 +1603,7 @@ virCgroupNewMachineSystemd(const char *name, path = init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement; init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL; - virCgroupFree(&init); + virCgroupFree(init); if (!path || STREQ(path, "/") || path[0] != '/') { VIR_DEBUG("Systemd didn't setup its controller"); @@ -1635,13 +1635,13 @@ virCgroupNewMachineSystemd(const char *name, goto cleanup; if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) { - virCgroupFree(&tmp); + virCgroupFree(tmp); goto cleanup; } if (t) { *t = '/'; offset = t; - virCgroupFree(&parent); + virCgroupFree(parent); parent = tmp; } else { *group = tmp; @@ -1652,7 +1652,7 @@ virCgroupNewMachineSystemd(const char *name, if (virCgroupAddTask(*group, pidleader) < 0) { virErrorPtr saved = virSaveLastError(); virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); if (saved) { virSetError(saved); virFreeError(saved); @@ -1661,7 +1661,7 @@ virCgroupNewMachineSystemd(const char *name, ret = 0; cleanup: - virCgroupFree(&parent); + virCgroupFree(parent); VIR_FREE(path); return ret; } @@ -1708,7 +1708,7 @@ virCgroupNewMachineManual(const char *name, if (virCgroupAddTask(*group, pidleader) < 0) { virErrorPtr saved = virSaveLastError(); virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); if (saved) { virSetError(saved); virFreeError(saved); @@ -1719,7 +1719,7 @@ virCgroupNewMachineManual(const char *name, ret = 0; cleanup: - virCgroupFree(&parent); + virCgroupFree(parent); return ret; } @@ -1786,21 +1786,21 @@ virCgroupNewIgnoreError(void) * @group: The group structure to free */ void -virCgroupFree(virCgroupPtr *group) +virCgroupFree(virCgroupPtr group) { size_t i; - if (*group == NULL) + if (!group) return; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_FREE((*group)->controllers[i].mountPoint); - VIR_FREE((*group)->controllers[i].linkPoint); - VIR_FREE((*group)->controllers[i].placement); + VIR_FREE(group->controllers[i].mountPoint); + VIR_FREE(group->controllers[i].linkPoint); + VIR_FREE(group->controllers[i].placement); } - VIR_FREE((*group)->path); - VIR_FREE(*group); + VIR_FREE(group->path); + VIR_FREE(group); } @@ -2514,7 +2514,7 @@ virCgroupMemoryOnceInit(void) "memory.limit_in_bytes", &mem_unlimited)); cleanup: - virCgroupFree(&group); + virCgroupFree(group); virCgroupMemoryUnlimitedKB = mem_unlimited >> 10; } @@ -3158,13 +3158,13 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, sum_cpu_time[j] += tmp; } - virCgroupFree(&group_vcpu); + virCgroupFree(group_vcpu); VIR_FREE(buf); } ret = 0; cleanup: - virCgroupFree(&group_vcpu); + virCgroupFree(group_vcpu); VIR_FREE(buf); return ret; } @@ -3722,7 +3722,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, if (dormdir) virCgroupRemove(subgroup); - virCgroupFree(&subgroup); + virCgroupFree(subgroup); } if (direrr < 0) goto cleanup; @@ -3731,7 +3731,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, ret = killedAny ? 1 : 0; cleanup: - virCgroupFree(&subgroup); + virCgroupFree(subgroup); VIR_FREE(keypath); VIR_DIR_CLOSE(dp); return ret; @@ -4118,7 +4118,7 @@ virCgroupControllerAvailable(int controller) return ret; ret = virCgroupHasController(cgroup, controller); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -4250,7 +4250,7 @@ virCgroupNewIgnoreError(void) void -virCgroupFree(virCgroupPtr *group ATTRIBUTE_UNUSED) +virCgroupFree(virCgroupPtr group ATTRIBUTE_UNUSED) { virReportSystemError(ENXIO, "%s", _("Control groups not supported on this platform")); @@ -4915,7 +4915,7 @@ virCgroupDelThread(virCgroupPtr cgroup, /* Remove the offlined cgroup */ virCgroupRemove(new_cgroup); - virCgroupFree(&new_cgroup); + virCgroupFree(new_cgroup); } return 0; diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index d833927..e4ffd57 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -122,7 +122,7 @@ int virCgroupTerminateMachine(const char *name) bool virCgroupNewIgnoreError(void); -void virCgroupFree(virCgroupPtr *group); +void virCgroupFree(virCgroupPtr group); bool virCgroupHasController(virCgroupPtr cgroup, int controller); int virCgroupPathOfController(virCgroupPtr group, diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index be50f3e..e5190e3 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -198,7 +198,7 @@ testCgroupDetectMounts(const void *args) cleanup: VIR_FREE(mounts); VIR_FREE(parsed); - virCgroupFree(&group); + virCgroupFree(group); virBufferFreeAndReset(&buf); return result; } @@ -227,7 +227,7 @@ static int testCgroupNewForSelf(const void *args ATTRIBUTE_UNUSED) ret = validateCgroup(cgroup, "", mountsFull, links, placement); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -304,7 +304,7 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED) goto cleanup; } ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/virtualmachines", true, -1, &cgroup)) != 0) { fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); @@ -313,7 +313,7 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED) ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -353,7 +353,7 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED) } /* Should now work */ - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment/production cgroup: %d\n", -rv); goto cleanup; @@ -363,7 +363,7 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED) mountsFull, links, placementFull); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -402,14 +402,14 @@ static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED goto cleanup; } - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); goto cleanup; } /* Should now work */ - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user/production cgroup: %d\n", -rv); goto cleanup; @@ -419,7 +419,7 @@ static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED mountsFull, links, placementFull); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -455,8 +455,8 @@ static int testCgroupNewForPartitionDomain(const void *args ATTRIBUTE_UNUSED) ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement); cleanup: - virCgroupFree(&partitioncgroup); - virCgroupFree(&domaincgroup); + virCgroupFree(partitioncgroup); + virCgroupFree(domaincgroup); return ret; } @@ -506,10 +506,10 @@ static int testCgroupNewForPartitionDomainEscaped(const void *args ATTRIBUTE_UNU ret = validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement); cleanup: - virCgroupFree(&partitioncgroup3); - virCgroupFree(&partitioncgroup2); - virCgroupFree(&partitioncgroup1); - virCgroupFree(&domaincgroup); + virCgroupFree(partitioncgroup3); + virCgroupFree(partitioncgroup2); + virCgroupFree(partitioncgroup1); + virCgroupFree(domaincgroup); return ret; } @@ -535,7 +535,7 @@ static int testCgroupNewForSelfAllInOne(const void *args ATTRIBUTE_UNUSED) ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -563,7 +563,7 @@ static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED) ret = validateCgroup(cgroup, "", mountsLogind, linksLogind, placement); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -690,7 +690,7 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); VIR_FREE(params); return ret; } @@ -723,7 +723,7 @@ static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -773,7 +773,7 @@ static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -846,7 +846,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args ATTRIBUTE_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } -- 1.8.3.1

On Mon, 23 Jul 2018 at 16:29, Erik Skultety <eskultet(a)redhat.com&gt; wrote: I am assuming that you are referring to `group` variable. If so, then I cannot apply cleanup attribute to function parameters and `group` is one of them.

On Tue, 24 Jul 2018 at 11:33, Erik Skultety <eskultet(a)redhat.com&gt; wrote: You mean something like this? VIR_AUTOPTR(virCgroup) ptr = NULL; ... return 0; cleanup: VIR_STEAL_PTR(ptr, *group); return -1;

On Sat, Jul 21, 2018 at 05:36:35PM +0530, Sukrit Bhatnagar wrote: Usually, a sentence like ^this indicates, that the change itself should come separately. ^This should come as a separate patch. Patch doesn't compile, please make sure that each patch can pass make check && make syntax-check. ^This function should be introduced in the following patch. ^This should come together with the typedef above. Erik

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When variables of type virBufferPtr and virBufferEscapePairPtr are declared using VIR_AUTOPTR, the functions virBufferFreeAndReset and virBufferEscapePairFree, respectively, will be run automatically on them when they go out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virbuffer.c | 2 +- src/util/virbuffer.h | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 8076cd3..65f4a08 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -31,7 +31,6 @@ #define __VIR_BUFFER_C__ #include "virbuffer.h" -#include "viralloc.h" #include "virerror.h" #include "virstring.h" @@ -664,6 +663,7 @@ virBufferEscapePairFree(virBufferEscapePairPtr pair) VIR_FREE(pair); } +VIR_DEFINE_AUTOPTR_FUNC(virBufferEscapePair, virBufferEscapePairFree) /** diff --git a/src/util/virbuffer.h b/src/util/virbuffer.h index e95ee87..3b31060 100644 --- a/src/util/virbuffer.h +++ b/src/util/virbuffer.h @@ -23,10 +23,13 @@ #ifndef __VIR_BUFFER_H__ # define __VIR_BUFFER_H__ -# include "internal.h" - # include <stdarg.h> +# include "internal.h" + +# include "viralloc.h" + + /** * virBuffer: * @@ -119,4 +122,6 @@ int virBufferGetIndent(const virBuffer *buf, bool dynamic); void virBufferTrim(virBufferPtr buf, const char *trim, int len); void virBufferAddStr(virBufferPtr buf, const char *str); +VIR_DEFINE_AUTOPTR_FUNC(virBuffer, virBufferFreeAndReset) + #endif /* __VIR_BUFFER_H__ */ -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virbuffer.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 65f4a08..a35feb6 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -455,7 +455,8 @@ void virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) { int len; - char *escaped, *out; + VIR_AUTOFREE(char *) escaped = NULL; + char *out; const char *cur; const char forbidden_characters[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, @@ -532,7 +533,6 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) *out = 0; virBufferAsprintf(buf, format, escaped); - VIR_FREE(escaped); } /** @@ -611,7 +611,8 @@ virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str) { int len; - char *escaped, *out; + VIR_AUTOFREE(char *) escaped = NULL; + char *out; const char *cur; if ((format == NULL) || (buf == NULL) || (str == NULL)) @@ -643,7 +644,6 @@ virBufferEscape(virBufferPtr buf, char escape, const char *toescape, *out = 0; virBufferAsprintf(buf, format, escaped); - VIR_FREE(escaped); } @@ -687,7 +687,7 @@ virBufferEscapeN(virBufferPtr buf, { int len; size_t i; - char *escaped = NULL; + VIR_AUTOFREE(char *) escaped = NULL; char *out; const char *cur; virBufferEscapePair escapeItem; @@ -750,7 +750,6 @@ virBufferEscapeN(virBufferPtr buf, cleanup: va_end(ap); VIR_FREE(escapeList); - VIR_FREE(escaped); } @@ -815,7 +814,8 @@ void virBufferEscapeShell(virBufferPtr buf, const char *str) { int len; - char *escaped, *out; + VIR_AUTOFREE(char *) escaped = NULL; + char *out; const char *cur; if ((buf == NULL) || (str == NULL)) @@ -859,7 +859,6 @@ virBufferEscapeShell(virBufferPtr buf, const char *str) *out = 0; virBufferAdd(buf, escaped, -1); - VIR_FREE(escaped); } /** -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virbuffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index a35feb6..d2827f6 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -691,7 +691,7 @@ virBufferEscapeN(virBufferPtr buf, char *out; const char *cur; virBufferEscapePair escapeItem; - virBufferEscapePairPtr escapeList = NULL; + VIR_AUTOPTR(virBufferEscapePair) escapeList = NULL; size_t nescapeList = 0; va_list ap; @@ -749,7 +749,6 @@ virBufferEscapeN(virBufferPtr buf, cleanup: va_end(ap); - VIR_FREE(escapeList); } -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When variables of type virHashTablePtr are declared using VIR_AUTOPTR, the function virHashFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virhash.c | 1 - src/util/virhash.h | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index ecda55d..006ffd8 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -26,7 +26,6 @@ #include "virerror.h" #include "virhash.h" -#include "viralloc.h" #include "virlog.h" #include "virhashcode.h" #include "virrandom.h" diff --git a/src/util/virhash.h b/src/util/virhash.h index 5b24fc0..dd789c6 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -16,6 +16,8 @@ # include <stdint.h> # include <stdbool.h> +# include "viralloc.h" + /* * The hash table. */ @@ -200,4 +202,6 @@ void *virHashSearch(const virHashTable *table, virHashSearcher iter, /* Convenience for when VIR_FREE(value) is sufficient as a data freer. */ void virHashValueFree(void *value, const void *name); +VIR_DEFINE_AUTOPTR_FUNC(virHashTable, virHashFree) + #endif /* ! __VIR_HASH_H__ */ -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virCgroupPtr is declared using VIR_AUTOPTR, the function virCgroupFree will be run automatically on it when it goes out of scope. This commit also adds an intermediate typedef for virCgroup type for use with the cleanup macros by renaming the struct in src/util/vircgrouppriv.h from virCgroup to _virCgroup. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/vircgroup.c | 1 - src/util/vircgroup.h | 9 +++++++-- src/util/vircgrouppriv.h | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 140b016..bc5f774 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -50,7 +50,6 @@ #include "vircgrouppriv.h" #include "virutil.h" -#include "viralloc.h" #include "virerror.h" #include "virlog.h" #include "virfile.h" diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index e4ffd57..065861d 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -27,9 +27,11 @@ # include "virutil.h" # include "virbitmap.h" +# include "viralloc.h" -struct virCgroup; -typedef struct virCgroup *virCgroupPtr; +struct _virCgroup; +typedef struct _virCgroup virCgroup; +typedef virCgroup *virCgroupPtr; enum { VIR_CGROUP_CONTROLLER_CPU, @@ -297,4 +299,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller); bool virCgroupControllerAvailable(int controller); + +VIR_DEFINE_AUTOPTR_FUNC(virCgroup, virCgroupFree) + #endif /* __VIR_CGROUP_H__ */ diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 722863e..a72bee1 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -42,7 +42,7 @@ struct virCgroupController { char *placement; }; -struct virCgroup { +struct _virCgroup { char *path; struct virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/vircgroup.c | 528 ++++++++++++++++++--------------------------------- 1 file changed, 181 insertions(+), 347 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index bc5f774..4bb4408 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -162,7 +162,7 @@ virCgroupPartitionNeedsEscaping(const char *path) { FILE *fp = NULL; int ret = 0; - char *line = NULL; + VIR_AUTOFREE(char *) line = NULL; size_t buflen; /* If it starts with 'cgroup.' or a '_' of any @@ -222,7 +222,6 @@ virCgroupPartitionNeedsEscaping(const char *path) } cleanup: - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); return ret; } @@ -255,41 +254,40 @@ virCgroupValidateMachineGroup(virCgroupPtr group, const char *machinename) { size_t i; - bool valid = false; - char *partname = NULL; - char *scopename_old = NULL; - char *scopename_new = NULL; - char *partmachinename = NULL; + VIR_AUTOFREE(char *) partname = NULL; + VIR_AUTOFREE(char *) scopename_old = NULL; + VIR_AUTOFREE(char *) scopename_new = NULL; + VIR_AUTOFREE(char *) partmachinename = NULL; if (virAsprintf(&partname, "%s.libvirt-%s", name, drivername) < 0) - goto cleanup; + return false; if (virCgroupPartitionEscape(&partname) < 0) - goto cleanup; + return false; if (machinename && (virAsprintf(&partmachinename, "%s.libvirt-%s", machinename, drivername) < 0 || virCgroupPartitionEscape(&partmachinename) < 0)) - goto cleanup; + return false; if (!(scopename_old = virSystemdMakeScopeName(name, drivername, true))) - goto cleanup; + return false; /* We should keep trying even if this failed */ if (!machinename) virResetLastError(); else if (!(scopename_new = virSystemdMakeScopeName(machinename, drivername, false))) - goto cleanup; + return false; if (virCgroupPartitionEscape(&scopename_old) < 0) - goto cleanup; + return false; if (scopename_new && virCgroupPartitionEscape(&scopename_new) < 0) - goto cleanup; + return false; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { char *tmp; @@ -302,7 +300,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group, tmp = strrchr(group->controllers[i].placement, '/'); if (!tmp) - goto cleanup; + return false; if (stripEmulatorSuffix && (i == VIR_CGROUP_CONTROLLER_CPU || @@ -312,7 +310,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group, *tmp = '\0'; tmp = strrchr(group->controllers[i].placement, '/'); if (!tmp) - goto cleanup; + return false; } tmp++; @@ -328,18 +326,11 @@ virCgroupValidateMachineGroup(virCgroupPtr group, tmp, virCgroupControllerTypeToString(i), name, NULLSTR(machinename), partname, scopename_old, NULLSTR(scopename_new)); - goto cleanup; + return false; } } - valid = true; - - cleanup: - VIR_FREE(partmachinename); - VIR_FREE(partname); - VIR_FREE(scopename_old); - VIR_FREE(scopename_new); - return valid; + return true; } @@ -377,7 +368,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, FILE *mounts = NULL; struct mntent entry; char buf[CGROUP_MAX_VAL]; - char *linksrc = NULL; int ret = -1; mounts = fopen(path, "r"); @@ -432,8 +422,9 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, /* If it is a co-mount it has a filename like "cpu,cpuacct" * and we must identify the symlink path */ if (checkLinks && strchr(tmp2 + 1, ',')) { + VIR_AUTOFREE(char *) linksrc = NULL; + *tmp2 = '\0'; - VIR_FREE(linksrc); if (virAsprintf(&linksrc, "%s/%s", entry.mnt_dir, typestr) < 0) goto cleanup; @@ -467,7 +458,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, ret = 0; cleanup: - VIR_FREE(linksrc); VIR_FORCE_FCLOSE(mounts); return ret; } @@ -546,7 +536,7 @@ virCgroupDetectPlacement(virCgroupPtr group, FILE *mapping = NULL; char line[1024]; int ret = -1; - char *procfile; + VIR_AUTOFREE(char *) procfile = NULL; VIR_DEBUG("Detecting placement for pid %lld path %s", (long long) pid, path); @@ -627,9 +617,7 @@ virCgroupDetectPlacement(virCgroupPtr group, ret = 0; cleanup: - VIR_FREE(procfile); VIR_FORCE_FCLOSE(mapping); - return ret; } @@ -785,8 +773,7 @@ virCgroupSetValueStr(virCgroupPtr group, const char *key, const char *value) { - int ret = -1; - char *keypath = NULL; + VIR_AUTOFREE(char *) keypath = NULL; char *tmp = NULL; if (virCgroupPathOfController(group, controller, key, &keypath) < 0) @@ -799,18 +786,14 @@ virCgroupSetValueStr(virCgroupPtr group, virReportSystemError(errno, _("Invalid value '%s' for '%s'"), value, tmp + 1); - goto cleanup; + return -1; } virReportSystemError(errno, _("Unable to write to '%s'"), keypath); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(keypath); - return ret; + return 0; } @@ -820,8 +803,8 @@ virCgroupGetValueStr(virCgroupPtr group, const char *key, char **value) { - char *keypath = NULL; - int ret = -1, rc; + VIR_AUTOFREE(char *) keypath = NULL; + int rc; *value = NULL; @@ -833,18 +816,14 @@ virCgroupGetValueStr(virCgroupPtr group, if ((rc = virFileReadAll(keypath, 1024*1024, value)) < 0) { virReportSystemError(errno, _("Unable to read from '%s'"), keypath); - goto cleanup; + return -1; } /* Terminated with '\n' has sometimes harmful effects to the caller */ if (rc > 0 && (*value)[rc - 1] == '\n') (*value)[rc - 1] = '\0'; - ret = 0; - - cleanup: - VIR_FREE(keypath); - return ret; + return 0; } @@ -855,8 +834,8 @@ virCgroupGetValueForBlkDev(virCgroupPtr group, const char *path, char **value) { - char *prefix = NULL; - char *str = NULL; + VIR_AUTOFREE(char *) prefix = NULL; + VIR_AUTOFREE(char *) str = NULL; char **lines = NULL; int ret = -1; @@ -874,8 +853,6 @@ virCgroupGetValueForBlkDev(virCgroupPtr group, ret = 0; error: - VIR_FREE(str); - VIR_FREE(prefix); virStringListFree(lines); return ret; } @@ -887,17 +864,12 @@ virCgroupSetValueU64(virCgroupPtr group, const char *key, unsigned long long int value) { - char *strval = NULL; - int ret; + VIR_AUTOFREE(char *) strval = NULL; if (virAsprintf(&strval, "%llu", value) < 0) return -1; - ret = virCgroupSetValueStr(group, controller, key, strval); - - VIR_FREE(strval); - - return ret; + return virCgroupSetValueStr(group, controller, key, strval); } @@ -907,17 +879,12 @@ virCgroupSetValueI64(virCgroupPtr group, const char *key, long long int value) { - char *strval = NULL; - int ret; + VIR_AUTOFREE(char *) strval = NULL; if (virAsprintf(&strval, "%lld", value) < 0) return -1; - ret = virCgroupSetValueStr(group, controller, key, strval); - - VIR_FREE(strval); - - return ret; + return virCgroupSetValueStr(group, controller, key, strval); } @@ -927,24 +894,19 @@ virCgroupGetValueI64(virCgroupPtr group, const char *key, long long int *value) { - char *strval = NULL; - int ret = -1; + VIR_AUTOFREE(char *) strval = NULL; if (virCgroupGetValueStr(group, controller, key, &strval) < 0) - goto cleanup; + return -1; if (virStrToLong_ll(strval, NULL, 10, value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), strval); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(strval); - return ret; + return 0; } @@ -954,24 +916,19 @@ virCgroupGetValueU64(virCgroupPtr group, const char *key, unsigned long long int *value) { - char *strval = NULL; - int ret = -1; + VIR_AUTOFREE(char *) strval = NULL; if (virCgroupGetValueStr(group, controller, key, &strval) < 0) - goto cleanup; + return -1; if (virStrToLong_ull(strval, NULL, 10, value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), strval); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(strval); - return ret; + return 0; } @@ -987,7 +944,7 @@ virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path); for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) { - char *value; + VIR_AUTOFREE(char *) value = NULL; if (virCgroupGetValueStr(parent, VIR_CGROUP_CONTROLLER_CPUSET, @@ -1000,11 +957,8 @@ virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) if (virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPUSET, inherit_values[i], - value) < 0) { - VIR_FREE(value); + value) < 0) return -1; - } - VIR_FREE(value); } return 0; @@ -1043,11 +997,10 @@ virCgroupMakeGroup(virCgroupPtr parent, unsigned int flags) { size_t i; - int ret = -1; VIR_DEBUG("Make group %s", group->path); for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; /* We must never mkdir() in systemd's hierarchy */ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { @@ -1073,10 +1026,8 @@ virCgroupMakeGroup(virCgroupPtr parent, if (!virFileExists(path)) { if (!create || mkdir(path, 0755) < 0) { - if (errno == EEXIST) { - VIR_FREE(path); + if (errno == EEXIST) continue; - } /* With a kernel that doesn't support multi-level directory * for blkio controller, libvirt will fail and disable all * other controllers even though they are available. So @@ -1084,24 +1035,20 @@ virCgroupMakeGroup(virCgroupPtr parent, if (i == VIR_CGROUP_CONTROLLER_BLKIO) { VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel probably too old"); VIR_FREE(group->controllers[i].mountPoint); - VIR_FREE(path); continue; } else { virReportSystemError(errno, _("Failed to create controller %s for group"), virCgroupControllerTypeToString(i)); - VIR_FREE(path); - goto cleanup; + return -1; } } if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL && (i == VIR_CGROUP_CONTROLLER_CPUSET || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) { - if (virCgroupCpuSetInherit(parent, group) < 0) { - VIR_FREE(path); - goto cleanup; - } + if (virCgroupCpuSetInherit(parent, group) < 0) + return -1; } /* * Note that virCgroupSetMemoryUseHierarchy should always be @@ -1112,21 +1059,14 @@ virCgroupMakeGroup(virCgroupPtr parent, (i == VIR_CGROUP_CONTROLLER_MEMORY || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { - if (virCgroupSetMemoryUseHierarchy(group) < 0) { - VIR_FREE(path); - goto cleanup; - } + if (virCgroupSetMemoryUseHierarchy(group) < 0) + return -1; } } - - VIR_FREE(path); } VIR_DEBUG("Done making controllers for group"); - ret = 0; - - cleanup: - return ret; + return 0; } @@ -1338,9 +1278,9 @@ virCgroupNewPartition(const char *path, virCgroupPtr *group) { int ret = -1; - char *parentPath = NULL; + VIR_AUTOFREE(char *) parentPath = NULL; + VIR_AUTOFREE(char *) newPath = NULL; virCgroupPtr parent = NULL; - char *newPath = NULL; VIR_DEBUG("path=%s create=%d controllers=%x", path, create, controllers); @@ -1380,8 +1320,6 @@ virCgroupNewPartition(const char *path, if (ret != 0) virCgroupFree(*group); virCgroupFree(parent); - VIR_FREE(parentPath); - VIR_FREE(newPath); return ret; } @@ -1420,18 +1358,17 @@ virCgroupNewDomainPartition(virCgroupPtr partition, bool create, virCgroupPtr *group) { - int ret = -1; - char *grpname = NULL; + VIR_AUTOFREE(char *)grpname = NULL; if (virAsprintf(&grpname, "%s.libvirt-%s", name, driver) < 0) - goto cleanup; + return -1; if (virCgroupPartitionEscape(&grpname) < 0) - goto cleanup; + return -1; if (virCgroupNew(-1, grpname, partition, -1, group) < 0) - goto cleanup; + return -1; /* * Create a cgroup with memory.use_hierarchy enabled to @@ -1447,14 +1384,10 @@ virCgroupNewDomainPartition(virCgroupPtr partition, VIR_CGROUP_MEM_HIERACHY) < 0) { virCgroupRemove(*group); virCgroupFree(*group); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(grpname); - return ret; + return 0; } @@ -1476,27 +1409,26 @@ virCgroupNewThread(virCgroupPtr domain, bool create, virCgroupPtr *group) { - int ret = -1; - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; int controllers; switch (nameval) { case VIR_CGROUP_THREAD_VCPU: if (virAsprintf(&name, "vcpu%d", id) < 0) - goto cleanup; + return -1; break; case VIR_CGROUP_THREAD_EMULATOR: if (VIR_STRDUP(name, "emulator") < 0) - goto cleanup; + return -1; break; case VIR_CGROUP_THREAD_IOTHREAD: if (virAsprintf(&name, "iothread%d", id) < 0) - goto cleanup; + return -1; break; case VIR_CGROUP_THREAD_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected name value %d"), nameval); - goto cleanup; + return -1; } controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -1504,18 +1436,15 @@ virCgroupNewThread(virCgroupPtr domain, (1 << VIR_CGROUP_CONTROLLER_CPUSET)); if (virCgroupNew(-1, name, domain, controllers, group) < 0) - goto cleanup; + return -1; if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { virCgroupRemove(*group); virCgroupFree(*group); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(name); - return ret; + return 0; } @@ -1576,7 +1505,7 @@ virCgroupNewMachineSystemd(const char *name, int ret = -1; int rv; virCgroupPtr init, parent = NULL; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *offset; VIR_DEBUG("Trying to setup machine '%s' via systemd", name); @@ -1661,7 +1590,6 @@ virCgroupNewMachineSystemd(const char *name, ret = 0; cleanup: virCgroupFree(parent); - VIR_FREE(path); return ret; } @@ -1893,9 +1821,11 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, long long *requests_write) { long long stats_val; - char *str1 = NULL, *str2 = NULL, *p1, *p2; + VIR_AUTOFREE(char *) str1 = NULL; + VIR_AUTOFREE(char *) str2 = NULL; + char *p1; + char *p2; size_t i; - int ret = -1; const char *value_names[] = { "Read ", @@ -1918,12 +1848,12 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.io_service_bytes", &str1) < 0) - goto cleanup; + return -1; if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.io_serviced", &str2) < 0) - goto cleanup; + return -1; /* sum up all entries of the same kind, from all devices */ for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { @@ -1937,7 +1867,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, _("Cannot parse byte %sstat '%s'"), value_names[i], p1); - goto cleanup; + return -1; } if (stats_val < 0 || @@ -1946,7 +1876,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, virReportError(VIR_ERR_OVERFLOW, _("Sum of byte %sstat overflows"), value_names[i]); - goto cleanup; + return -1; } *bytes_ptrs[i] += stats_val; } @@ -1958,7 +1888,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, _("Cannot parse %srequest stat '%s'"), value_names[i], p2); - goto cleanup; + return -1; } if (stats_val < 0 || @@ -1967,18 +1897,13 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, virReportError(VIR_ERR_OVERFLOW, _("Sum of %srequest stat overflows"), value_names[i]); - goto cleanup; + return -1; } *requests_ptrs[i] += stats_val; } } - ret = 0; - - cleanup: - VIR_FREE(str2); - VIR_FREE(str1); - return ret; + return 0; } @@ -2002,9 +1927,12 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, long long *requests_read, long long *requests_write) { - char *str1 = NULL, *str2 = NULL, *str3 = NULL, *p1, *p2; + VIR_AUTOFREE(char *) str1 = NULL; + VIR_AUTOFREE(char *) str2 = NULL; + VIR_AUTOFREE(char *) str3 = NULL; + char *p1; + char *p2; size_t i; - int ret = -1; const char *value_names[] = { "Read ", @@ -2022,28 +1950,28 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.io_service_bytes", &str1) < 0) - goto cleanup; + return -1; if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.io_serviced", &str2) < 0) - goto cleanup; + return -1; if (!(str3 = virCgroupGetBlockDevString(path))) - goto cleanup; + return -1; if (!(p1 = strstr(str1, str3))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find byte stats for block device '%s'"), str3); - goto cleanup; + return -1; } if (!(p2 = strstr(str2, str3))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find request stats for block device '%s'"), str3); - goto cleanup; + return -1; } for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { @@ -2051,38 +1979,32 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find byte %sstats for block device '%s'"), value_names[i], str3); - goto cleanup; + return -1; } if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse %sstat '%s'"), value_names[i], p1 + strlen(value_names[i])); - goto cleanup; + return -1; } if (!(p2 = strstr(p2, value_names[i]))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot find request %sstats for block device '%s'"), value_names[i], str3); - goto cleanup; + return -1; } if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i]) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse %sstat '%s'"), value_names[i], p2 + strlen(value_names[i])); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - VIR_FREE(str3); - VIR_FREE(str2); - VIR_FREE(str1); - return ret; + return 0; } @@ -2138,24 +2060,19 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int riops) { - char *str = NULL; - char *blkstr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; if (!(blkstr = virCgroupGetBlockDevString(path))) return -1; if (virAsprintf(&str, "%s%u", blkstr, riops) < 0) - goto error; + return -1; - ret = virCgroupSetValueStr(group, + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.read_iops_device", str); - error: - VIR_FREE(blkstr); - VIR_FREE(str); - return ret; } @@ -2172,24 +2089,19 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int wiops) { - char *str = NULL; - char *blkstr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; if (!(blkstr = virCgroupGetBlockDevString(path))) return -1; if (virAsprintf(&str, "%s%u", blkstr, wiops) < 0) - goto error; + return -1; - ret = virCgroupSetValueStr(group, + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.write_iops_device", str); - error: - VIR_FREE(blkstr); - VIR_FREE(str); - return ret; } @@ -2206,24 +2118,19 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long rbps) { - char *str = NULL; - char *blkstr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; if (!(blkstr = virCgroupGetBlockDevString(path))) return -1; if (virAsprintf(&str, "%s%llu", blkstr, rbps) < 0) - goto error; + return -1; - ret = virCgroupSetValueStr(group, + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.read_bps_device", str); - error: - VIR_FREE(blkstr); - VIR_FREE(str); - return ret; } /** @@ -2239,24 +2146,19 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long wbps) { - char *str = NULL; - char *blkstr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; if (!(blkstr = virCgroupGetBlockDevString(path))) return -1; if (virAsprintf(&str, "%s%llu", blkstr, wbps) < 0) - goto error; + return -1; - ret = virCgroupSetValueStr(group, + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.write_bps_device", str); - error: - VIR_FREE(blkstr); - VIR_FREE(str); - return ret; } @@ -2274,24 +2176,19 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int weight) { - char *str = NULL; - char *blkstr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; if (!(blkstr = virCgroupGetBlockDevString(path))) return -1; if (virAsprintf(&str, "%s%d", blkstr, weight) < 0) - goto error; + return -1; - ret = virCgroupSetValueStr(group, + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.weight_device", str); - error: - VIR_FREE(blkstr); - VIR_FREE(str); - return ret; } /** @@ -2307,15 +2204,14 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int *riops) { - char *str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (virCgroupGetValueForBlkDev(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.read_iops_device", path, &str) < 0) - goto error; + return -1; if (!str) { *riops = 0; @@ -2323,13 +2219,10 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), str); - goto error; + return -1; } - ret = 0; - error: - VIR_FREE(str); - return ret; + return 0; } /** @@ -2345,15 +2238,14 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int *wiops) { - char *str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (virCgroupGetValueForBlkDev(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.write_iops_device", path, &str) < 0) - goto error; + return -1; if (!str) { *wiops = 0; @@ -2361,13 +2253,10 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), str); - goto error; + return -1; } - ret = 0; - error: - VIR_FREE(str); - return ret; + return 0; } /** @@ -2383,15 +2272,14 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long *rbps) { - char *str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (virCgroupGetValueForBlkDev(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.read_bps_device", path, &str) < 0) - goto error; + return -1; if (!str) { *rbps = 0; @@ -2399,13 +2287,10 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), str); - goto error; + return -1; } - ret = 0; - error: - VIR_FREE(str); - return ret; + return 0; } /** @@ -2421,15 +2306,14 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long *wbps) { - char *str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (virCgroupGetValueForBlkDev(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.throttle.write_bps_device", path, &str) < 0) - goto error; + return -1; if (!str) { *wbps = 0; @@ -2437,13 +2321,10 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), str); - goto error; + return -1; } - ret = 0; - error: - VIR_FREE(str); - return ret; + return 0; } /** @@ -2459,15 +2340,14 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int *weight) { - char *str = NULL; - int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (virCgroupGetValueForBlkDev(group, VIR_CGROUP_CONTROLLER_BLKIO, "blkio.weight_device", path, &str) < 0) - goto error; + return -1; if (!str) { *weight = 0; @@ -2475,13 +2355,10 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse '%s' as an integer"), str); - goto error; + return -1; } - ret = 0; - error: - VIR_FREE(str); - return ret; + return 0; } @@ -2940,36 +2817,29 @@ int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - int ret = -1; - char *devstr = NULL; - char *majorstr = NULL; - char *minorstr = NULL; + VIR_AUTOFREE(char *) devstr = NULL; + VIR_AUTOFREE(char *) majorstr = NULL; + VIR_AUTOFREE(char *) minorstr = NULL; if ((major < 0 && VIR_STRDUP(majorstr, "*") < 0) || (major >= 0 && virAsprintf(&majorstr, "%i", major) < 0)) - goto cleanup; + return -1; if ((minor < 0 && VIR_STRDUP(minorstr, "*") < 0) || (minor >= 0 && virAsprintf(&minorstr, "%i", minor) < 0)) - goto cleanup; + return -1; if (virAsprintf(&devstr, "%c %s:%s %s", type, majorstr, minorstr, virCgroupGetDevicePermsString(perms)) < 0) - goto cleanup; + return -1; if (virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_DEVICES, "devices.allow", devstr) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - VIR_FREE(devstr); - VIR_FREE(majorstr); - VIR_FREE(minorstr); - return ret; + return 0; } @@ -3031,36 +2901,29 @@ int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - int ret = -1; - char *devstr = NULL; - char *majorstr = NULL; - char *minorstr = NULL; + VIR_AUTOFREE(char *) devstr = NULL; + VIR_AUTOFREE(char *) majorstr = NULL; + VIR_AUTOFREE(char *) minorstr = NULL; if ((major < 0 && VIR_STRDUP(majorstr, "*") < 0) || (major >= 0 && virAsprintf(&majorstr, "%i", major) < 0)) - goto cleanup; + return -1; if ((minor < 0 && VIR_STRDUP(minorstr, "*") < 0) || (minor >= 0 && virAsprintf(&minorstr, "%i", minor) < 0)) - goto cleanup; + return -1; if (virAsprintf(&devstr, "%c %s:%s %s", type, majorstr, minorstr, virCgroupGetDevicePermsString(perms)) < 0) - goto cleanup; + return -1; if (virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_DEVICES, "devices.deny", devstr) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - VIR_FREE(devstr); - VIR_FREE(majorstr); - VIR_FREE(minorstr); - return ret; + return 0; } @@ -3130,10 +2993,10 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, { int ret = -1; ssize_t i = -1; - char *buf = NULL; virCgroupPtr group_vcpu = NULL; while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) { + VIR_AUTOFREE(char *) buf = NULL; char *pos; unsigned long long tmp; ssize_t j; @@ -3158,13 +3021,11 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, } virCgroupFree(group_vcpu); - VIR_FREE(buf); } ret = 0; cleanup: virCgroupFree(group_vcpu); - VIR_FREE(buf); return ret; } @@ -3201,8 +3062,8 @@ virCgroupGetPercpuStats(virCgroupPtr group, size_t i; int need_cpus, total_cpus; char *pos; - char *buf = NULL; - unsigned long long *sum_cpu_time = NULL; + VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOFREE(unsigned long long *) sum_cpu_time = NULL; virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; @@ -3288,8 +3149,6 @@ virCgroupGetPercpuStats(virCgroupPtr group, cleanup: virBitmapFree(cpumap); - VIR_FREE(sum_cpu_time); - VIR_FREE(buf); return ret; } @@ -3460,7 +3319,7 @@ virCgroupRemoveRecursively(char *grppath) /* This is best-effort cleanup: we want to log failures with just * VIR_ERROR instead of normal virReportError */ while ((direrr = virDirRead(grpdir, &ent, NULL)) > 0) { - char *path; + VIR_AUTOFREE(char *) path = NULL; if (ent->d_type != DT_DIR) continue; @@ -3469,7 +3328,6 @@ virCgroupRemoveRecursively(char *grppath) break; } rc = virCgroupRemoveRecursively(path); - VIR_FREE(path); if (rc != 0) break; } @@ -3507,10 +3365,11 @@ virCgroupRemove(virCgroupPtr group) { int rc = 0; size_t i; - char *grppath = NULL; VIR_DEBUG("Removing cgroup %s", group->path); for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + VIR_AUTOFREE(char *) grppath = NULL; + /* Skip over controllers not mounted */ if (!group->controllers[i].mountPoint) continue; @@ -3532,7 +3391,6 @@ virCgroupRemove(virCgroupPtr group) VIR_DEBUG("Removing cgroup %s and all child cgroups", grppath); rc = virCgroupRemoveRecursively(grppath); - VIR_FREE(grppath); } VIR_DEBUG("Done removing cgroup %s", group->path); @@ -3548,7 +3406,7 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids) { int ret = -1; bool killedAny = false; - char *keypath = NULL; + VIR_AUTOFREE(char *) keypath = NULL; bool done = false; FILE *fp = NULL; VIR_DEBUG("group=%p path=%s signum=%d pids=%p", @@ -3612,7 +3470,6 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids) ret = killedAny ? 1 : 0; cleanup: - VIR_FREE(keypath); VIR_FORCE_FCLOSE(fp); return ret; @@ -3677,7 +3534,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int ret = -1; int rc; bool killedAny = false; - char *keypath = NULL; + VIR_AUTOFREE(char *) keypath = NULL; DIR *dp = NULL; virCgroupPtr subgroup = NULL; struct dirent *ent; @@ -3731,7 +3588,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, cleanup: virCgroupFree(subgroup); - VIR_FREE(keypath); VIR_DIR_CLOSE(dp); return ret; } @@ -3845,9 +3701,8 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, unsigned long long *sys) { - char *str; + VIR_AUTOFREE(char *) str = NULL; char *p; - int ret = -1; static double scale = -1.0; if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, @@ -3859,14 +3714,14 @@ virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse user stat '%s'"), p); - goto cleanup; + return -1; } if (!(p = STRSKIP(p, "\nsystem ")) || virStrToLong_ull(p, NULL, 10, sys) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot parse sys stat '%s'"), p); - goto cleanup; + return -1; } /* times reported are in system ticks (generally 100 Hz), but that * rate can theoretically vary between machines. Scale things @@ -3876,17 +3731,14 @@ virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, if (ticks_per_sec == -1) { virReportSystemError(errno, "%s", _("Cannot determine system clock HZ")); - goto cleanup; + return -1; } scale = 1000000000.0 / ticks_per_sec; } *user *= scale; *sys *= scale; - ret = 0; - cleanup: - VIR_FREE(str); - return ret; + return 0; } @@ -3912,10 +3764,9 @@ int virCgroupBindMount(virCgroupPtr group, const char *oldroot, const char *mountopts) { - int ret = -1; size_t i; - char *opts = NULL; - char *root = NULL; + VIR_AUTOFREE(char *) opts = NULL; + VIR_AUTOFREE(char *) root = NULL; if (!(root = virCgroupIdentifyRoot(group))) return -1; @@ -3926,18 +3777,18 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, virReportSystemError(errno, _("Unable to create directory %s"), root); - goto cleanup; + return -1; } if (virAsprintf(&opts, "mode=755,size=65536%s", mountopts) < 0) - goto cleanup; + return -1; if (mount("tmpfs", root, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC, opts) < 0) { virReportSystemError(errno, _("Failed to mount %s on %s type %s"), "tmpfs", root, "tmpfs"); - goto cleanup; + return -1; } for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -3945,11 +3796,11 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, continue; if (!virFileExists(group->controllers[i].mountPoint)) { - char *src; + VIR_AUTOFREE(char *) src = NULL; if (virAsprintf(&src, "%s%s", oldroot, group->controllers[i].mountPoint) < 0) - goto cleanup; + return -1; VIR_DEBUG("Create mount point '%s'", group->controllers[i].mountPoint); @@ -3957,8 +3808,7 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, virReportSystemError(errno, _("Unable to create directory %s"), group->controllers[i].mountPoint); - VIR_FREE(src); - goto cleanup; + return -1; } if (mount(src, group->controllers[i].mountPoint, "none", MS_BIND, @@ -3966,11 +3816,8 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, virReportSystemError(errno, _("Failed to bind cgroup '%s' on '%s'"), src, group->controllers[i].mountPoint); - VIR_FREE(src); - goto cleanup; + return -1; } - - VIR_FREE(src); } if (group->controllers[i].linkPoint) { @@ -3983,16 +3830,12 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, _("Unable to symlink directory %s to %s"), group->controllers[i].mountPoint, group->controllers[i].linkPoint); - goto cleanup; + return -1; } } } - ret = 0; - cleanup: - VIR_FREE(root); - VIR_FREE(opts); - return ret; + return 0; } @@ -4003,11 +3846,11 @@ int virCgroupSetOwner(virCgroupPtr cgroup, { int ret = -1; size_t i; - char *base = NULL, *entry = NULL; DIR *dh = NULL; int direrr; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + VIR_AUTOFREE(char *) base = NULL; struct dirent *de; if (!((1 << i) & controllers)) @@ -4024,6 +3867,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup, goto cleanup; while ((direrr = virDirRead(dh, &de, base)) > 0) { + VIR_AUTOFREE(char *) entry = NULL; + if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) goto cleanup; @@ -4033,8 +3878,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup, entry, uid, gid); goto cleanup; } - - VIR_FREE(entry); } if (direrr < 0) goto cleanup; @@ -4046,7 +3889,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup, goto cleanup; } - VIR_FREE(base); VIR_DIR_CLOSE(dh); } @@ -4054,8 +3896,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup, cleanup: VIR_DIR_CLOSE(dh); - VIR_FREE(entry); - VIR_FREE(base); return ret; } @@ -4070,8 +3910,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, bool virCgroupSupportsCpuBW(virCgroupPtr cgroup) { - char *path = NULL; - bool ret = false; + VIR_AUTOFREE(char *) path = NULL; if (!cgroup) return false; @@ -4079,21 +3918,17 @@ virCgroupSupportsCpuBW(virCgroupPtr cgroup) if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, "cpu.cfs_period_us", &path) < 0) { virResetLastError(); - goto cleanup; + return false; } - ret = virFileExists(path); - - cleanup: - VIR_FREE(path); - return ret; + return virFileExists(path); } int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) { int ret = -1; - char *content = NULL; + VIR_AUTOFREE(char *) content = NULL; if (!cgroup) return -1; @@ -4103,7 +3938,6 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) if (ret == 0 && content[0] == '\0') ret = 1; - VIR_FREE(content); return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/vircgroup.c | 155 +++++++++++++++++---------------------------------- 1 file changed, 52 insertions(+), 103 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 4bb4408..cdb493e 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -836,25 +836,21 @@ virCgroupGetValueForBlkDev(virCgroupPtr group, { VIR_AUTOFREE(char *) prefix = NULL; VIR_AUTOFREE(char *) str = NULL; - char **lines = NULL; - int ret = -1; + VIR_AUTOPTR(virString) lines = NULL; if (virCgroupGetValueStr(group, controller, key, &str) < 0) - goto error; + return -1; if (!(prefix = virCgroupGetBlockDevString(path))) - goto error; + return -1; if (!(lines = virStringSplit(str, "\n", -1))) - goto error; + return -1; if (VIR_STRDUP(*value, virStringListGetFirstWithPrefix(lines, prefix)) < 0) - goto error; + return -1; - ret = 0; - error: - virStringListFree(lines); - return ret; + return 0; } @@ -1217,12 +1213,11 @@ virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) static int virCgroupSetPartitionSuffix(const char *path, char **res) { - char **tokens; + VIR_AUTOPTR(virString) tokens = NULL; size_t i; - int ret = -1; if (!(tokens = virStringSplit(path, "/", 0))) - return ret; + return -1; for (i = 0; tokens[i] != NULL; i++) { /* Whitelist the 3 top level fixed dirs @@ -1241,22 +1236,18 @@ virCgroupSetPartitionSuffix(const char *path, char **res) !strchr(tokens[i], '.')) { if (VIR_REALLOC_N(tokens[i], strlen(tokens[i]) + strlen(".partition") + 1) < 0) - goto cleanup; + return -1; strcat(tokens[i], ".partition"); } if (virCgroupPartitionEscape(&(tokens[i])) < 0) - goto cleanup; + return -1; } if (!(*res = virStringListJoin((const char **)tokens, "/"))) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virStringListFree(tokens); - return ret; + return 0; } @@ -1280,7 +1271,7 @@ virCgroupNewPartition(const char *path, int ret = -1; VIR_AUTOFREE(char *) parentPath = NULL; VIR_AUTOFREE(char *) newPath = NULL; - virCgroupPtr parent = NULL; + VIR_AUTOPTR(virCgroup) parent = NULL; VIR_DEBUG("path=%s create=%d controllers=%x", path, create, controllers); @@ -1319,7 +1310,6 @@ virCgroupNewPartition(const char *path, cleanup: if (ret != 0) virCgroupFree(*group); - virCgroupFree(parent); return ret; } @@ -1502,9 +1492,9 @@ virCgroupNewMachineSystemd(const char *name, int controllers, virCgroupPtr *group) { - int ret = -1; int rv; - virCgroupPtr init, parent = NULL; + VIR_AUTOPTR(virCgroup) init = NULL; + VIR_AUTOPTR(virCgroup) parent = NULL; VIR_AUTOFREE(char *) path = NULL; char *offset; @@ -1531,12 +1521,10 @@ virCgroupNewMachineSystemd(const char *name, path = init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement; init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL; - virCgroupFree(init); if (!path || STREQ(path, "/") || path[0] != '/') { VIR_DEBUG("Systemd didn't setup its controller"); - ret = -2; - goto cleanup; + return -2; } offset = path; @@ -1546,7 +1534,7 @@ virCgroupNewMachineSystemd(const char *name, NULL, controllers, &parent) < 0) - goto cleanup; + return -1; for (;;) { @@ -1560,11 +1548,11 @@ virCgroupNewMachineSystemd(const char *name, parent, controllers, &tmp) < 0) - goto cleanup; + return -1; if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) { virCgroupFree(tmp); - goto cleanup; + return -1; } if (t) { *t = '/'; @@ -1587,10 +1575,7 @@ virCgroupNewMachineSystemd(const char *name, } } - ret = 0; - cleanup: - virCgroupFree(parent); - return ret; + return 0; } @@ -1611,8 +1596,7 @@ virCgroupNewMachineManual(const char *name, int controllers, virCgroupPtr *group) { - virCgroupPtr parent = NULL; - int ret = -1; + VIR_AUTOPTR(virCgroup) parent = NULL; VIR_DEBUG("Fallback to non-systemd setup"); if (virCgroupNewPartition(partition, @@ -1620,9 +1604,9 @@ virCgroupNewMachineManual(const char *name, controllers, &parent) < 0) { if (virCgroupNewIgnoreError()) - goto done; + return 0; - goto cleanup; + return -1; } if (virCgroupNewDomainPartition(parent, @@ -1630,7 +1614,7 @@ virCgroupNewMachineManual(const char *name, name, true, group) < 0) - goto cleanup; + return -1; if (virCgroupAddTask(*group, pidleader) < 0) { virErrorPtr saved = virSaveLastError(); @@ -1642,12 +1626,7 @@ virCgroupNewMachineManual(const char *name, } } - done: - ret = 0; - - cleanup: - virCgroupFree(parent); - return ret; + return 0; } @@ -2376,7 +2355,7 @@ static virOnceControl virCgroupMemoryOnce = VIR_ONCE_CONTROL_INITIALIZER; static void virCgroupMemoryOnceInit(void) { - virCgroupPtr group; + VIR_AUTOPTR(virCgroup) group = NULL; unsigned long long int mem_unlimited = 0ULL; if (virCgroupNew(-1, "/", NULL, -1, &group) < 0) @@ -2390,7 +2369,6 @@ virCgroupMemoryOnceInit(void) "memory.limit_in_bytes", &mem_unlimited)); cleanup: - virCgroupFree(group); virCgroupMemoryUnlimitedKB = mem_unlimited >> 10; } @@ -2991,22 +2969,21 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, size_t nsum, virBitmapPtr cpumap) { - int ret = -1; ssize_t i = -1; - virCgroupPtr group_vcpu = NULL; while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) { VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOPTR(virCgroup) group_vcpu = NULL; char *pos; unsigned long long tmp; ssize_t j; if (virCgroupNewThread(group, VIR_CGROUP_THREAD_VCPU, i, false, &group_vcpu) < 0) - goto cleanup; + return -1; if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0) - goto cleanup; + return -1; pos = buf; for (j = virBitmapNextSetBit(cpumap, -1); @@ -3015,18 +2992,13 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); - goto cleanup; + return -1; } sum_cpu_time[j] += tmp; } - - virCgroupFree(group_vcpu); } - ret = 0; - cleanup: - virCgroupFree(group_vcpu); - return ret; + return 0; } @@ -3058,7 +3030,6 @@ virCgroupGetPercpuStats(virCgroupPtr group, unsigned int ncpus, virBitmapPtr guestvcpus) { - int ret = -1; size_t i; int need_cpus, total_cpus; char *pos; @@ -3067,7 +3038,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; - virBitmapPtr cpumap = NULL; + VIR_AUTOPTR(virBitmap) cpumap = NULL; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { @@ -3084,21 +3055,19 @@ virCgroupGetPercpuStats(virCgroupPtr 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 */ @@ -3113,14 +3082,14 @@ virCgroupGetPercpuStats(virCgroupPtr 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 = &params[(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 */ @@ -3128,10 +3097,10 @@ virCgroupGetPercpuStats(virCgroupPtr group, if (guestvcpus && param_idx < nparams) { if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) - goto cleanup; + return -1; if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time, need_cpus, cpumap) < 0) - goto cleanup; + return -1; for (i = start_cpu; i < need_cpus; i++) { if (virTypedParameterAssign(&params[(i - start_cpu) * nparams + @@ -3139,17 +3108,13 @@ virCgroupGetPercpuStats(virCgroupPtr 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; } @@ -3505,23 +3470,18 @@ int virCgroupKill(virCgroupPtr group, int signum) { VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); - int ret; /* The 'tasks' file in cgroups can contain duplicated * pids, so we use a hash to track which we've already * killed. */ - virHashTablePtr pids = virHashCreateFull(100, + VIR_AUTOPTR(virHashTable) pids = virHashCreateFull(100, NULL, virCgroupPidCode, virCgroupPidEqual, virCgroupPidCopy, NULL); - ret = virCgroupKillInternal(group, signum, pids); - - virHashFree(pids); - - return ret; + return virCgroupKillInternal(group, signum, pids); } @@ -3536,7 +3496,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, bool killedAny = false; VIR_AUTOFREE(char *) keypath = NULL; DIR *dp = NULL; - virCgroupPtr subgroup = NULL; struct dirent *ent; int direrr; VIR_DEBUG("group=%p path=%s signum=%d pids=%p", @@ -3561,6 +3520,8 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, } while ((direrr = virDirRead(dp, &ent, keypath)) > 0) { + VIR_AUTOPTR(virCgroup) subgroup = NULL; + if (ent->d_type != DT_DIR) continue; @@ -3577,8 +3538,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, if (dormdir) virCgroupRemove(subgroup); - - virCgroupFree(subgroup); } if (direrr < 0) goto cleanup; @@ -3587,7 +3546,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, ret = killedAny ? 1 : 0; cleanup: - virCgroupFree(subgroup); VIR_DIR_CLOSE(dp); return ret; } @@ -3596,20 +3554,15 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, int virCgroupKillRecursive(virCgroupPtr group, int signum) { - int ret; VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); - virHashTablePtr pids = virHashCreateFull(100, + VIR_AUTOPTR(virHashTable) pids = virHashCreateFull(100, NULL, virCgroupPidCode, virCgroupPidEqual, virCgroupPidCopy, NULL); - ret = virCgroupKillRecursiveInternal(group, signum, pids, false); - - virHashFree(pids); - - return ret; + return virCgroupKillRecursiveInternal(group, signum, pids, false); } @@ -3944,15 +3897,12 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) bool virCgroupControllerAvailable(int controller) { - virCgroupPtr cgroup; - bool ret = false; + VIR_AUTOPTR(virCgroup) cgroup = NULL; if (virCgroupNewSelf(&cgroup) < 0) - return ret; + return false; - ret = virCgroupHasController(cgroup, controller); - virCgroupFree(cgroup); - return ret; + return virCgroupHasController(cgroup, controller); } #else /* !VIR_CGROUP_SUPPORTED */ @@ -4740,7 +4690,7 @@ virCgroupDelThread(virCgroupPtr cgroup, virCgroupThreadName nameval, int idx) { - virCgroupPtr new_cgroup = NULL; + VIR_AUTOPTR(virCgroup) new_cgroup = NULL; if (cgroup) { if (virCgroupNewThread(cgroup, nameval, idx, false, &new_cgroup) < 0) @@ -4748,7 +4698,6 @@ virCgroupDelThread(virCgroupPtr cgroup, /* Remove the offlined cgroup */ virCgroupRemove(new_cgroup); - virCgroupFree(new_cgroup); } return 0; -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When variables of type virMediatedDevicePtr and virMediatedDeviceTypePtr are declared using VIR_AUTOPTR, the functions virMediatedDeviceFree and virMediatedDeviceTypeFree, respectively, will be run automatically on them when they go out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virmdev.c | 1 - src/util/virmdev.h | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 6c51388..d7bcb1d 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -21,7 +21,6 @@ #include "dirname.h" #include "virmdev.h" #include "virlog.h" -#include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virstring.h" diff --git a/src/util/virmdev.h b/src/util/virmdev.h index cfda2ca..7c93c4d 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -22,6 +22,7 @@ # include "internal.h" # include "virobject.h" # include "virutil.h" +# include "viralloc.h" typedef enum { VIR_MDEV_MODEL_TYPE_VFIO_PCI = 0, @@ -135,4 +136,7 @@ int virMediatedDeviceTypeReadAttrs(const char *sysfspath, virMediatedDeviceTypePtr *type); +VIR_DEFINE_AUTOPTR_FUNC(virMediatedDevice, virMediatedDeviceFree) +VIR_DEFINE_AUTOPTR_FUNC(virMediatedDeviceType, virMediatedDeviceTypeFree) + #endif /* __VIR_MDEV_H__ */ -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virmdev.c | 53 +++++++++++++++++++---------------------------------- 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/src/util/virmdev.c b/src/util/virmdev.c index d7bcb1d..04c9302 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -72,24 +72,23 @@ static int virMediatedDeviceGetSysfsDeviceAPI(virMediatedDevicePtr dev, char **device_api) { - int ret = -1; - char *buf = NULL; - char *file = NULL; + VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOFREE(char *) file = NULL; char *tmp = NULL; if (virAsprintf(&file, "%s/mdev_type/device_api", dev->path) < 0) - goto cleanup; + return -1; /* TODO - make this a generic method to access sysfs files for various * kinds of devices */ if (!virFileExists(file)) { virReportSystemError(errno, _("failed to read '%s'"), file); - goto cleanup; + return -1; } if (virFileReadAll(file, 1024, &buf) < 0) - goto cleanup; + return -1; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -97,11 +96,7 @@ virMediatedDeviceGetSysfsDeviceAPI(virMediatedDevicePtr dev, *device_api = buf; buf = NULL; - ret = 0; - cleanup: - VIR_FREE(file); - VIR_FREE(buf); - return ret; + return 0; } @@ -109,8 +104,7 @@ static int virMediatedDeviceCheckModel(virMediatedDevicePtr dev, virMediatedDeviceModelType model) { - int ret = -1; - char *dev_api = NULL; + VIR_AUTOFREE(char *) dev_api = NULL; int actual_model; if (virMediatedDeviceGetSysfsDeviceAPI(dev, &dev_api) < 0) @@ -123,7 +117,7 @@ virMediatedDeviceCheckModel(virMediatedDevicePtr dev, virReportError(VIR_ERR_INTERNAL_ERROR, _("device API '%s' not supported yet"), dev_api); - goto cleanup; + return -1; } if (actual_model != model) { @@ -132,13 +126,10 @@ virMediatedDeviceCheckModel(virMediatedDevicePtr dev, "device only supports '%s'"), virMediatedDeviceModelTypeToString(model), dev->path, dev_api); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(dev_api); - return ret; + return 0; } @@ -147,7 +138,7 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model) { virMediatedDevicePtr ret = NULL; virMediatedDevicePtr dev = NULL; - char *sysfspath = NULL; + VIR_AUTOFREE(char *) sysfspath = NULL; if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr))) goto cleanup; @@ -173,7 +164,6 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model) VIR_STEAL_PTR(ret, dev); cleanup: - VIR_FREE(sysfspath); virMediatedDeviceFree(dev); return ret; } @@ -218,34 +208,30 @@ virMediatedDeviceGetPath(virMediatedDevicePtr dev) char * virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr) { - char *result_path = NULL; - char *iommu_path = NULL; + VIR_AUTOFREE(char *) result_path = NULL; + VIR_AUTOFREE(char *) iommu_path = NULL; + VIR_AUTOFREE(char *) dev_path = virMediatedDeviceGetSysfsPath(uuidstr); char *vfio_path = NULL; - char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr); if (!dev_path) return NULL; if (virAsprintf(&iommu_path, "%s/iommu_group", dev_path) < 0) - goto cleanup; + return NULL; if (!virFileExists(iommu_path)) { virReportSystemError(errno, _("failed to access '%s'"), iommu_path); - goto cleanup; + return NULL; } if (virFileResolveLink(iommu_path, &result_path) < 0) { virReportSystemError(errno, _("failed to resolve '%s'"), iommu_path); - goto cleanup; + return NULL; } if (virAsprintf(&vfio_path, "/dev/vfio/%s", last_component(result_path)) < 0) - goto cleanup; + return vfio_path; - cleanup: - VIR_FREE(result_path); - VIR_FREE(iommu_path); - VIR_FREE(dev_path); return vfio_path; } @@ -253,7 +239,7 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr) int virMediatedDeviceGetIOMMUGroupNum(const char *uuidstr) { - char *vfio_path = NULL; + VIR_AUTOFREE(char *) vfio_path = NULL; char *group_num_str = NULL; unsigned int group_num = -1; @@ -263,7 +249,6 @@ virMediatedDeviceGetIOMMUGroupNum(const char *uuidstr) group_num_str = last_component(vfio_path); ignore_value(virStrToLong_ui(group_num_str, NULL, 10, &group_num)); - VIR_FREE(vfio_path); return group_num; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virmdev.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 04c9302..46999bb 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -137,20 +137,20 @@ virMediatedDevicePtr virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model) { virMediatedDevicePtr ret = NULL; - virMediatedDevicePtr dev = NULL; + VIR_AUTOPTR(virMediatedDevice) dev = NULL; VIR_AUTOFREE(char *) sysfspath = NULL; if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr))) - goto cleanup; + return NULL; if (!virFileExists(sysfspath)) { virReportError(VIR_ERR_DEVICE_MISSING, _("mediated device '%s' not found"), uuidstr); - goto cleanup; + return NULL; } if (VIR_ALLOC(dev) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(dev->path, sysfspath); @@ -158,13 +158,11 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model) * supported mediated device's API. */ if (virMediatedDeviceCheckModel(dev, model)) - goto cleanup; + return NULL; dev->model = model; VIR_STEAL_PTR(ret, dev); - cleanup: - virMediatedDeviceFree(dev); return ret; } @@ -372,8 +370,7 @@ void virMediatedDeviceListDel(virMediatedDeviceListPtr list, virMediatedDevicePtr dev) { - virMediatedDevicePtr ret = virMediatedDeviceListSteal(list, dev); - virMediatedDeviceFree(ret); + VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev); } @@ -494,23 +491,22 @@ int virMediatedDeviceTypeReadAttrs(const char *sysfspath, virMediatedDeviceTypePtr *type) { - int ret = -1; - virMediatedDeviceTypePtr tmp = NULL; + VIR_AUTOPTR(virMediatedDeviceType) tmp = NULL; #define MDEV_GET_SYSFS_ATTR(attr, dst, cb, optional) \ do { \ int rc; \ if ((rc = cb(dst, "%s/%s", sysfspath, attr)) < 0) { \ if (rc != -2 || !optional) \ - goto cleanup; \ + return 0; \ } \ } while (0) if (VIR_ALLOC(tmp) < 0) - goto cleanup; + return -1; if (VIR_STRDUP(tmp->id, last_component(sysfspath)) < 0) - goto cleanup; + return -1; /* @name sysfs attribute is optional, so getting ENOENT is fine */ MDEV_GET_SYSFS_ATTR("name", &tmp->name, virFileReadValueString, true); @@ -522,8 +518,6 @@ virMediatedDeviceTypeReadAttrs(const char *sysfspath, #undef MDEV_GET_SYSFS_ATTR VIR_STEAL_PTR(*type, tmp); - ret = 0; - cleanup: - virMediatedDeviceTypeFree(tmp); - return ret; + + return 0; } -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virFirewallPtr is declared using VIR_AUTOPTR, the function virFirewallFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virfirewall.c | 1 - src/util/virfirewall.h | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 10c370a..dfd792f 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -27,7 +27,6 @@ #include <stdarg.h> -#include "viralloc.h" #include "virfirewallpriv.h" #include "virerror.h" #include "virutil.h" diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index b04ab48..e024e88 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -25,6 +25,7 @@ # define __VIR_FIREWALL_H__ # include "internal.h" +# include "viralloc.h" typedef struct _virFirewall virFirewall; typedef virFirewall *virFirewallPtr; @@ -116,4 +117,6 @@ int virFirewallApply(virFirewallPtr firewall); void virFirewallSetLockOverride(bool avoid); +VIR_DEFINE_AUTOPTR_FUNC(virFirewall, virFirewallFree) + #endif /* __VIR_FIREWALL_H__ */ -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virfirewall.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index dfd792f..b4a4d06 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -511,7 +511,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall, virFirewallRulePtr rule, const char *fmt, ...) { - char *arg; + VIR_AUTOFREE(char *) arg = NULL; va_list list; VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule); @@ -525,13 +525,11 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall, va_end(list); - VIR_FREE(arg); return; no_memory: firewall->err = ENOMEM; va_end(list); - VIR_FREE(arg); } @@ -678,7 +676,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule, virCommandPtr cmd = NULL; int status; int ret = -1; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; if (!bin) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -702,11 +700,10 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule, if (ignoreErrors) { VIR_DEBUG("Ignoring error running command"); } else { - char *args = virCommandToString(cmd); + VIR_AUTOFREE(char *) args = virCommandToString(cmd); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to apply firewall rules %s: %s"), NULLSTR(args), NULLSTR(error)); - VIR_FREE(args); VIR_FREE(*output); goto cleanup; } @@ -714,7 +711,6 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule, ret = 0; cleanup: - VIR_FREE(error); virCommandFree(cmd); return ret; } @@ -807,12 +803,11 @@ virFirewallApplyRule(virFirewallPtr firewall, virFirewallRulePtr rule, bool ignoreErrors) { - char *output = NULL; + VIR_AUTOFREE(char *) output = NULL; + VIR_AUTOFREE(char *) str = virFirewallRuleToString(rule); char **lines = NULL; int ret = -1; - char *str = virFirewallRuleToString(rule); VIR_INFO("Applying rule '%s'", NULLSTR(str)); - VIR_FREE(str); if (rule->ignoreErrors) ignoreErrors = rule->ignoreErrors; @@ -857,7 +852,6 @@ virFirewallApplyRule(virFirewallPtr firewall, ret = 0; cleanup: virStringListFree(lines); - VIR_FREE(output); return ret; } -- 1.8.3.1

On Mon, 23 Jul 2018 at 18:39, Erik Skultety <eskultet(a)redhat.com&gt; wrote: There are many functions in virfirewall.c where no_memory is used instead of cleanup. So I should change either all of them, or none of them.

On Tue, 24 Jul 2018 at 11:54, Erik Skultety <eskultet(a)redhat.com&gt; wrote: Also, according to libvirt hacking page, no_memory is a standard label [1]. Should we replace it then? [1]: https://libvirt.org/hacking.html#goto

On Tue, Jul 24, 2018 at 10:46:54PM +0530, Sukrit Bhatnagar wrote: Fair enough, you can disregard my comment then. Erik

On Sat, Jul 21, 2018 at 05:36:49PM +0530, Sukrit Bhatnagar wrote: ^this should be reworked in the following way: if (!virFileExists) { VIR_DEBUG; return 0; } if (!virFileIsExecutable) { VIR_DEBUG; return 0; } VIR_DEBUG; return 1; Otherwise looks okay. Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt;

On Sat, Jul 21, 2018 at 05:36:52PM +0530, Sukrit Bhatnagar wrote: ... Drop the 'else' clause, it's not needed anymore. ... ... here too.. ... virPCIDevicePtr ret = NULL; VIR_AUTOPTR(virPCIDevicePtr) tmp = NULL; VIR_STEAL_PTR(ret, dev); Preserve the 'cleanup' label... Drop 'error' label ^this can be dropped too... ^return ret; Erik

On Sat, Jul 21, 2018 at 05:36:53PM +0530, Sukrit Bhatnagar wrote: ... if (!dev || VIR_STRDUP(*pciConfigAddr, dev->name) < 0)) return -1; ^return 0; You'll first need to define copy as VIR_AUTOPTR, if you want to use ^this assignment, otherwise it's a NOP. There may be some leftovers from the previous patch that will need to go into this one, otherwise looks fine. Erik

On Sat, Jul 21, 2018 at 05:36:57PM +0530, Sukrit Bhatnagar wrote: Technically, there's also a virUSBDevicePtr instance in virUSBDeviceSearch that could be converted to VIR_AUTOPTR, but virUSBDeviceListAdd would have to take a double pointer to @dev instead of a single pointer. A bit more background info - the current issue is that virUSBDeviceListAdd calls our VIR_APPEND_ELEMENT helper which does clear the original pointer which we could utilize here, but not while passing a single pointer. Not a deal breaker, though, it's just a nice to have, since you're already working in this area, because I don't suppose we'd make such a change any time soon after your assignment is over. (regardless) Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt;

On Thu, 26 Jul 2018 at 00:32, Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; wrote: For now, I am changing virUSBDeviceListAdd and the related occurrences.

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When variables of type virSCSIDevicePtr and virUsedByInfoPtr are declared using VIR_AUTOPTR, the functions virSCSIDeviceFree and virSCSIDeviceUsedByInfoFree, respectively, will be run automatically on them when they go out of scope. This commit also adds an intermediate typedef for virCgroup type for use with the cleanup macros by renaming the struct in src/util/vircgrouppriv.h from virCgroup to _virCgroup. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virscsi.c | 5 +++-- src/util/virscsi.h | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/virscsi.c b/src/util/virscsi.c index b51103a..33292f6 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -37,7 +37,6 @@ #include "virlog.h" #include "virscsi.h" -#include "viralloc.h" #include "virfile.h" #include "virutil.h" #include "virstring.h" @@ -54,7 +53,8 @@ struct _virUsedByInfo { char *drvname; /* which driver */ char *domname; /* which domain */ }; -typedef struct _virUsedByInfo *virUsedByInfoPtr; +typedef struct _virUsedByInfo virUsedByInfo; +typedef virUsedByInfo *virUsedByInfoPtr; struct _virSCSIDevice { unsigned int adapter; @@ -264,6 +264,7 @@ virSCSIDeviceUsedByInfoFree(virUsedByInfoPtr used_by) VIR_FREE(used_by->domname); VIR_FREE(used_by); } +VIR_DEFINE_AUTOPTR_FUNC(virUsedByInfo, virSCSIDeviceUsedByInfoFree) void virSCSIDeviceFree(virSCSIDevicePtr dev) diff --git a/src/util/virscsi.h b/src/util/virscsi.h index 9f8b3ec..b96d862 100644 --- a/src/util/virscsi.h +++ b/src/util/virscsi.h @@ -26,6 +26,7 @@ # include "internal.h" # include "virobject.h" +# include "viralloc.h" typedef struct _virSCSIDevice virSCSIDevice; typedef virSCSIDevice *virSCSIDevicePtr; @@ -95,4 +96,6 @@ void virSCSIDeviceListDel(virSCSIDeviceListPtr list, virSCSIDevicePtr virSCSIDeviceListFind(virSCSIDeviceListPtr list, virSCSIDevicePtr dev); +VIR_DEFINE_AUTOPTR_FUNC(virSCSIDevice, virSCSIDeviceFree) + #endif /* __VIR_SCSI_H__ */ -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virscsi.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 33292f6..ba0a688 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -115,7 +115,7 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, { DIR *dir = NULL; struct dirent *entry; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *sg = NULL; unsigned int adapter_id; const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_DEVICES; @@ -139,7 +139,6 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix, cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(path); return sg; } @@ -155,7 +154,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, { DIR *dir = NULL; struct dirent *entry; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *name = NULL; unsigned int adapter_id; const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_DEVICES; @@ -178,7 +177,6 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix, cleanup: VIR_DIR_CLOSE(dir); - VIR_FREE(path); return name; } @@ -192,11 +190,11 @@ virSCSIDeviceNew(const char *sysfs_prefix, bool shareable) { virSCSIDevicePtr dev, ret = NULL; - char *sg = NULL; - char *vendor_path = NULL; - char *model_path = NULL; - char *vendor = NULL; - char *model = NULL; + VIR_AUTOFREE(char *) sg = NULL; + VIR_AUTOFREE(char *) vendor_path = NULL; + VIR_AUTOFREE(char *) model_path = NULL; + VIR_AUTOFREE(char *) vendor = NULL; + VIR_AUTOFREE(char *) model = NULL; const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_DEVICES; if (VIR_ALLOC(dev) < 0) @@ -247,11 +245,6 @@ virSCSIDeviceNew(const char *sysfs_prefix, ret = dev; cleanup: - VIR_FREE(sg); - VIR_FREE(vendor); - VIR_FREE(model); - VIR_FREE(vendor_path); - VIR_FREE(model_path); if (!ret) virSCSIDeviceFree(dev); return ret; -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virscsi.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/util/virscsi.c b/src/util/virscsi.c index ba0a688..abe699b 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -189,7 +189,8 @@ virSCSIDeviceNew(const char *sysfs_prefix, bool readonly, bool shareable) { - virSCSIDevicePtr dev, ret = NULL; + VIR_AUTOPTR(virSCSIDevice) dev = NULL; + virSCSIDevicePtr ret = NULL; VIR_AUTOFREE(char *) sg = NULL; VIR_AUTOFREE(char *) vendor_path = NULL; VIR_AUTOFREE(char *) model_path = NULL; @@ -207,46 +208,44 @@ virSCSIDeviceNew(const char *sysfs_prefix, dev->shareable = shareable; if (!(sg = virSCSIDeviceGetSgName(prefix, adapter, bus, target, unit))) - goto cleanup; + return NULL; if (virSCSIDeviceGetAdapterId(adapter, &dev->adapter) < 0) - goto cleanup; + return NULL; if (virAsprintf(&dev->name, "%d:%u:%u:%llu", dev->adapter, dev->bus, dev->target, dev->unit) < 0 || virAsprintf(&dev->sg_path, "%s/%s", sysfs_prefix ? sysfs_prefix : "/dev", sg) < 0) - goto cleanup; + return NULL; if (!virFileExists(dev->sg_path)) { virReportSystemError(errno, _("SCSI device '%s': could not access %s"), dev->name, dev->sg_path); - goto cleanup; + return NULL; } if (virAsprintf(&vendor_path, "%s/%s/vendor", prefix, dev->name) < 0 || virAsprintf(&model_path, "%s/%s/model", prefix, dev->name) < 0) - goto cleanup; + return NULL; if (virFileReadAll(vendor_path, 1024, &vendor) < 0) - goto cleanup; + return NULL; if (virFileReadAll(model_path, 1024, &model) < 0) - goto cleanup; + return NULL; virTrimSpaces(vendor, NULL); virTrimSpaces(model, NULL); if (virAsprintf(&dev->id, "%s:%s", vendor, model) < 0) - goto cleanup; + return NULL; ret = dev; - cleanup: - if (!ret) - virSCSIDeviceFree(dev); + dev = NULL; return ret; } @@ -281,21 +280,17 @@ virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, const char *drvname, const char *domname) { - virUsedByInfoPtr copy; + VIR_AUTOPTR(virUsedByInfo) copy = NULL; if (VIR_ALLOC(copy) < 0) return -1; if (VIR_STRDUP(copy->drvname, drvname) < 0 || VIR_STRDUP(copy->domname, domname) < 0) - goto cleanup; + return -1; if (VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy) < 0) - goto cleanup; + return -1; return 0; - - cleanup: - virSCSIDeviceUsedByInfoFree(copy); - return -1; } bool @@ -442,7 +437,6 @@ virSCSIDeviceListDel(virSCSIDeviceListPtr list, const char *drvname, const char *domname) { - virSCSIDevicePtr tmp = NULL; size_t i; for (i = 0; i < dev->n_used_by; i++) { @@ -452,8 +446,8 @@ virSCSIDeviceListDel(virSCSIDeviceListPtr list, virSCSIDeviceUsedByInfoFree(dev->used_by[i]); VIR_DELETE_ELEMENT(dev->used_by, i, dev->n_used_by); } else { - tmp = virSCSIDeviceListSteal(list, dev); - virSCSIDeviceFree(tmp); + VIR_AUTOPTR(virSCSIDevice) tmp = + virSCSIDeviceListSteal(list, dev); } break; } -- 1.8.3.1

Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in src/util/viralloc.h, define a new wrapper around an existing cleanup function which will be called when a variable declared with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant viralloc.h include, since that has moved from the source module into the header. When a variable of type virSCSIVHostDevicePtr is declared using VIR_AUTOPTR, the function virSCSIVHostDeviceFree will be run automatically on it when it goes out of scope. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virscsivhost.c | 1 - src/util/virscsivhost.h | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index 84c09e8..ef216b3 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -26,7 +26,6 @@ #include "virscsivhost.h" #include "virlog.h" -#include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virstring.h" diff --git a/src/util/virscsivhost.h b/src/util/virscsivhost.h index 21887dd..31ad429 100644 --- a/src/util/virscsivhost.h +++ b/src/util/virscsivhost.h @@ -27,6 +27,7 @@ # include "internal.h" # include "virobject.h" # include "virutil.h" +# include "viralloc.h" typedef struct _virSCSIVHostDevice virSCSIVHostDevice; typedef virSCSIVHostDevice *virSCSIVHostDevicePtr; @@ -63,4 +64,6 @@ void virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevicePtr dev, void virSCSIVHostDeviceFree(virSCSIVHostDevicePtr dev); int virSCSIVHostOpenVhostSCSI(int *vhostfd) ATTRIBUTE_NOINLINE; +VIR_DEFINE_AUTOPTR_FUNC(virSCSIVHostDevice, virSCSIVHostDeviceFree) + #endif /* __VIR_SCSIHOST_H__ */ -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virscsivhost.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c index ef216b3..3cd421e 100644 --- a/src/util/virscsivhost.c +++ b/src/util/virscsivhost.c @@ -109,8 +109,7 @@ void virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list, virSCSIVHostDevicePtr dev) { - virSCSIVHostDevicePtr tmp = virSCSIVHostDeviceListSteal(list, dev); - virSCSIVHostDeviceFree(tmp); + VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list, dev); } @@ -271,13 +270,11 @@ virSCSIVHostDeviceNew(const char *name) VIR_DEBUG("%s: initialized", dev->name); - cleanup: return dev; error: virSCSIVHostDeviceFree(dev); - dev = NULL; - goto cleanup; + return NULL; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virhostdev.c | 91 +++++++++++++++++---------------------------------- 1 file changed, 30 insertions(+), 61 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index f4bd19d..c4335a9 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -186,17 +186,14 @@ virHostdevManagerNew(void) goto error; } } else { - char *rundir = NULL; + VIR_AUTOFREE(char *) rundir = NULL; mode_t old_umask; if (!(rundir = virGetUserRuntimeDirectory())) goto error; - if (virAsprintf(&hostdevMgr->stateDir, "%s/hostdevmgr", rundir) < 0) { - VIR_FREE(rundir); + if (virAsprintf(&hostdevMgr->stateDir, "%s/hostdevmgr", rundir) < 0) goto error; - } - VIR_FREE(rundir); old_umask = umask(077); @@ -289,17 +286,12 @@ virHostdevPCISysfsPath(virDomainHostdevDefPtr hostdev, static int virHostdevIsVirtualFunction(virDomainHostdevDefPtr hostdev) { - char *sysfs_path = NULL; - int ret = -1; + VIR_AUTOFREE(char *) sysfs_path = NULL; if (virHostdevPCISysfsPath(hostdev, &sysfs_path) < 0) - return ret; + return -1; - ret = virPCIIsVirtualFunction(sysfs_path); - - VIR_FREE(sysfs_path); - - return ret; + return virPCIIsVirtualFunction(sysfs_path); } @@ -309,17 +301,15 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, int *vf) { - int ret = -1; - char *sysfs_path = NULL; + VIR_AUTOFREE(char *) sysfs_path = NULL; if (virHostdevPCISysfsPath(hostdev, &sysfs_path) < 0) - return ret; + return -1; if (virPCIIsVirtualFunction(sysfs_path) == 1) { if (virPCIGetVirtualFunctionInfo(sysfs_path, pfNetDevIdx, - linkdev, vf) < 0) { - goto cleanup; - } + linkdev, vf) < 0) + return -1; } else { /* In practice this should never happen, since we currently * only support assigning SRIOV VFs via <interface @@ -327,24 +317,19 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, * end up calling this function. */ if (virPCIGetNetName(sysfs_path, 0, NULL, linkdev) < 0) - goto cleanup; + return -1; if (!linkdev) { virReportError(VIR_ERR_INTERNAL_ERROR, _("The device at %s has no network device name"), sysfs_path); - goto cleanup; + return -1; } *vf = -1; } - ret = 0; - - cleanup: - VIR_FREE(sysfs_path); - - return ret; + return 0; } @@ -443,8 +428,7 @@ static int virHostdevSaveNetConfig(virDomainHostdevDefPtr hostdev, const char *stateDir) { - int ret = -1; - char *linkdev = NULL; + VIR_AUTOFREE(char *) linkdev = NULL; int vf = -1; if (!virHostdevIsPCINetDevice(hostdev) || @@ -455,19 +439,16 @@ virHostdevSaveNetConfig(virDomainHostdevDefPtr hostdev, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Interface type hostdev is currently supported on" " SR-IOV Virtual Functions only")); - goto cleanup; + return -1; } if (virHostdevNetDevice(hostdev, -1, &linkdev, &vf) < 0) - goto cleanup; + return -1; if (virNetDevSaveNetConfig(linkdev, vf, stateDir, true) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - VIR_FREE(linkdev); - return ret; + return 0; } @@ -486,10 +467,9 @@ static int virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, const unsigned char *uuid) { - char *linkdev = NULL; + VIR_AUTOFREE(char *) linkdev = NULL; virNetDevVlanPtr vlan; virNetDevVPortProfilePtr virtPort; - int ret = -1; int vf = -1; bool port_profile_associate = true; @@ -497,7 +477,7 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, return 0; if (virHostdevNetDevice(hostdev, -1, &linkdev, &vf) < 0) - goto cleanup; + return -1; vlan = virDomainNetGetActualVlan(hostdev->parent.data.net); virtPort = virDomainNetGetActualVirtPortProfile(hostdev->parent.data.net); @@ -507,24 +487,19 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, _("direct setting of the vlan tag is not allowed " "for hostdev devices using %s mode"), virNetDevVPortTypeToString(virtPort->virtPortType)); - goto cleanup; + return -1; } if (virHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, &hostdev->parent.data.net->mac, - uuid, port_profile_associate) < 0) { - goto cleanup; - } + uuid, port_profile_associate) < 0) + return -1; } else { if (virNetDevSetNetConfig(linkdev, vf, &hostdev->parent.data.net->mac, - vlan, NULL, true) < 0) { - goto cleanup; - } + vlan, NULL, true) < 0) + return -1; } - ret = 0; - cleanup: - VIR_FREE(linkdev); - return ret; + return 0; } @@ -540,13 +515,13 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, const char *stateDir, const char *oldStateDir) { - char *linkdev = NULL; + VIR_AUTOFREE(char *) linkdev = NULL; + VIR_AUTOFREE(virMacAddrPtr) MAC = NULL; + VIR_AUTOFREE(virMacAddrPtr) adminMAC = NULL; virNetDevVPortProfilePtr virtPort; int ret = -1; int vf = -1; bool port_profile_associate = false; - virMacAddrPtr MAC = NULL; - virMacAddrPtr adminMAC = NULL; virNetDevVlanPtr vlan = NULL; @@ -656,9 +631,6 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, } cleanup: - VIR_FREE(linkdev); - VIR_FREE(MAC); - VIR_FREE(adminMAC); virNetDevVlanFree(vlan); return ret; @@ -763,8 +735,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, mgr->inactivePCIHostdevs) < 0) goto reattachdevs; } else { - char *driverPath; - char *driverName; + VIR_AUTOFREE(char *) driverPath = NULL; + VIR_AUTOFREE(char *) driverName = NULL; int stub; /* Unmanaged devices should already have been marked as @@ -790,9 +762,6 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, stub = virPCIStubDriverTypeFromString(driverName); - VIR_FREE(driverPath); - VIR_FREE(driverName); - if (stub > VIR_PCI_STUB_DRIVER_NONE && stub < VIR_PCI_STUB_DRIVER_LAST) { -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virhostdev.c | 71 ++++++++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index c4335a9..b5315b1 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -518,11 +518,10 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, VIR_AUTOFREE(char *) linkdev = NULL; VIR_AUTOFREE(virMacAddrPtr) MAC = NULL; VIR_AUTOFREE(virMacAddrPtr) adminMAC = NULL; + VIR_AUTOPTR(virNetDevVlan) vlan = NULL; virNetDevVPortProfilePtr virtPort; - int ret = -1; int vf = -1; bool port_profile_associate = false; - virNetDevVlanPtr vlan = NULL; /* This is only needed for PCI devices that have been defined @@ -535,16 +534,16 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Interface type hostdev is currently supported on" " SR-IOV Virtual Functions only")); - return ret; + return -1; } if (virHostdevNetDevice(hostdev, 0, &linkdev, &vf) < 0) - return ret; + return -1; virtPort = virDomainNetGetActualVirtPortProfile( hostdev->parent.data.net); if (virtPort) { - ret = virHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, + return virHostdevNetConfigVirtPortProfile(linkdev, vf, virtPort, &hostdev->parent.data.net->mac, NULL, port_profile_associate); @@ -574,7 +573,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, /* 1) standard location */ if (virNetDevReadNetConfig(linkdev, vf, stateDir, &adminMAC, &vlan, &MAC) < 0) { - goto cleanup; + return -1; } /* 2) "old" (pre-1.2.3 circa 2014) location - whenever we get @@ -585,7 +584,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, if (!(adminMAC || vlan || MAC) && oldStateDir && virNetDevReadNetConfig(linkdev, vf, oldStateDir, &adminMAC, &vlan, &MAC) < 0) { - goto cleanup; + return -1; } /* 3) try using the PF's "port 2" netdev as the name of the @@ -597,7 +596,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) < 0 || virNetDevReadNetConfig(linkdev, vf, stateDir, &adminMAC, &vlan, &MAC) < 0) { - goto cleanup; + return -1; } } @@ -627,13 +626,8 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, ignore_value(virNetDevSetNetConfig(linkdev, vf, adminMAC, vlan, MAC, true)); - ret = 0; + return 0; } - - cleanup: - virNetDevVlanFree(vlan); - - return ret; } int @@ -1117,7 +1111,6 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, const char *dom_name) { virDomainHostdevDefPtr hostdev = NULL; - virPCIDevicePtr actual = NULL; size_t i; int ret = -1; @@ -1128,6 +1121,7 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->inactivePCIHostdevs); for (i = 0; i < nhostdevs; i++) { + VIR_AUTOPTR(virPCIDevice) actual = NULL; virDomainHostdevSubsysPCIPtr pcisrc; hostdev = hostdevs[i]; pcisrc = &hostdev->source.subsys.u.pci; @@ -1165,7 +1159,6 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, ret = 0; cleanup: - virPCIDeviceFree(actual); virObjectUnlock(mgr->activePCIHostdevs); virObjectUnlock(mgr->inactivePCIHostdevs); return ret; @@ -1226,31 +1219,27 @@ virHostdevUpdateActiveSCSIHostDevices(virHostdevManagerPtr mgr, virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = NULL; virSCSIDevicePtr tmp = NULL; - int ret = -1; if (!(scsi = virSCSIDeviceNew(NULL, scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit, hostdev->readonly, hostdev->shareable))) - goto cleanup; + return -1; if ((tmp = virSCSIDeviceListFind(mgr->activeSCSIHostdevs, scsi))) { if (virSCSIDeviceSetUsedBy(tmp, drv_name, dom_name) < 0) { virSCSIDeviceFree(scsi); - goto cleanup; + return -1; } virSCSIDeviceFree(scsi); } else { if (virSCSIDeviceSetUsedBy(scsi, drv_name, dom_name) < 0 || virSCSIDeviceListAdd(mgr->activeSCSIHostdevs, scsi) < 0) { virSCSIDeviceFree(scsi); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - return ret; + return 0; } int @@ -1301,7 +1290,7 @@ virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr, { int ret = -1; size_t i; - virMediatedDevicePtr mdev = NULL; + VIR_AUTOPTR(virMediatedDevice) mdev = NULL; if (nhostdevs == 0) return 0; @@ -1327,7 +1316,6 @@ virHostdevUpdateActiveMediatedDevices(virHostdevManagerPtr mgr, ret = 0; cleanup: - virMediatedDeviceFree(mdev); virObjectUnlock(mgr->activeMediatedHostdevs); return ret; } @@ -1560,29 +1548,25 @@ virHostdevPrepareSCSIHostDevices(virDomainHostdevDefPtr hostdev, { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi; - int ret = -1; if (hostdev->managed) { virReportError(VIR_ERR_XML_ERROR, "%s", _("SCSI host device doesn't support managed mode")); - goto cleanup; + return -1; } if (!(scsi = virSCSIDeviceNew(NULL, scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit, hostdev->readonly, hostdev->shareable))) - goto cleanup; + return -1; if (virSCSIDeviceListAdd(list, scsi) < 0) { virSCSIDeviceFree(scsi); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } int @@ -1859,7 +1843,8 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr, for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; - virUSBDevicePtr usb, tmp; + VIR_AUTOPTR(virUSBDevice) usb = NULL; + virUSBDevicePtr tmp; const char *usedby_drvname; const char *usedby_domname; @@ -1883,7 +1868,6 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr, * the list which were taken by @name */ tmp = virUSBDeviceListFind(mgr->activeUSBHostdevs, usb); - virUSBDeviceFree(usb); if (!tmp) { VIR_WARN("Unable to find device %03d.%03d " @@ -1911,7 +1895,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr mgr, const char *dom_name) { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - virSCSIDevicePtr scsi; + VIR_AUTOPTR(virSCSIDevice) scsi = NULL; virSCSIDevicePtr tmp; if (!(scsi = virSCSIDeviceNew(NULL, @@ -1932,7 +1916,6 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr mgr, "in list of active SCSI devices", scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit); - virSCSIDeviceFree(scsi); return; } @@ -1942,7 +1925,6 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr mgr, virSCSIDeviceListDel(mgr->activeSCSIHostdevs, tmp, drv_name, dom_name); - virSCSIDeviceFree(scsi); } void @@ -1982,14 +1964,14 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr, int nhostdevs) { size_t i; - virSCSIVHostDevicePtr host, tmp; - if (!nhostdevs) return; virObjectLock(mgr->activeSCSIVHostHostdevs); for (i = 0; i < nhostdevs; i++) { + VIR_AUTOPTR(virSCSIVHostDevice) host = NULL; + virSCSIVHostDevicePtr tmp; virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysSCSIVHostPtr hostsrc = &hostdev->source.subsys.u.scsi_host; const char *usedby_drvname; @@ -2017,7 +1999,6 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr, VIR_WARN("Unable to find device %s " "in list of active SCSI_host devices", hostsrc->wwpn); - virSCSIVHostDeviceFree(host); virObjectUnlock(mgr->activeSCSIVHostHostdevs); return; } @@ -2031,8 +2012,6 @@ virHostdevReAttachSCSIVHostDevices(virHostdevManagerPtr mgr, virSCSIVHostDeviceListDel(mgr->activeSCSIVHostHostdevs, tmp); } - - virSCSIVHostDeviceFree(host); } virObjectUnlock(mgr->activeSCSIVHostHostdevs); } @@ -2060,7 +2039,8 @@ virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->activeMediatedHostdevs); for (i = 0; i < nhostdevs; i++) { - virMediatedDevicePtr mdev, tmp; + VIR_AUTOPTR(virMediatedDevice) mdev = NULL; + virMediatedDevicePtr tmp; virDomainHostdevSubsysMediatedDevPtr mdevsrc; virDomainHostdevDefPtr hostdev = hostdevs[i]; @@ -2076,7 +2056,6 @@ virHostdevReAttachMediatedDevices(virHostdevManagerPtr mgr, /* Remove from the list only mdevs assigned to @drv_name/@dom_name */ tmp = virMediatedDeviceListFind(mgr->activeMediatedHostdevs, mdev); - virMediatedDeviceFree(mdev); /* skip inactive devices */ if (!tmp) -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virhostmem.c | 57 ++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 39 deletions(-) diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index c923a1e..08ed7a5 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -263,7 +263,7 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, #ifdef __linux__ { int ret; - char *meminfo_path = NULL; + VIR_AUTOFREE(char *) meminfo_path = NULL; FILE *meminfo; int max_node; @@ -299,12 +299,10 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, if (!meminfo) { virReportSystemError(errno, _("cannot open %s"), meminfo_path); - VIR_FREE(meminfo_path); return -1; } ret = virHostMemGetStatsLinux(meminfo, cellNum, params, nparams); VIR_FORCE_FCLOSE(meminfo); - VIR_FREE(meminfo_path); return ret; } @@ -322,45 +320,36 @@ virHostMemGetStats(int cellNum ATTRIBUTE_UNUSED, static int virHostMemSetParameterValue(virTypedParameterPtr param) { - char *path = NULL; - char *strval = NULL; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) strval = NULL; int rc = -1; char *field = strchr(param->field, '_'); sa_assert(field); field++; if (virAsprintf(&path, "%s/%s", - SYSFS_MEMORY_SHARED_PATH, field) < 0) { - ret = -2; - goto cleanup; - } + SYSFS_MEMORY_SHARED_PATH, field) < 0) + return -2; - if (virAsprintf(&strval, "%u", param->value.ui) == -1) { - ret = -2; - goto cleanup; - } + if (virAsprintf(&strval, "%u", param->value.ui) == -1) + return -2; if ((rc = virFileWriteStr(path, strval, 0)) < 0) { virReportSystemError(-rc, _("failed to set %s"), param->field); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(strval); - return ret; + return 0; } static bool virHostMemParametersAreAllSupported(virTypedParameterPtr params, int nparams) { - char *path = NULL; size_t i; for (i = 0; i < nparams; i++) { + VIR_AUTOFREE(char *) path = NULL; virTypedParameterPtr param = &params[i]; char *field = strchr(param->field, '_'); @@ -374,11 +363,8 @@ virHostMemParametersAreAllSupported(virTypedParameterPtr params, virReportError(VIR_ERR_OPERATION_INVALID, _("Parameter '%s' is not supported by " "this kernel"), param->field); - VIR_FREE(path); return false; } - - VIR_FREE(path); } return true; @@ -430,23 +416,20 @@ static int virHostMemGetParameterValue(const char *field, void *value) { - char *path = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) buf = NULL; char *tmp = NULL; - int ret = -1; int rc = -1; if (virAsprintf(&path, "%s/%s", SYSFS_MEMORY_SHARED_PATH, field) < 0) - goto cleanup; + return -1; - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return -1; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -465,14 +448,10 @@ virHostMemGetParameterValue(const char *field, if (rc < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse %s"), field); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(buf); - return ret; + return 0; } #endif -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/viriptables.c | 52 +++++++++++++++++--------------------------------- 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index e921954..e65e8dc 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -215,7 +215,7 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, unsigned int prefix) { virSocketAddr network; - char *netstr; + VIR_AUTOFREE(char *) netstr = NULL; char *ret; if (!(VIR_SOCKET_ADDR_IS_FAMILY(netaddr, AF_INET) || @@ -238,7 +238,6 @@ static char *iptablesFormatNetwork(virSocketAddr *netaddr, ignore_value(virAsprintf(&ret, "%s/%d", netstr, prefix)); - VIR_FREE(netstr); return ret; } @@ -254,7 +253,7 @@ iptablesForwardAllowOut(virFirewallPtr fw, const char *physdev, int action) { - char *networkstr; + VIR_AUTOFREE(char *) networkstr = NULL; virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? VIR_FIREWALL_LAYER_IPV4 : VIR_FIREWALL_LAYER_IPV6; @@ -279,7 +278,6 @@ iptablesForwardAllowOut(virFirewallPtr fw, "--jump", "ACCEPT", NULL); - VIR_FREE(networkstr); return 0; } @@ -343,7 +341,7 @@ iptablesForwardAllowRelatedIn(virFirewallPtr fw, { virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? VIR_FIREWALL_LAYER_IPV4 : VIR_FIREWALL_LAYER_IPV6; - char *networkstr; + VIR_AUTOFREE(char *) networkstr = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; @@ -370,7 +368,6 @@ iptablesForwardAllowRelatedIn(virFirewallPtr fw, "--jump", "ACCEPT", NULL); - VIR_FREE(networkstr); return 0; } @@ -432,7 +429,7 @@ iptablesForwardAllowIn(virFirewallPtr fw, { virFirewallLayer layer = VIR_SOCKET_ADDR_FAMILY(netaddr) == AF_INET ? VIR_FIREWALL_LAYER_IPV4 : VIR_FIREWALL_LAYER_IPV6; - char *networkstr; + VIR_AUTOFREE(char *) networkstr = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; @@ -454,7 +451,6 @@ iptablesForwardAllowIn(virFirewallPtr fw, "--out-interface", iface, "--jump", "ACCEPT", NULL); - VIR_FREE(networkstr); return 0; } @@ -661,12 +657,11 @@ iptablesForwardMasquerade(virFirewallPtr fw, const char *protocol, int action) { - int ret = -1; - char *networkstr = NULL; - char *addrStartStr = NULL; - char *addrEndStr = NULL; - char *portRangeStr = NULL; - char *natRangeStr = NULL; + VIR_AUTOFREE(char *) networkstr = NULL; + VIR_AUTOFREE(char *) addrStartStr = NULL; + VIR_AUTOFREE(char *) addrEndStr = NULL; + VIR_AUTOFREE(char *) portRangeStr = NULL; + VIR_AUTOFREE(char *) natRangeStr = NULL; virFirewallRulePtr rule; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) @@ -677,15 +672,15 @@ iptablesForwardMasquerade(virFirewallPtr fw, virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempted to NAT '%s'. NAT is only supported for IPv4."), networkstr); - goto cleanup; + return -1; } if (VIR_SOCKET_ADDR_IS_FAMILY(&addr->start, AF_INET)) { if (!(addrStartStr = virSocketAddrFormat(&addr->start))) - goto cleanup; + return -1; if (VIR_SOCKET_ADDR_IS_FAMILY(&addr->end, AF_INET)) { if (!(addrEndStr = virSocketAddrFormat(&addr->end))) - goto cleanup; + return -1; } } @@ -718,7 +713,7 @@ iptablesForwardMasquerade(virFirewallPtr fw, if (port->start < port->end && port->end < 65536) { if (virAsprintf(&portRangeStr, ":%u-%u", port->start, port->end) < 0) - goto cleanup; + return -1; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid port range '%u-%u'."), @@ -739,7 +734,7 @@ iptablesForwardMasquerade(virFirewallPtr fw, } if (r < 0) - goto cleanup; + return -1; virFirewallRuleAddArgList(fw, rule, "--jump", "SNAT", @@ -753,14 +748,7 @@ iptablesForwardMasquerade(virFirewallPtr fw, "--to-ports", &portRangeStr[1], NULL); } - ret = 0; - cleanup: - VIR_FREE(networkstr); - VIR_FREE(addrStartStr); - VIR_FREE(addrEndStr); - VIR_FREE(portRangeStr); - VIR_FREE(natRangeStr); - return ret; + return 0; } /** @@ -827,8 +815,7 @@ iptablesForwardDontMasquerade(virFirewallPtr fw, const char *destaddr, int action) { - int ret = -1; - char *networkstr = NULL; + VIR_AUTOFREE(char *) networkstr = NULL; if (!(networkstr = iptablesFormatNetwork(netaddr, prefix))) return -1; @@ -838,7 +825,7 @@ iptablesForwardDontMasquerade(virFirewallPtr fw, virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempted to NAT '%s'. NAT is only supported for IPv4."), networkstr); - goto cleanup; + return -1; } if (physdev && physdev[0]) @@ -859,10 +846,7 @@ iptablesForwardDontMasquerade(virFirewallPtr fw, "--jump", "RETURN", NULL); - ret = 0; - cleanup: - VIR_FREE(networkstr); - return ret; + return 0; } /** -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/viriscsi.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index d4c745a..13fd02c 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -80,7 +80,7 @@ virISCSIGetSession(const char *devpath, .session = NULL, .devpath = devpath, }; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; int exitstatus = 0; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", @@ -101,7 +101,6 @@ virISCSIGetSession(const char *devpath, NULLSTR(error)); cleanup: - VIR_FREE(error); virCommandFree(cmd); return cbdata.session; } @@ -120,7 +119,10 @@ virStorageBackendIQNFound(const char *initiatoriqn, int ret = IQN_MISSING, fd = -1; char ebuf[64]; FILE *fp = NULL; - char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL; + VIR_AUTOFREE(char *) line = NULL; + char *newline = NULL; + char *iqn = NULL; + char *token = NULL; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL); @@ -192,7 +194,6 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (ret == IQN_MISSING) VIR_DEBUG("Could not find interface with IQN '%s'", iqn); - VIR_FREE(line); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(fd); virCommandFree(cmd); @@ -206,7 +207,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { int ret = -1, exitstatus = -1; - char *temp_ifacename; + VIR_AUTOFREE(char *) temp_ifacename = NULL; virCommandPtr cmd = NULL; if (virAsprintf(&temp_ifacename, @@ -267,7 +268,6 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, cleanup: virCommandFree(cmd); - VIR_FREE(temp_ifacename); if (ret != 0) VIR_FREE(*ifacename); return ret; @@ -289,7 +289,7 @@ virISCSIConnection(const char *portal, NULL }; virCommandPtr cmd; - char *ifacename = NULL; + VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); virCommandAddArgSet(cmd, extraargv); @@ -326,7 +326,6 @@ virISCSIConnection(const char *portal, cleanup: virCommandFree(cmd); - VIR_FREE(ifacename); return ret; } @@ -377,15 +376,13 @@ virISCSIGetTargets(char **const groups, void *data) { struct virISCSITargetList *list = data; - char *target; + VIR_AUTOFREE(char *) target = NULL; if (VIR_STRDUP(target, groups[1]) < 0) return -1; - if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) { - VIR_FREE(target); + if (VIR_APPEND_ELEMENT(list->targets, list->ntargets, target) < 0) return -1; - } return 0; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/viriscsi.c | 68 +++++++++++++++++------------------------------------ 1 file changed, 22 insertions(+), 46 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 13fd02c..a3367bc 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -83,7 +83,7 @@ virISCSIGetSession(const char *devpath, VIR_AUTOFREE(char *) error = NULL; int exitstatus = 0; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL); virCommandSetErrorBuffer(cmd, &error); @@ -93,15 +93,13 @@ virISCSIGetSession(const char *devpath, vars, virISCSIExtractSession, &cbdata, NULL, &exitstatus) < 0) - goto cleanup; + return cbdata.session; if (cbdata.session == NULL && !probe) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find iscsiadm session: %s"), NULLSTR(error)); - cleanup: - virCommandFree(cmd); return cbdata.session; } @@ -123,7 +121,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, char *newline = NULL; char *iqn = NULL; char *token = NULL; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL); if (VIR_ALLOC_N(line, LINE_SIZE) != 0) { @@ -196,7 +194,6 @@ virStorageBackendIQNFound(const char *initiatoriqn, VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(fd); - virCommandFree(cmd); return ret; } @@ -208,7 +205,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, { int ret = -1, exitstatus = -1; VIR_AUTOFREE(char *) temp_ifacename = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virAsprintf(&temp_ifacename, "libvirt-iface-%08llx", @@ -267,7 +264,6 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, ret = 0; cleanup: - virCommandFree(cmd); if (ret != 0) VIR_FREE(*ifacename); return ret; @@ -280,7 +276,6 @@ virISCSIConnection(const char *portal, const char *target, const char **extraargv) { - int ret = -1; const char *const baseargv[] = { ISCSIADM, "--mode", "node", @@ -288,7 +283,7 @@ virISCSIConnection(const char *portal, "--targetname", target, NULL }; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOFREE(char *) ifacename = NULL; cmd = virCommandNewArgs(baseargv); @@ -301,7 +296,7 @@ virISCSIConnection(const char *portal, break; case IQN_MISSING: if (virStorageBackendCreateIfaceIQN(initiatoriqn, &ifacename) != 0) - goto cleanup; + return -1; /* * iscsiadm doesn't let you send commands to the Interface IQN, * unless you've first issued a 'sendtargets' command to the @@ -309,25 +304,20 @@ virISCSIConnection(const char *portal, * "iscsiadm: No records found" */ if (virISCSIScanTargets(portal, NULL, NULL) < 0) - goto cleanup; + return -1; break; case IQN_ERROR: default: - goto cleanup; + return -1; } virCommandAddArgList(cmd, "--interface", ifacename, NULL); } if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } @@ -354,14 +344,12 @@ virISCSIConnectionLogout(const char *portal, int virISCSIRescanLUNs(const char *session) { - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", "-r", session, "-R", NULL); - int ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -409,8 +397,7 @@ virISCSIScanTargets(const char *portal, int vars[] = { 2 }; struct virISCSITargetList list; size_t i; - int ret = -1; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, @@ -425,7 +412,7 @@ virISCSIScanTargets(const char *portal, vars, virISCSIGetTargets, &list, NULL, NULL) < 0) - goto cleanup; + return -1; if (ntargetsret && targetsret) { *ntargetsret = list.ntargets; @@ -436,10 +423,7 @@ virISCSIScanTargets(const char *portal, VIR_FREE(list.targets); } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /* @@ -462,9 +446,8 @@ int virISCSINodeNew(const char *portal, const char *target) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; - int ret = -1; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -477,20 +460,17 @@ virISCSINodeNew(const char *portal, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed new node mode for target '%s'"), target); - goto cleanup; + return -1; } if (status != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s failed new mode for target '%s' with status '%d'"), ISCSIADM, target, status); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -500,9 +480,8 @@ virISCSINodeUpdate(const char *portal, const char *name, const char *value) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int status; - int ret = -1; cmd = virCommandNewArgList(ISCSIADM, "--mode", "node", @@ -518,11 +497,8 @@ virISCSINodeUpdate(const char *portal, virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to update '%s' of node mode for target '%s'"), name, target); - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return 0; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virkmod.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/util/virkmod.c b/src/util/virkmod.c index 219fad6..d981cd4 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -155,13 +155,12 @@ virKModUnload(const char *module) bool virKModIsBlacklisted(const char *module) { - bool retval = false; size_t i; - char *drvblklst = NULL; - char *outbuf = NULL; + VIR_AUTOFREE(char *) drvblklst = NULL; + VIR_AUTOFREE(char *) outbuf = NULL; if (virAsprintfQuiet(&drvblklst, "blacklist %s\n", module) < 0) - goto cleanup; + return false; /* modprobe will convert all '-' into '_', so we need to as well */ for (i = 0; i < drvblklst[i]; i++) @@ -169,13 +168,10 @@ virKModIsBlacklisted(const char *module) drvblklst[i] = '_'; if (doModprobe("-c", NULL, &outbuf, NULL) < 0) - goto cleanup; + return false; if (strstr(outbuf, drvblklst)) - retval = true; + return true; - cleanup: - VIR_FREE(drvblklst); - VIR_FREE(outbuf); - return retval; + return false; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virkmod.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/util/virkmod.c b/src/util/virkmod.c index d981cd4..9d0375b 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -28,8 +28,7 @@ static int doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNew(MODPROBE); if (opts) @@ -42,32 +41,23 @@ doModprobe(const char *opts, const char *module, char **outbuf, char **errbuf) virCommandSetErrorBuffer(cmd, errbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } static int doRmmod(const char *module, char **errbuf) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(RMMOD, module, NULL); virCommandSetErrorBuffer(cmd, errbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } /** -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virlease.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virlease.c b/src/util/virlease.c index b49105d..baaceaf 100644 --- a/src/util/virlease.c +++ b/src/util/virlease.c @@ -55,7 +55,7 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, const char *ip_to_delete, char **server_duid) { - char *lease_entries = NULL; + VIR_AUTOFREE(char *) lease_entries = NULL; virJSONValuePtr leases_array = NULL; long long expirytime; int custom_lease_file_len = 0; @@ -146,7 +146,6 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, cleanup: virJSONValueFree(leases_array); - VIR_FREE(lease_entries); return ret; } @@ -235,7 +234,7 @@ virLeaseNew(virJSONValuePtr *lease_ret, virJSONValuePtr lease_new = NULL; const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); long long expirytime = 0; - char *exptime = NULL; + VIR_AUTOFREE(char *) exptime = NULL; int ret = -1; /* In case hostname is still unknown, use the last known one */ @@ -291,7 +290,6 @@ virLeaseNew(virJSONValuePtr *lease_ret, *lease_ret = lease_new; lease_new = NULL; cleanup: - VIR_FREE(exptime); virJSONValueFree(lease_new); return ret; } -- 1.8.3.1

By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virlease.c | 76 ++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 47 deletions(-) diff --git a/src/util/virlease.c b/src/util/virlease.c index baaceaf..7c6c37e 100644 --- a/src/util/virlease.c +++ b/src/util/virlease.c @@ -56,40 +56,36 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, char **server_duid) { VIR_AUTOFREE(char *) lease_entries = NULL; - virJSONValuePtr leases_array = NULL; + VIR_AUTOPTR(virJSONValue) leases_array = NULL; long long expirytime; int custom_lease_file_len = 0; virJSONValuePtr lease_tmp = NULL; const char *ip_tmp = NULL; const char *server_duid_tmp = NULL; size_t i; - int ret = -1; /* Read entire contents */ if ((custom_lease_file_len = virFileReadAll(custom_lease_file, VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, &lease_entries)) < 0) { - goto cleanup; + return -1; } /* Check for previous leases */ - if (custom_lease_file_len == 0) { - ret = 0; - goto cleanup; - } + if (custom_lease_file_len == 0) + return 0; if (!(leases_array = virJSONValueFromString(lease_entries))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid json in file: %s, rewriting it"), custom_lease_file); - ret = 0; - goto cleanup; + return 0; } if (!virJSONValueIsArray(leases_array)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("couldn't fetch array of leases")); - goto cleanup; + return -1; } i = 0; @@ -97,14 +93,14 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse json")); - goto cleanup; + return -1; } if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &expirytime) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse json")); - goto cleanup; + return -1; } /* Check whether lease has to be included or not */ @@ -121,14 +117,14 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, /* Control reaches here when the 'action' is not for an * ipv6 lease or, for some weird reason the env var * DNSMASQ_SERVER_DUID wasn't set*/ - goto cleanup; + return -1; } } else { /* Inject server-duid into those ipv6 leases which * didn't have it previously, for example, those * created by leaseshelper from libvirt 1.2.6 */ if (virJSONValueObjectAppendString(lease_tmp, "server-duid", *server_duid) < 0) - goto cleanup; + return -1; } } @@ -136,17 +132,13 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create json")); - goto cleanup; + return -1; } ignore_value(virJSONValueArraySteal(leases_array, i)); } - ret = 0; - - cleanup: - virJSONValueFree(leases_array); - return ret; + return 0; } @@ -157,7 +149,6 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new, virJSONValuePtr lease_tmp = NULL; const char *ip_tmp = NULL; long long expirytime = 0; - int ret = -1; size_t i; /* Man page of dnsmasq says: the script (helper program, in our case) @@ -174,7 +165,7 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new, if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse json")); - goto cleanup; + return -1; } if (!strchr(ip_tmp, ':')) { if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", @@ -198,7 +189,7 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new, if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse json")); - goto cleanup; + return -1; } if (strchr(ip_tmp, ':')) { if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", @@ -215,10 +206,7 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new, } } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -231,24 +219,21 @@ virLeaseNew(virJSONValuePtr *lease_ret, const char *iaid, const char *server_duid) { - virJSONValuePtr lease_new = NULL; + VIR_AUTOPTR(virJSONValue) lease_new = NULL; const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); long long expirytime = 0; VIR_AUTOFREE(char *) exptime = NULL; - int ret = -1; /* In case hostname is still unknown, use the last known one */ if (!hostname) hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME"); - if (!mac) { - ret = 0; - goto cleanup; - } + if (!mac) + return 0; if (exptime_tmp) { if (VIR_STRDUP(exptime, exptime_tmp) < 0) - goto cleanup; + return -1; /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES * (dnsmasq < 2.52) */ @@ -261,35 +246,32 @@ virLeaseNew(virJSONValuePtr *lease_ret, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to convert lease expiry time to long long: %s"), NULLSTR(exptime)); - goto cleanup; + return -1; } /* Create new lease */ if (!(lease_new = virJSONValueNewObject())) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create json")); - goto cleanup; + return -1; } if (iaid && virJSONValueObjectAppendString(lease_new, "iaid", iaid) < 0) - goto cleanup; + return -1; if (ip && virJSONValueObjectAppendString(lease_new, "ip-address", ip) < 0) - goto cleanup; + return -1; if (mac && virJSONValueObjectAppendString(lease_new, "mac-address", mac) < 0) - goto cleanup; + return -1; if (hostname && virJSONValueObjectAppendString(lease_new, "hostname", hostname) < 0) - goto cleanup; + return -1; if (clientid && virJSONValueObjectAppendString(lease_new, "client-id", clientid) < 0) - goto cleanup; + return -1; if (server_duid && virJSONValueObjectAppendString(lease_new, "server-duid", server_duid) < 0) - goto cleanup; + return -1; if (expirytime && virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", expirytime) < 0) - goto cleanup; + return -1; - ret = 0; *lease_ret = lease_new; lease_new = NULL; - cleanup: - virJSONValueFree(lease_new); - return ret; + return 0; } -- 1.8.3.1