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

15 Oct 2010

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

    