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

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(a)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

Daniel