[libvirt] [PATCH 2/3] lxc: Resolve memory leak

John Ferlan jferlan at redhat.com
Thu Sep 20 21:34:37 UTC 2018


Commit 40b5c99a modified the virConfGetValue callers to use
virConfGetValueString. However, using the virConfGetValueString
resulted in leaking the returned @value string in each case.
So, let's modify each instance to use the VIR_AUTOFREE(char *)
syntax. In some instances changing the variable name since
@value was used more than once.

Found by Coverity

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/lxc/lxc_native.c | 110 +++++++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 47 deletions(-)

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index e1992fd1f9..6c5640536b 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -198,7 +198,7 @@ lxcSetRootfs(virDomainDefPtr def,
              virConfPtr properties)
 {
     int type = VIR_DOMAIN_FS_TYPE_MOUNT;
-    char *value = NULL;
+    VIR_AUTOFREE(char *) value = NULL;
 
     if (virConfGetValueString(properties, "lxc.rootfs", &value) <= 0)
         return -1;
@@ -679,7 +679,7 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties)
 static int
 lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) value = NULL;
     int nbttys = 0;
     virDomainChrDefPtr console;
     size_t i;
@@ -756,14 +756,20 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr value, void *data)
 static int
 lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) hard_limit = NULL;
+    VIR_AUTOFREE(char *) soft_limit = NULL;
+    VIR_AUTOFREE(char *) swap_hard_limit = NULL;
     unsigned long long size = 0;
 
     if (virConfGetValueString(properties,
                               "lxc.cgroup.memory.limit_in_bytes",
-                              &value) > 0) {
-        if (lxcConvertSize(value, &size) < 0)
-            goto error;
+                              &hard_limit) > 0) {
+        if (lxcConvertSize(hard_limit, &size) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"),
+                           hard_limit);
+            return -1;
+        }
         size = size / 1024;
         virDomainDefSetMemoryTotal(def, size);
         def->mem.hard_limit = virMemoryLimitTruncate(size);
@@ -771,76 +777,85 @@ lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
 
     if (virConfGetValueString(properties,
                               "lxc.cgroup.memory.soft_limit_in_bytes",
-                              &value) > 0) {
-        if (lxcConvertSize(value, &size) < 0)
-            goto error;
+                              &soft_limit) > 0) {
+        if (lxcConvertSize(soft_limit, &size) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"),
+                           soft_limit);
+            return -1;
+        }
         def->mem.soft_limit = virMemoryLimitTruncate(size / 1024);
     }
 
     if (virConfGetValueString(properties,
                               "lxc.cgroup.memory.memsw.limit_in_bytes",
-                              &value) > 0) {
-        if (lxcConvertSize(value, &size) < 0)
-            goto error;
+                              &swap_hard_limit) > 0) {
+        if (lxcConvertSize(swap_hard_limit, &size) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"),
+                           swap_hard_limit);
+            return -1;
+        }
         def->mem.swap_hard_limit = virMemoryLimitTruncate(size / 1024);
     }
     return 0;
-
- error:
-    virReportError(VIR_ERR_INTERNAL_ERROR,
-                   _("failed to parse integer: '%s'"), value);
-    return -1;
-
 }
 
 static int
 lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) shares = NULL;
+    VIR_AUTOFREE(char *) quota = NULL;
+    VIR_AUTOFREE(char *) period = NULL;
 
     if (virConfGetValueString(properties, "lxc.cgroup.cpu.shares",
-                              &value) > 0) {
-        if (virStrToLong_ull(value, NULL, 10, &def->cputune.shares) < 0)
-            goto error;
+                              &shares) > 0) {
+        if (virStrToLong_ull(shares, NULL, 10, &def->cputune.shares) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"), shares);
+            return -1;
+        }
         def->cputune.sharesSpecified = true;
     }
 
     if (virConfGetValueString(properties, "lxc.cgroup.cpu.cfs_quota_us",
-                              &value) > 0) {
-        if (virStrToLong_ll(value, NULL, 10, &def->cputune.quota) < 0)
-            goto error;
+                              &quota) > 0) {
+        if (virStrToLong_ll(quota, NULL, 10, &def->cputune.quota) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"), quota);
+            return -1;
+        }
     }
 
     if (virConfGetValueString(properties, "lxc.cgroup.cpu.cfs_period_us",
-                               &value) > 0) {
-        if (virStrToLong_ull(value, NULL, 10, &def->cputune.period) < 0)
-            goto error;
+                              &period) > 0) {
+        if (virStrToLong_ull(period, NULL, 10, &def->cputune.period) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("failed to parse integer: '%s'"), period);
+            return -1;
+        }
     }
 
     return 0;
