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

Re: [libvirt] [PATCH v2] Make sure that filedescriptors are only closed once



 On 10/15/2010 07:16 AM, Daniel P. Berrange wrote:
On Thu, Oct 14, 2010 at 09:31:59PM -0400, Stefan Berger wrote:
  V2:
   also the path where close() failed needs to be corrected;

->  time for a VIR_CLOSE() ?

Setting fd=-1 in these places makes sure that the fd is not closed twice
later on in 'clean:'. This is of course a recipe for disaster in a
multi-threaded system where the filedescriptor can belong to something
else right away.

At least we have a pattern...

(looks like I can close the augeas bug as well...)

Signed-off-by: Stefan Berger<stefanb us ibm com>

---
  src/conf/domain_conf.c   |    5 ++++-
  src/conf/network_conf.c  |    5 ++++-
  src/conf/nwfilter_conf.c |   10 ++++++++--
  src/conf/storage_conf.c  |    5 ++++-
  4 files changed, 20 insertions(+), 5 deletions(-)

Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -6802,13 +6802,16 @@ int virDomainSaveXML(const char *configD
          virReportSystemError(errno,
                               _("cannot save config file '%s'"),
                               configFile);
-        goto cleanup;
+        goto cleanup_free;
      }

+    fd = -1;
+
      ret = 0;
   cleanup:
      if (fd != -1)
          close(fd);
+ cleanup_free:
      VIR_FREE(configFile);
      return ret;
  }
Index: libvirt-acl/src/conf/network_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/network_conf.c
+++ libvirt-acl/src/conf/network_conf.c
@@ -691,15 +691,18 @@ int virNetworkSaveXML(const char *config
          virReportSystemError(errno,
                               _("cannot save config file '%s'"),
                               configFile);
-        goto cleanup;
+        goto cleanup_free;
      }

+    fd = -1;
+
      ret = 0;

   cleanup:
      if (fd != -1)
          close(fd);

+ cleanup_free:
      VIR_FREE(configFile);

      return ret;
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2197,15 +2197,18 @@ int virNWFilterSaveXML(const char *confi
          virReportSystemError(errno,
                               _("cannot save config file '%s'"),
                               configFile);
-        goto cleanup;
+        goto cleanup_free;
      }

+    fd = -1;
+
      ret = 0;

   cleanup:
      if (fd != -1)
          close(fd);

+ cleanup_free:
      VIR_FREE(configFile);

      return ret;
@@ -2608,15 +2611,18 @@ virNWFilterPoolObjSaveDef(virNWFilterDri
          virReportSystemError(errno,
                               _("cannot save config file %s"),
                               pool->configFile);
-        goto cleanup;
+        goto cleanup_free;
      }

+    fd = -1;
+
      ret = 0;

   cleanup:
      if (fd != -1)
          close(fd);

+ cleanup_free:
      VIR_FREE(xml);

      return ret;
Index: libvirt-acl/src/conf/storage_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/storage_conf.c
+++ libvirt-acl/src/conf/storage_conf.c
@@ -1564,15 +1564,18 @@ virStoragePoolObjSaveDef(virStorageDrive
          virReportSystemError(errno,
                               _("cannot save config file %s"),
                               pool->configFile);
-        goto cleanup;
+        goto cleanup_free;
      }

+    fd = -1;
+
      ret = 0;

   cleanup:
      if (fd != -1)
          close(fd);

+ cleanup_free:
      VIR_FREE(xml);

      return ret;
ACK

Well, what about a VIR_CLOSE() to prevent such mistakes in the future.

   Stefan
Daniel


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