Re: [libvirt] [PATCH 1/6] conf: make hostdev info a separate object

[Laine Stump <laine laine org>]

On 02/20/2012 01:48 PM, Eric Blake wrote:
> Overall, the idea looks reasonable, but you'll need a v2 to fix the
> memory issues in qemu_command.c.

Actually, there were problems with freeing the hostdevdef on error paths
in all 4 places virDomainHostdevDefAlloc() was used :-( I don't know
what I was thinking...

I've updated the patch with the following diff and will repost the full
patch.

>From ab626a94d03e8129c8b2a041012476bd56330a3d Mon Sep 17 00:00:00 2001
From: Laine Stump <laine laine org>
Date: Mon, 20 Feb 2012 14:43:06 -0500
Subject: [PATCH] fix memory leaks in "make hostdev info a separate object"

---
 src/conf/domain_conf.c  |    1 +
 src/qemu/qemu_command.c |   42 +++++++++++++++++++-----------------------
 src/xenxs/xen_sxpr.c    |    1 +
 src/xenxs/xen_xm.c      |    4 +++-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0503f4..df7d87d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1321,6 +1321,7 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
         return NULL;
     }
     if (VIR_ALLOC(def->info) < 0) {
+        virReportOOMError();
         VIR_FREE(def);
         return NULL;
     }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a5a0054..0e295cc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6534,34 +6534,31 @@ qemuParseCommandLinePCI(const char *val)
     virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
 
     if (!def)
-       goto cleanup;
+       goto error;
 
     if (!STRPREFIX(val, "host=")) {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         _("unknown PCI device syntax '%s'"), val);
-        goto cleanup;
+        goto error;
     }
 
     start = val + strlen("host=");
     if (virStrToLong_i(start, &end, 16, &bus) < 0 || *end != ':') {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         _("cannot extract PCI device bus '%s'"), val);
-        VIR_FREE(def);
-        goto cleanup;
+        goto error;
     }
     start = end + 1;
     if (virStrToLong_i(start, &end, 16, &slot) < 0 || *end != '.') {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         _("cannot extract PCI device slot '%s'"), val);
-        VIR_FREE(def);
-        goto cleanup;
+        goto error;
     }
     start = end + 1;
     if (virStrToLong_i(start, NULL, 16, &func) < 0) {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         _("cannot extract PCI device function '%s'"), val);
-        VIR_FREE(def);
-        goto cleanup;
+        goto error;
     }
 
     def->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
@@ -6570,9 +6567,11 @@ qemuParseCommandLinePCI(const char *val)
     def->source.subsys.u.pci.bus = bus;
     def->source.subsys.u.pci.slot = slot;
     def->source.subsys.u.pci.function = func;
-
-cleanup:
     return def;
+
+ error:
+    virDomainHostdevDefFree(def);
+    return NULL;
 }
 
 
@@ -6588,13 +6587,12 @@ qemuParseCommandLineUSB(const char *val)
     char *end;
 
     if (!def)
-       goto cleanup;
+       goto error;
 
     if (!STRPREFIX(val, "host:")) {
         qemuReportError(VIR_ERR_INTERNAL_ERROR,
                         _("unknown USB device syntax '%s'"), val);
-        VIR_FREE(def);
-        goto cleanup;
+        goto error;
     }
 
     start = val + strlen("host:");
@@ -6602,29 +6600,25 @@ qemuParseCommandLineUSB(const char *val)
         if (virStrToLong_i(start, &end, 16, &first) < 0 || *end != ':') {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("cannot extract USB device vendor '%s'"), val);
-            VIR_FREE(def);
-            goto cleanup;
+            goto error;
         }
         start = end + 1;
         if (virStrToLong_i(start, NULL, 16, &second) < 0) {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("cannot extract USB device product '%s'"), val);
-            VIR_FREE(def);
-            goto cleanup;
+            goto error;
         }
     } else {
         if (virStrToLong_i(start, &end, 10, &first) < 0 || *end != '.') {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                              _("cannot extract USB device bus '%s'"), val);
-            VIR_FREE(def);
-            goto cleanup;
+            goto error;
         }
         start = end + 1;
         if (virStrToLong_i(start, NULL, 10, &second) < 0) {
             qemuReportError(VIR_ERR_INTERNAL_ERROR,
                             _("cannot extract USB device address '%s'"), val);
-            VIR_FREE(def);
-            goto cleanup;
+            goto error;
         }
     }
 
@@ -6638,9 +6632,11 @@ qemuParseCommandLineUSB(const char *val)
         def->source.subsys.u.usb.vendor = first;
         def->source.subsys.u.usb.product = second;
     }
-
-cleanup:
     return def;
+
+ error:
+    virDomainHostdevDefFree(def);
+    return NULL;
 }
 
 
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index b0e1b36..e5df953 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1088,6 +1088,7 @@ xenParseSxprPCI(virDomainDefPtr def,
         dev->source.subsys.u.pci.function = funcID;
 
         if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) {
+            virDomainHostdevDefFree(dev);
             goto no_memory;
         }
 
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 6e72aea..5862168 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -825,8 +825,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
             hostdev->source.subsys.u.pci.slot = slotID;
             hostdev->source.subsys.u.pci.function = funcID;
 
-            if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0)
+            if (VIR_REALLOC_N(def->hostdevs, def->nhostdevs+1) < 0) {
+                virDomainHostdevDefFree(hostdev);
                 goto no_memory;
+            }
             def->hostdevs[def->nhostdevs++] = hostdev;
             hostdev = NULL;
 
-- 
1.7.7.6