-
- error:
-    virReportError(VIR_ERR_INTERNAL_ERROR,
-                   _("failed to parse integer: '%s'"), value);
-    return -1;
 }
 
 static int
 lxcSetCpusetTune(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) cpus = NULL;
+    VIR_AUTOFREE(char *) mems = NULL;
     virBitmapPtr nodeset = NULL;
 
     if (virConfGetValueString(properties, "lxc.cgroup.cpuset.cpus",
-                              &value) > 0) {
-        if (virBitmapParse(value, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
+                              &cpus) > 0) {
+        if (virBitmapParse(cpus, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
             return -1;
         def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC;
     }
 
     if (virConfGetValueString(properties, "lxc.cgroup.cpuset.mems",
-                               &value) > 0) {
-        if (virBitmapParse(value, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
+                              &mems) > 0) {
+        if (virBitmapParse(mems, &nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0)
             return -1;
         if (virDomainNumatuneSet(def->numa,
                                  def->placement_mode ==
@@ -949,7 +964,7 @@ lxcBlkioDeviceWalkCallback(const char *name, virConfValuePtr value, void *data)
 static int
 lxcSetBlkioTune(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) value = NULL;
 
     if (virConfGetValueString(properties, "lxc.cgroup.blkio.weight",
                               &value) > 0) {
@@ -969,7 +984,7 @@ lxcSetBlkioTune(virDomainDefPtr def, virConfPtr properties)
 static void
 lxcSetCapDrop(virDomainDefPtr def, virConfPtr properties)
 {
-    char *value = NULL;
+    VIR_AUTOFREE(char *) value = NULL;
     char **toDrop = NULL;
     const char *capString;
     size_t i;
@@ -996,7 +1011,8 @@ lxcParseConfigString(const char *config,
 {
     virDomainDefPtr vmdef = NULL;
     virConfPtr properties = NULL;
-    char *value = NULL;
+    VIR_AUTOFREE(char *) lxcarch = NULL;
+    VIR_AUTOFREE(char *) lxcutsname = NULL;
 
     if (!(properties = virConfReadString(config, VIR_CONF_FLAG_LXC_FORMAT)))
         return NULL;
@@ -1029,11 +1045,11 @@ lxcParseConfigString(const char *config,
     vmdef->nfss = 0;
     vmdef->os.type = VIR_DOMAIN_OSTYPE_EXE;
 
-    if (virConfGetValueString(properties, "lxc.arch", &value) > 0) {
-        virArch arch = virArchFromString(value);
-        if (arch == VIR_ARCH_NONE && STREQ(value, "x86"))
+    if (virConfGetValueString(properties, "lxc.arch", &lxcarch) > 0) {
+        virArch arch = virArchFromString(lxcarch);
+        if (arch == VIR_ARCH_NONE && STREQ(lxcarch, "x86"))
             arch = VIR_ARCH_I686;
-        else if (arch == VIR_ARCH_NONE && STREQ(value, "amd64"))
+        else if (arch == VIR_ARCH_NONE && STREQ(lxcarch, "amd64"))
             arch = VIR_ARCH_X86_64;
         vmdef->os.arch = arch;
     }
@@ -1041,8 +1057,8 @@ lxcParseConfigString(const char *config,
     if (VIR_STRDUP(vmdef->os.init, "/sbin/init") < 0)
         goto error;
 
-    if (virConfGetValueString(properties, "lxc.utsname", &value) <= 0 ||
-        VIR_STRDUP(vmdef->name, value) < 0)
+    if (virConfGetValueString(properties, "lxc.utsname", &lxcutsname) <= 0 ||
+        VIR_STRDUP(vmdef->name, lxcutsname) < 0)
         goto error;
     if (!vmdef->name && (VIR_STRDUP(vmdef->name, "unnamed") < 0))
         goto error;
-- 
2.17.1




More information about the libvir-list mailing list