10:52 a.m.
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>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/util/vircgroup.c | 185 +++++++++++++++++++--------------------------------
1 file changed, 67 insertions(+), 118 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6f7b5b4..61fafe2 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;
}
@@ -1277,10 +1268,10 @@ virCgroupNewPartition(const char *path,
int controllers,
virCgroupPtr *group)
{
- int ret = -1;
VIR_AUTOFREE(char *) parentPath = NULL;
VIR_AUTOFREE(char *) newPath = NULL;
- virCgroupPtr parent = NULL;
+ VIR_AUTOPTR(virCgroup) parent = NULL;
+ VIR_AUTOPTR(virCgroup) tmpGroup = NULL;
VIR_DEBUG("path=%s create=%d controllers=%x",
path, create, controllers);
@@ -1315,12 +1306,11 @@ virCgroupNewPartition(const char *path,
}
}
- ret = 0;
+ return 0;
+
cleanup:
- if (ret != 0)
- virCgroupFree(*group);
- virCgroupFree(parent);
- return ret;
+ VIR_STEAL_PTR(tmpGroup, *group);
+ return -1;
}
@@ -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 = ¶ms[(i - start_cpu) * nparams + param_idx];
if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
- goto cleanup;
+ return -1;
}
/* return percpu vcputime in index 1 */
@@ -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(¶ms[(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,
- NULL,
- virCgroupPidCode,
- virCgroupPidEqual,
- virCgroupPidCopy,
- NULL);
+ 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,
- NULL,
- virCgroupPidCode,
- virCgroupPidEqual,
- virCgroupPidCopy,
- NULL);
+ 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