On 10/15/2010 12:54 PM, Matthias Bolte wrote:
2010/10/15 Stefan Berger<stefanb@xxxxxxxxxxxxxxxxxx>:
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@xxxxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list