[libvirt] [PATCH v3] bye to close(), welcome to VIR_(FORCE_)CLOSE()
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Nov 9 00:45:05 UTC 2010
On 11/08/2010 05:17 PM, Eric Blake wrote:
>
>> 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);
>> *
>> * Returns the number of bytes written, which may be less
>> * than requested.
>> @@ -10884,8 +10884,8 @@ error:
>> * ... report an error ....
>> * done:
>> * virStreamFree(st);
>> - * close(fd);
>> - *
>> + * if (VIR_CLOSE(fd)< 0)
>> + * virReportSystemError(errno, "%s", _("failed to close file"));
>> *
>> * Returns the number of bytes read, which may be less
>> * than requested.
>> @@ -10964,7 +10964,7 @@ error:
>> * if (virStreamFinish(st)< 0)
>> * ...report an error...
>> * virStreamFree(st);
>> - * close(fd);
>> + * VIR_FORCE_CLOSE(fd);
>> *
>> * Returns 0 if all the data was successfully sent. The caller
>> * should invoke virStreamFinish(st) to flush the stream upon
> These first three are okay.
>
>> @@ -11061,7 +11061,7 @@ cleanup:
>> * if (virStreamFinish(st)< 0)
>> * ...report an error...
>> * virStreamFree(st);
>> - * close(fd);
>> + * VIR_FORCE_CLOSE(fd);
> But the comment for virStreamRecvAll should match the comment for
> virStreamRecv. (Comments only, so trivial to fix).
>
The comment for virStreamRecvAll is correct in terms of the return
values. They are different from those of virStreamRecv, if that's what
you meant.
>> Index: libvirt-acl/src/phyp/phyp_driver.c
>> ===================================================================
>> @@ -764,7 +769,11 @@ phypUUIDTable_Pull(virConnectPtr conn)
>> }
>> break;
>> }
>> - close(fd);
>> + if (VIR_CLOSE(fd)< 0) {
>> + virReportSystemError(errno, _("Could not close %s\n"),
>> + local_file);
>> + goto err;
>> + }
>> goto exit;
>>
>> exit:
> Looks funny to goto the label on the next line, but no harm in leaving
> it (at any rate, unrelated to your actual patch).
>
>> Index: libvirt-acl/src/xen/proxy_internal.c
>> ===================================================================
>> --- libvirt-acl.orig/src/xen/proxy_internal.c
>> +++ libvirt-acl/src/xen/proxy_internal.c
>> Index: libvirt-acl/proxy/libvirt_proxy.c
>> ===================================================================
>> --- libvirt-acl.orig/proxy/libvirt_proxy.c
>> +++ libvirt-acl/proxy/libvirt_proxy.c
> Merge conflict - you can ignore any changes to these two files, now that
> they is deleted.
> ACK. You may find some more instances of close() pop up as you rebase
> on top of the latest tree, but it shouldn't be too hard to figure out.
>
This is what I may have missed before or was added recently:
Index: libvirt-acl/tools/virsh.c
===================================================================
--- libvirt-acl.orig/tools/virsh.c
+++ libvirt-acl/tools/virsh.c
@@ -11269,11 +11269,9 @@ static void
vshCloseLogFile(vshControl *ctl)
{
/* log file close */
- if (ctl->log_fd >= 0) {
- if (close(ctl->log_fd) < 0)
- vshError(ctl, _("%s: failed to write log file: %s"),
- ctl->logfile ? ctl->logfile : "?", strerror (errno));
- ctl->log_fd = -1;
+ if (VIR_CLOSE(ctl->log_fd) < 0)
+ vshError(ctl, _("%s: failed to write log file: %s"),
+ ctl->logfile ? ctl->logfile : "?", strerror (errno));
}
if (ctl->logfile) {
I haven't pushed it yet, but would do so tomorrow, unless there are
concerns.
Stefan
More information about the libvir-list
mailing list