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

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



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


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