[libvirt] [PATCH] introduce VIR_CLOSE to be used rather than close()
Stefan Berger
stefanb at linux.vnet.ibm.com
Fri Oct 15 17:01:21 UTC 2010
On 10/15/2010 12:54 PM, Matthias Bolte wrote:
> 2010/10/15 Stefan Berger<stefanb at 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 at 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
More information about the libvir-list
mailing list