[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/6] uml: Plug memory leak on umlStartVMDaemon() error path



On 12/01/2011 07:39 AM, Eric Blake wrote:
On 11/29/2011 10:57 PM, ajia redhat com wrote:
From: Alex Jia<ajia redhat com>

Detected by Coverity.  Leak introduced in commit 8866eed.

Signed-off-by: Alex Jia<ajia redhat com>
---
  src/uml/uml_driver.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 073f362..fe6d5ba 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1051,6 +1051,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
          VIR_FREE(vm->def->consoles[i]->info.alias);
          if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i)<  0) {
              virReportOOMError();
+            VIR_FORCE_CLOSE(logfd);
              goto cleanup;
Good catch, but while reading the function, I found another bug - if we
failed to add the domain to autodestroy, we still returned success but
without setting the domain to transient.  I'm pushing this more
comprehensive fix:
Indeed, the return value hasn't been stored in 'ret' if we failed to add domain
to autodestroy. Eric, thanks.
diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
index fe6d5ba..faea313 100644
--- i/src/uml/uml_driver.c
+++ w/src/uml/uml_driver.c
@@ -1033,56 +1033,51 @@ static int umlStartVMDaemon(virConnectPtr conn,
      /* Do this upfront, so any part of the startup process can add
       * runtime state to vm->def that won't be persisted. This let's us
       * report implicit runtime defaults in the XML, like vnc listen/socket
       */
      VIR_DEBUG("Setting current domain def as transient");
      if (virDomainObjSetDefTransient(driver->caps, vm, true)<  0) {
          VIR_FORCE_CLOSE(logfd);
          return -1;
      }

-    if (!(cmd = umlBuildCommandLine(conn, driver, vm))) {
-        VIR_FORCE_CLOSE(logfd);
-        virDomainConfVMNWFilterTeardown(vm);
-        umlCleanupTapDevices(vm);
-        return -1;
-    }
+    if (!(cmd = umlBuildCommandLine(conn, driver, vm)))
+        goto cleanup;

      for (i = 0 ; i<  vm->def->nconsoles ; i++) {
          VIR_FREE(vm->def->consoles[i]->info.alias);
          if (virAsprintf(&vm->def->consoles[i]->info.alias,
"console%zu", i)<  0) {
              virReportOOMError();
-            VIR_FORCE_CLOSE(logfd);
              goto cleanup;
          }
      }

      virCommandWriteArgLog(cmd, logfd);

      priv->monitor = -1;

      virCommandClearCaps(cmd);
      virCommandSetOutputFD(cmd,&logfd);
      virCommandSetErrorFD(cmd,&logfd);
      virCommandDaemonize(cmd);

      ret = virCommandRun(cmd, NULL);
-    VIR_FORCE_CLOSE(logfd);
      if (ret<  0)
          goto cleanup;

      if (autoDestroy&&
-        umlProcessAutoDestroyAdd(driver, vm, conn)<  0)
+        (ret = umlProcessAutoDestroyAdd(driver, vm, conn))<  0)
          goto cleanup;

      ret = virDomainObjSetDefTransient(driver->caps, vm, false);
  cleanup:
+    VIR_FORCE_CLOSE(logfd);
      virCommandFree(cmd);

      if (ret<  0) {
          virDomainConfVMNWFilterTeardown(vm);
          umlCleanupTapDevices(vm);
          if (vm->newDef) {
              virDomainDefFree(vm->def);
              vm->def = vm->newDef;
              vm->def->id = -1;
              vm->newDef = NULL;




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]