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

Stefan Berger stefanb at linux.vnet.ibm.com
Fri Oct 15 19:02:45 UTC 2010


  On 10/15/2010 01:45 PM, Eric Blake wrote:
> On 10/15/2010 11:32 AM, Eric Blake wrote:
>>> +int virClose(int *fdptr);
>>
>> Needs an ATTRIBUTE_NONNULL(1). I'm also debating whether it needs
>> ATTRIBUTE_RETURN_CHECK; normally, close() fails in very few situations,
>> and in cleanup paths, you tend to already have another error more
>> important so you can ignore the secondary close() failures. So I'm
>> probably 80-20 against adding ATTRIBUTE_RETURN_CHECK.
>>
>>> +
>>> +# define VIR_CLOSE(FD) \
>>> + virClose(&(FD))
>
> Thinking about it a bit more, what if we provide two variants?
>
> int virClose(int *fdptr, bool preserve_errno)
> {
>     int saved_errno;
>     int rc;
>
>     if (*fdptr >= 0) {
>         if (preserve_errno)
>             saved_errno = errno;
>         rc = close(*fdptr);
>         *fdptr = -1;
>         if (preserve_errno)
>             errno = saved_errno;
>     } else
>         rc = 0;
>
>     return rc;
> }
>
> int virClose(int *fdptr) ATTRIBUTE_RETURN_CHECK;
>
'approximately', yes

> /* For use on normal paths; caller must check return value, and 
> failure sets errno per close(). */
> #define VIR_CLOSE(FD) virClose(&(FD),false);
>
> /* For use on cleanup paths; errno is unaffected by close, and no 
> return value to worry about. */
> #define VIR_FORCE_CLOSE(FD) ignore_value(virClose(&(FD),true))
>
Fine by me. I am picking this up for the next patch 'as-is'. You can 
edit the file header (copyright and so on).

    Stefan




More information about the libvir-list mailing list