[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]