[libvirt] [PATCH v1 08/18] use VIR_AUTOFREE in src/util/vircgroup.c

Sukrit Bhatnagar posted 18 patches 6 years, 11 months ago
[libvirt] [PATCH v1 08/18] use VIR_AUTOFREE in src/util/vircgroup.c
Posted by Sukrit Bhatnagar 6 years, 11 months ago
Modify code to use VIR_AUTOFREE macro wherever required.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 src/util/vircgroup.c | 526 ++++++++++++++++++---------------------------------
 1 file changed, 179 insertions(+), 347 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0a31947..ed86d31 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -163,7 +163,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
@@ -223,7 +223,6 @@ virCgroupPartitionNeedsEscaping(const char *path)
     }
 
  cleanup:
-    VIR_FREE(line);
     VIR_FORCE_FCLOSE(fp);
     return ret;
 }
@@ -256,41 +255,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;
@@ -303,7 +301,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
 
         tmp = strrchr(group->controllers[i].placement, '/');
         if (!tmp)
-            goto cleanup;
+            return false;
 
         if (stripEmulatorSuffix &&
             (i == VIR_CGROUP_CONTROLLER_CPU ||
@@ -313,7 +311,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
                 *tmp = '\0';
             tmp = strrchr(group->controllers[i].placement, '/');
             if (!tmp)
-                goto cleanup;
+                return false;
         }
 
         tmp++;
@@ -329,18 +327,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;
 }
 
 
@@ -378,7 +369,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
     FILE *mounts = NULL;
     struct mntent entry;
     char buf[CGROUP_MAX_VAL];
-    char *linksrc = NULL;
+    VIR_AUTOFREE(char *) linksrc = NULL;
     int ret = -1;
 
     mounts = fopen(path, "r");
@@ -468,7 +459,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
 
     ret = 0;
  cleanup:
-    VIR_FREE(linksrc);
     VIR_FORCE_FCLOSE(mounts);
     return ret;
 }
@@ -547,7 +537,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);
@@ -628,9 +618,7 @@ virCgroupDetectPlacement(virCgroupPtr group,
     ret = 0;
 
  cleanup:
-    VIR_FREE(procfile);
     VIR_FORCE_FCLOSE(mapping);
-
     return ret;
 }
 
@@ -786,8 +774,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)
@@ -800,18 +787,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;
 }
 
 
@@ -821,8 +804,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;
 
@@ -834,18 +817,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;
 }
 
 
@@ -856,8 +835,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;
 
@@ -875,8 +854,6 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,
 
     ret = 0;
  error:
-    VIR_FREE(str);
-    VIR_FREE(prefix);
     virStringListFree(lines);
     return ret;
 }
@@ -888,17 +865,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);
 }
 
 
@@ -908,17 +880,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);
 }
 
 
@@ -928,24 +895,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;
 }
 
 
@@ -955,24 +917,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;
 }
 
 
@@ -988,7 +945,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,
@@ -1001,11 +958,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;
@@ -1044,11 +998,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) {
@@ -1074,10 +1027,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
@@ -1085,24 +1036,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
@@ -1113,21 +1060,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;
 }
 
 
@@ -1339,9 +1279,9 @@ virCgroupNewPartition(const char *path,
                       virCgroupPtr *group)
 {
     int ret = -1;
-    char *parentPath = NULL;
+    VIR_AUTOFREE(char *) parentPath = NULL;
     virCgroupPtr parent = NULL;
-    char *newPath = NULL;
+    VIR_AUTOFREE(char *) newPath = NULL;
     VIR_DEBUG("path=%s create=%d controllers=%x",
               path, create, controllers);
 
@@ -1381,8 +1321,6 @@ virCgroupNewPartition(const char *path,
     if (ret != 0)
         virCgroupFree(group);
     virCgroupFree(&parent);
-    VIR_FREE(parentPath);
-    VIR_FREE(newPath);
     return ret;
 }
 
@@ -1421,18 +1359,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
@@ -1448,14 +1385,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;
 }
 
 
@@ -1477,27 +1410,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) |
@@ -1505,18 +1437,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;
 }
 
 
@@ -1577,7 +1506,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);
@@ -1662,7 +1591,6 @@ virCgroupNewMachineSystemd(const char *name,
     ret = 0;
  cleanup:
     virCgroupFree(&parent);
-    VIR_FREE(path);
     return ret;
 }
 
@@ -1894,9 +1822,10 @@ 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, *p2;
     size_t i;
-    int ret = -1;
 
     const char *value_names[] = {
         "Read ",
@@ -1919,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++) {
@@ -1938,7 +1867,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
                                _("Cannot parse byte %sstat '%s'"),
                                value_names[i],
                                p1);
