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

[Stefan Berger]

  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