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

Re: [libvirt] [PATCH] introduce VIR_CLOSE to be used rather than close()



 On 10/15/2010 12:54 PM, Matthias Bolte wrote:
2010/10/15 Stefan Berger<stefanb linux vnet ibm com>:
  Since bugs due to double-closed filedescriptors are difficult to track down
in a multi-threaded system, I am introducing the VIR_CLOSE(fd) macro to help
avoid mistakes here.

There are lots of places where close() is being used. In this patch I am
only cleaning up usage of close() in src/conf where the problems were.

I also dare to declare close() as being deprecated in libvirt code base
(HACKING). :-)

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

Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -46,6 +46,7 @@
  #include "nwfilter_conf.h"
  #include "ignore-value.h"
  #include "storage_file.h"
+#include "files.h"

  #define VIR_FROM_THIS VIR_FROM_DOMAIN

@@ -6798,17 +6799,18 @@ int virDomainSaveXML(const char *configD
         goto cleanup;
     }

-    if (close(fd)<  0) {
+    if (VIR_CLOSE(fd)<  0) {
         virReportSystemError(errno,
                              _("cannot save config file '%s'"),
                              configFile);
-        goto cleanup;
+        goto cleanup_free;
     }

     ret = 0;
  cleanup:
-    if (fd != -1)
-        close(fd);
+    VIR_CLOSE(fd);
+
+ cleanup_free:
     VIR_FREE(configFile);
     return ret;
  }
Why did you add a new label here? You build VIR_CLOSE in a way that
allows safe double invocation. Therefore, this is just fine, isn't it:

I had it without this new label and then changed it since I think it's still good practice to not try to invoke the function twice, even though nothing can go wrong.

    Stefan


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