-                goto cleanup;
+                return -1;
             }
 
             if (stats_val < 0 ||
@@ -1947,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;
         }
@@ -1959,7 +1888,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
                                _("Cannot parse %srequest stat '%s'"),
                                value_names[i],
                                p2);
-                goto cleanup;
+                return -1;
             }
 
             if (stats_val < 0 ||
@@ -1968,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;
 }
 
 
@@ -2003,9 +1927,11 @@ 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, *p2;
     size_t i;
-    int ret = -1;
 
     const char *value_names[] = {
         "Read ",
@@ -2023,28 +1949,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++) {
@@ -2052,38 +1978,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;
 }
 
 
@@ -2139,24 +2059,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;
 }
 
 
@@ -2173,24 +2088,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;
 }
 
 
@@ -2207,24 +2117,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;
 }
 
 /**
@@ -2240,24 +2145,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;
 }
 
 
@@ -2275,24 +2175,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;
 }
 
 /**
@@ -2308,15 +2203,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;
@@ -2324,13 +2218,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;
 }
 
 /**
@@ -2346,15 +2237,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;
@@ -2362,13 +2252,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;
 }
 
 /**
@@ -2384,15 +2271,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;
@@ -2400,13 +2286,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;
 }
 
 /**
@@ -2422,15 +2305,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;
@@ -2438,13 +2320,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;
 }
 
 /**
@@ -2460,15 +2339,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;
@@ -2476,13 +2354,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;
 }
 
 
@@ -2941,36 +2816,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;
-
-    ret = 0;
+        return -1;
 
- cleanup:
-    VIR_FREE(devstr);
-    VIR_FREE(majorstr);
-    VIR_FREE(minorstr);
-    return ret;
+    return 0;
 }
 
 
@@ -3032,36 +2900,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;
-
-    ret = 0;
+        return -1;
 
- cleanup:
-    VIR_FREE(devstr);
-    VIR_FREE(majorstr);
-    VIR_FREE(minorstr);
-    return ret;
+    return 0;
 }
 
 
@@ -3131,10 +2992,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;
@@ -3159,13 +3020,11 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
         }
 
         virCgroupFree(&group_vcpu);
-        VIR_FREE(buf);
     }
 
     ret = 0;
  cleanup:
     virCgroupFree(&group_vcpu);
-    VIR_FREE(buf);
     return ret;
 }
 
@@ -3202,8 +3061,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;
@@ -3289,8 +3148,6 @@ virCgroupGetPercpuStats(virCgroupPtr group,
 
  cleanup:
     virBitmapFree(cpumap);
-    VIR_FREE(sum_cpu_time);
-    VIR_FREE(buf);
     return ret;
 }
 
@@ -3461,7 +3318,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;
 
@@ -3470,7 +3327,6 @@ virCgroupRemoveRecursively(char *grppath)
             break;
         }
         rc = virCgroupRemoveRecursively(path);
-        VIR_FREE(path);
         if (rc != 0)
             break;
     }
@@ -3508,10 +3364,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;
@@ -3533,7 +3390,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);
 
@@ -3549,7 +3405,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",
@@ -3613,7 +3469,6 @@ virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids)
     ret = killedAny ? 1 : 0;
 
  cleanup:
-    VIR_FREE(keypath);
     VIR_FORCE_FCLOSE(fp);
 
     return ret;
@@ -3678,7 +3533,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;
@@ -3732,7 +3587,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
 
  cleanup:
     virCgroupFree(&subgroup);
-    VIR_FREE(keypath);
     VIR_DIR_CLOSE(dp);
     return ret;
 }
@@ -3846,9 +3700,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,
@@ -3860,14 +3713,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
@@ -3877,17 +3730,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;
 }
 
 
@@ -3913,10 +3763,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;
@@ -3927,18 +3776,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++) {
@@ -3946,11 +3795,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);
@@ -3958,8 +3807,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, NULL, MS_BIND,
@@ -3967,11 +3815,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) {
@@ -3984,16 +3829,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;
 }
 
 
@@ -4004,11 +3845,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))
@@ -4025,6 +3866,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;
 
@@ -4034,8 +3877,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
                                      entry, uid, gid);
                 goto cleanup;
             }
-
-            VIR_FREE(entry);
         }
         if (direrr < 0)
             goto cleanup;
@@ -4047,7 +3888,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
             goto cleanup;
         }
 
-        VIR_FREE(base);
         VIR_DIR_CLOSE(dh);
     }
 
@@ -4055,8 +3895,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
 
  cleanup:
     VIR_DIR_CLOSE(dh);
-    VIR_FREE(entry);
-    VIR_FREE(base);
     return ret;
 }
 
@@ -4071,8 +3909,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
 bool
 virCgroupSupportsCpuBW(virCgroupPtr cgroup)
 {
-    char *path = NULL;
-    bool ret = false;
+    VIR_AUTOFREE(char *) path = NULL;
 
     if (!cgroup)
         return false;
@@ -4080,21 +3917,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;
+    int ret;
+    VIR_AUTOFREE(char *) content = NULL;
 
     if (!cgroup)
         return -1;
@@ -4104,7 +3937,6 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
     if (ret == 0 && content[0] == '\0')
         ret = 1;
 
-    VIR_FREE(content);
     return ret;
 }
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 08/18] use VIR_AUTOFREE in src/util/vircgroup.c
Posted by Erik Skultety 6 years, 11 months ago
On Sun, Jun 03, 2018 at 01:42:06PM +0530, Sukrit Bhatnagar wrote:
> Modify code to use VIR_AUTOFREE macro wherever required.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  src/util/vircgroup.c | 526 ++++++++++++++++++---------------------------------
>  1 file changed, 179 insertions(+), 347 deletions(-)
>
...

> @@ -378,7 +369,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
>      FILE *mounts = NULL;
>      struct mntent entry;
>      char buf[CGROUP_MAX_VAL];
> -    char *linksrc = NULL;
> +    VIR_AUTOFREE(char *) linksrc = NULL;

@linksrc is being used only in an 'if' block further in the function body
(the context of which is not seen in the patch), so I'd move the declaration to
that block which will allow us to ditch the VIR_FREE(linksrc) in the same
block.

...

>
>
> @@ -1339,9 +1279,9 @@ virCgroupNewPartition(const char *path,
>                        virCgroupPtr *group)
>  {
>      int ret = -1;
> -    char *parentPath = NULL;
> +    VIR_AUTOFREE(char *) parentPath = NULL;
>      virCgroupPtr parent = NULL;
> -    char *newPath = NULL;
> +    VIR_AUTOFREE(char *) newPath = NULL;

move ^this one line up, so the AUTOFREEs are nicely packed...

>      VIR_DEBUG("path=%s create=%d controllers=%x",
>                path, create, controllers);
>
> @@ -1381,8 +1321,6 @@ virCgroupNewPartition(const char *path,
>      if (ret != 0)
>          virCgroupFree(group);
>      virCgroupFree(&parent);
> -    VIR_FREE(parentPath);
> -    VIR_FREE(newPath);
>      return ret;
>  }

...

> @@ -1577,7 +1506,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);
> @@ -1662,7 +1591,6 @@ virCgroupNewMachineSystemd(const char *name,
>      ret = 0;
>   cleanup:
>      virCgroupFree(&parent);
> -    VIR_FREE(path);
>      return ret;
>  }
>
> @@ -1894,9 +1822,10 @@ 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, *p2;

since you're changing it already, p1 and p2 should be on separate lines...

>      size_t i;
> -    int ret = -1;
>
>      const char *value_names[] = {
>          "Read ",
> @@ -1919,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++) {
> @@ -1938,7 +1867,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
>                                 _("Cannot parse byte %sstat '%s'"),
>                                 value_names[i],
>                                 p1);
> -                goto cleanup;
> +                return -1;
>              }
>
>              if (stats_val < 0 ||
> @@ -1947,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;
>          }
> @@ -1959,7 +1888,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
>                                 _("Cannot parse %srequest stat '%s'"),
>                                 value_names[i],
>                                 p2);
> -                goto cleanup;
> +                return -1;
>              }
>
>              if (stats_val < 0 ||
> @@ -1968,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;
>  }
>
>
> @@ -2003,9 +1927,11 @@ 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, *p2;

here too..separate lines...

...

>
>
> @@ -3131,10 +2992,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;

more cleanup possible here, since there's no point in having @pos at all, on
the contrary, it's a bit confusing what it's being used for...so @pos should be
dropped in a separate patch and @buf should be used directly, beware though:

virStrToLong_ull(buf, &buf, 10, &tmp) would leak memory, so
virStrToLong_ull(buf, NULL, 10, &tmp) should be used instead...

...

>
>  int
>  virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
>  {
> -    int ret = -1;

^This is okay, so don't change it...

> -    char *content = NULL;
> +    int ret;
> +    VIR_AUTOFREE(char *) content = NULL;
>
>      if (!cgroup)
>          return -1;
> @@ -4104,7 +3937,6 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
>      if (ret == 0 && content[0] == '\0')
>          ret = 1;
>
> -    VIR_FREE(content);
>      return ret;
>  }

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list