[libvirt] [PATCH] Memory Leak Fix: Check def->info->alias before assigning

Nehal J Wani nehaljw.kkd1 at gmail.com
Tue Nov 26 22:31:45 UTC 2013


On running the command make -C tests valgrind, there used to be a bunch of
memory leaks shown by valgrind. Specifically, one can check it by running:
libtool --mode=execute valgrind --quiet --leak-check=full --suppressions=./.valgrind.supp qemuhotplugtest
The issue was that def->info->alias was already malloc'ed by xmlStrndup in
virDomainDeviceInfoParseXML (domain_conf.c:3439). The new alias was being
assigned again without freeing the old one in qemuAssignDeviceAliases().
This patch checks if the entity exists, and frees accordingly, hence making
valgrind cry lesser.

---
 src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 763417f..bbec1d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -979,6 +979,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
     size_t i;
 
     for (i = 0; i < def->ndisks; i++) {
+        if (def->disks[i]->info.alias)
+            VIR_FREE(def->disks[i]->info.alias);
         if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0)
             return -1;
     }
@@ -988,6 +990,9 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
             /* type='hostdev' interfaces are also on the hostdevs list,
              * and will have their alias assigned with other hostdevs.
              */
+            if (def->nets[i]->info.alias)
+                VIR_FREE(def->nets[i]->info.alias);
+
             if (virDomainNetGetActualType(def->nets[i])
                 != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
                 qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0) {
@@ -1000,70 +1005,104 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
         return 0;
 
     for (i = 0; i < def->nfss; i++) {
+        if (def->fss[i]->info.alias)
+            VIR_FREE(def->fss[i]->info.alias);
         if (virAsprintf(&def->fss[i]->info.alias, "fs%zu", i) < 0)
             return -1;
     }
     for (i = 0; i < def->nsounds; i++) {
+        if (def->sounds[i]->info.alias)
+            VIR_FREE(def->sounds[i]->info.alias);
         if (virAsprintf(&def->sounds[i]->info.alias, "sound%zu", i) < 0)
             return -1;
     }
     for (i = 0; i < def->nhostdevs; i++) {
+        if (def->hostdevs[i]->info->alias)
+            VIR_FREE(def->hostdevs[i]->info->alias);
         if (qemuAssignDeviceHostdevAlias(def, def->hostdevs[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nredirdevs; i++) {
+        if (def->redirdevs[i]->info.alias)
+            VIR_FREE(def->redirdevs[i]->info.alias);
         if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nvideos; i++) {
+        if (def->videos[i]->info.alias)
+            VIR_FREE(def->videos[i]->info.alias);
         if (virAsprintf(&def->videos[i]->info.alias, "video%zu", i) < 0)
             return -1;
     }
     for (i = 0; i < def->ncontrollers; i++) {
+        if (def->controllers[i]->info.alias)
+            VIR_FREE(def->controllers[i]->info.alias);
         if (qemuAssignDeviceControllerAlias(def->controllers[i]) < 0)
             return -1;
     }
     for (i = 0; i < def->ninputs; i++) {
+        if (def->inputs[i]->info.alias)
+            VIR_FREE(def->inputs[i]->info.alias);
         if (virAsprintf(&def->inputs[i]->info.alias, "input%zu", i) < 0)
             return -1;
     }
     for (i = 0; i < def->nparallels; i++) {
+        if (def->parallels[i]->info.alias)
+            VIR_FREE(def->parallels[i]->info.alias);
         if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nserials; i++) {
+        if (def->serials[i]->info.alias)
+            VIR_FREE(def->serials[i]->info.alias);
         if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nchannels; i++) {
+        if (def->channels[i]->info.alias)
+            VIR_FREE(def->channels[i]->info.alias);
         if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nconsoles; i++) {
+        if (def->consoles[i]->info.alias)
+            VIR_FREE(def->consoles[i]->info.alias);
         if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0)
             return -1;
     }
     for (i = 0; i < def->nhubs; i++) {
+        if (def->hubs[i]->info.alias)
+            VIR_FREE(def->hubs[i]->info.alias);
         if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0)
             return -1;
     }
     for (i = 0; i < def->nsmartcards; i++) {
+        if (def->smartcards[i]->info.alias)
+            VIR_FREE(def->smartcards[i]->info.alias);
         if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0)
             return -1;
     }
     if (def->watchdog) {
+        if (def->watchdog->info.alias)
+            VIR_FREE(def->watchdog->info.alias);
         if (virAsprintf(&def->watchdog->info.alias, "watchdog%d", 0) < 0)
             return -1;
     }
     if (def->memballoon) {
+        if (def->memballoon->info.alias)
+            VIR_FREE(def->memballoon->info.alias);
         if (virAsprintf(&def->memballoon->info.alias, "balloon%d", 0) < 0)
             return -1;
     }
     if (def->rng) {
+        if (def->rng->info.alias)
+            VIR_FREE(def->rng->info.alias);
         if (virAsprintf(&def->rng->info.alias, "rng%d", 0) < 0)
             return -1;
     }
     if (def->tpm) {
+        if (def->tpm->info.alias)
+            VIR_FREE(def->tpm->info.alias);
         if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0)
             return -1;
     }
-- 
1.8.1.4




More information about the libvir-list mailing list