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

2010/10/15 Stefan Berger&lt;stefanb(a)linux.vnet.ibm.com&gt;: > 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&lt;stefanb(a)us.ibm.com&gt; > 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: