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