util: cgroup: use VIR_AUTOPTR for aggregate types

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@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6f7b5b40f7a9904623d2fdd8654f64e2f55c51b5..472a8167f563c56e610f1250075a2cd4819e3fd1 100644 (file)
--- 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;
-
-    ret = 0;
+        return -1;
 
- 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);
 
@@ -1292,35 +1283,31 @@ virCgroupNewPartition(const char *path,
     }
 
     if (virCgroupSetPartitionSuffix(path, &newPath) < 0)
-        goto cleanup;
+        return -1;
 
-    if (virCgroupNew(-1, newPath, NULL, controllers, group) < 0)
-        goto cleanup;
+    if (virCgroupNew(-1, newPath, NULL, controllers, &tmpGroup) < 0)
+        return -1;
 
     if (STRNEQ(newPath, "/")) {
         char *tmp;
         if (VIR_STRDUP(parentPath, newPath) < 0)
-            goto cleanup;
+            return -1;
 
         tmp = strrchr(parentPath, '/');
         tmp++;
         *tmp = '\0';
 
         if (virCgroupNew(-1, parentPath, NULL, controllers, &parent) < 0)
-            goto cleanup;
+            return -1;
 
-        if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) {
-            virCgroupRemove(*group);
-            goto cleanup;
+        if (virCgroupMakeGroup(parent, tmpGroup, create, VIR_CGROUP_NONE) < 0) {
+            virCgroupRemove(tmpGroup);
+            return -1;
         }
     }
 
-    ret = 0;
- cleanup:
-    if (ret != 0)
-        virCgroupFree(*group);
-    virCgroupFree(parent);
-    return ret;
+    VIR_STEAL_PTR(*group, tmpGroup);
+    return 0;
 }
 
 
@@ -1502,9 +1489,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 +1518,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 +1531,7 @@ virCgroupNewMachineSystemd(const char *name,
                      NULL,
                      controllers,
                      &parent) < 0)
-        goto cleanup;
+        return -1;
 
 
     for (;;) {
@@ -1560,11 +1545,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 +1572,7 @@ virCgroupNewMachineSystemd(const char *name,
         }
     }
 
-    ret = 0;
- cleanup:
-    virCgroupFree(parent);
-    return ret;
+    return 0;
 }
 
 
@@ -1611,8 +1593,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 +1601,9 @@ virCgroupNewMachineManual(const char *name,
                               controllers,
                               &parent) < 0) {
         if (virCgroupNewIgnoreError())
-            goto done;
+            return 0;
 
-        goto cleanup;
+        return -1;
     }
 
     if (virCgroupNewDomainPartition(parent,
@@ -1630,7 +1611,7 @@ virCgroupNewMachineManual(const char *name,
                                     name,
                                     true,
                                     group) < 0)
-        goto cleanup;
+        return -1;
 
     if (virCgroupAddTask(*group, pidleader) < 0) {
         virErrorPtr saved = virSaveLastError();
@@ -1642,12 +1623,7 @@ virCgroupNewMachineManual(const char *name,
         }
     }
 
- done:
-    ret = 0;
-
- cleanup:
-    virCgroupFree(parent);
-    return ret;
+    return 0;
 }
 
 
@@ -2376,7 +2352,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 +2366,6 @@ virCgroupMemoryOnceInit(void)
                                       "memory.limit_in_bytes",
                                       &mem_unlimited));
  cleanup:
-    virCgroupFree(group);
     virCgroupMemoryUnlimitedKB = mem_unlimited >> 10;
 }
 
@@ -2991,22 +2966,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 +2989,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 +3027,6 @@ virCgroupGetPercpuStats(virCgroupPtr group,
                         unsigned int ncpus,
                         virBitmapPtr guestvcpus)
 {
-    int ret = -1;
     size_t i;
     int need_cpus, total_cpus;
     char *pos;
@@ -3067,7 +3035,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 +3052,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 +3079,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 +3094,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 +3105,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 +3467,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);
-
-    ret = virCgroupKillInternal(group, signum, pids);
+    VIR_AUTOPTR(virHashTable) pids = virHashCreateFull(100,
+                                                       NULL,
+                                                       virCgroupPidCode,
+                                                       virCgroupPidEqual,
+                                                       virCgroupPidCopy,
+                                                       NULL);
 
-    virHashFree(pids);
-
-    return ret;
+    return virCgroupKillInternal(group, signum, pids);
 }
 
 
@@ -3536,7 +3493,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 +3517,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 +3535,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
 
         if (dormdir)
             virCgroupRemove(subgroup);
-
-        virCgroupFree(subgroup);
     }
     if (direrr < 0)
         goto cleanup;
@@ -3587,7 +3543,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
     ret = killedAny ? 1 : 0;
 
  cleanup:
-    virCgroupFree(subgroup);
     VIR_DIR_CLOSE(dp);
     return ret;
 }
@@ -3596,20 +3551,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 +3894,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 +4687,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 +4695,6 @@ virCgroupDelThread(virCgroupPtr cgroup,
 
         /* Remove the offlined cgroup */
         virCgroupRemove(new_cgroup);
-        virCgroupFree(new_cgroup);
     }
 
     return 0;