[libvirt] [PATCH] bye to close(), welcome to VIR_(FORCE_)CLOSE()

[Eric Blake]

On 10/22/2010 05:19 AM, Stefan Berger wrote:
> Using automated replacement with sed and editing I have now replaced all
> occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of
> course. Some replacements were straight forward, others I needed to pay
> attention. I hope I payed attention in all the right places... Please
> have a look. This should have at least solved one more double-close
> error.
>
> Signed-off-by: Stefan Berger<stefanb at us.ibm.com>
>

>
> Index: libvirt-acl/src/libvirt.c
> ===================================================================
> --- libvirt-acl.orig/src/libvirt.c
> +++ libvirt-acl/src/libvirt.c
> @@ -10794,7 +10794,7 @@ virStreamRef(virStreamPtr stream)
>    *      ... report an error ....
>    * done:
>    *   virStreamFree(st);
> - *   close(fd);
> + *   VIR_FORCE_CLOSE(fd);

Ignoring close() failure on read seems reasonable...

>    *
>    * Returns the number of bytes written, which may be less
>    * than requested.
> @@ -10884,7 +10884,7 @@ error:
>    *      ... report an error ....
>    * done:
>    *   virStreamFree(st);
> - *   close(fd);
> + *   VIR_FORCE_CLOSE(fd);

but on write, we should instead change the recommendation to check for 
close() failure, since some file systems (yes, I'm looking at you, NFS) 
can successfully write() to kernel buffers but still fail to close() 
when actual network traffic is finally triggered.

  *   int fd = open("demo.iso", O_WRONLY, 0600)
  *
...
  *   if (virStreamFinish(st) < 0 || VIR_CLOSE(fd) < 0)
  *      ... report an error ....
  * done:
  *   virStreamFree(st);
  *   VIR_FORCE_CLOSE(fd);

I'm wondering how much of the rest of your patch might be impacted by 
adopting this idiom.

Also, should we be thinking of a separate patch for VIR_FCLOSE for the 
FILE* variant?

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org