Use safewrite in place of write, in many cases.

Also add "make syntax-check" rules to ensure no new uses sneak in.

There are many uses of write like this:

    if (write (fd, xml, towrite) != towrite)
        return -1;

The problem is that the syscall can succeed, yet write less than
the requested number of bytes, so the caller should retry
rather than simply failing.

This patch changes most of them to use util.c's safewrite wrapper,
which encapsulates the process.  Also, there were a few cases in
which the retry loop was open-coded, and I replaced those, too.

* Makefile.maint (sc_avoid_write): New rule, to avoid recurrence.
* .x-sc_avoid_write: New file.  Record two legitimate exemptions.
* qemud/qemud.c (sig_handler, qemudClientWriteBuf): Use safewrite, not write.
* src/conf.c (__virConfWriteFile): Likewise.
* src/qemu_conf.c (qemudSaveConfig, qemudSaveNetworkConfig): Likewise.
* src/qemu_driver.c (qemudWaitForMonitor, qemudStartVMDaemon)
(qemudVMData, PROC_IP_FORWARD): Likewise.
* proxy/libvirt_proxy.c: Include "util.h".
(proxyWriteClientSocket): Use safewrite.
* src/test.c (testDomainSave, testDomainCoreDump): Likewise.
* src/proxy_internal.c (virProxyWriteClientSocket): Likewise.
* src/virsh.c: Include "util-lib.h".
(vshOutputLogFile): Use safewrite.
* src/console.c: Include "util-lib.h".
(vshRunConsole): Use safewrite.

diff --git a/.x-sc_avoid_write b/.x-sc_avoid_write
new file mode 100644 (file)
index 0000000..72a196d
--- /dev/null
+++ b/.x-sc_avoid_write
@@ -0,0 +1,2 @@
+^src/util\.c$
+^src/xend_internal\.c$

diff --git a/ChangeLog b/ChangeLog
index 8700dd302d167c6faa625410c0fabc579b39a1d2..daeabf8ba8fe043ee0ae1109f9544bf580646af7 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 Fri Feb 22 13:32:11 CET 2008 Jim Meyering <meyering@redhat.com>
 
+       Use safewrite in place of write, in many cases.
+       Also add "make syntax-check" rules to ensure no new uses sneak in.
+       * Makefile.maint (sc_avoid_write): New rule, to avoid recurrence.
+       * .x-sc_avoid_write: New file.  Record two legitimate exemptions.
+       * qemud/qemud.c (sig_handler, qemudClientWriteBuf): Use safewrite,
+       not write.
+       * src/conf.c (__virConfWriteFile): Likewise.
+       * src/qemu_conf.c (qemudSaveConfig, qemudSaveNetworkConfig): Likewise.
+       * src/qemu_driver.c (qemudWaitForMonitor, qemudStartVMDaemon)
+       (qemudVMData, PROC_IP_FORWARD): Likewise.
+       * proxy/libvirt_proxy.c: Include "util.h".
+       (proxyWriteClientSocket): Use safewrite.
+       * src/test.c (testDomainSave, testDomainCoreDump): Likewise.
+       * src/proxy_internal.c (virProxyWriteClientSocket): Likewise.
+       * src/virsh.c: Include "util-lib.h".
+       (vshOutputLogFile): Use safewrite.
+       * src/console.c: Include "util-lib.h".
+       (vshRunConsole): Use safewrite.
+
        Move safewrite and saferead to a separate file.
        * src/util.c (saferead, safewrite): Move function definitions to
        util-lib.c and include that .c file.

diff --git a/Makefile.maint b/Makefile.maint
index 39320df088896a56c18ea8b94f97aaaa4d5b80d7..fe964a57130b7efb6ce2abb0731944442ea0f9d1 100644 (file)
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -46,6 +46,17 @@ sc_avoid_if_before_free:
          { echo '$(ME): found useless "if" before "free" above' 1>&2;  \
            exit 1; } || :
 
+# Avoid uses of write(2).  Either switch to streams (fwrite), or use
+# the safewrite wrapper.
+sc_avoid_write:
+       @if $(CVS_LIST_EXCEPT) | grep '\.c$$' > /dev/null; then         \
+         grep '\<write *(' $$($(CVS_LIST_EXCEPT) | grep '\.c$$') &&    \
+           { echo "$(ME): the above files use write;"                  \
+             " consider using the safewrite wrapper instead"           \
+                 1>&2; exit 1; } || :;                                 \
+       else :;                                                         \
+       fi
+
 sc_cast_of_argument_to_free:
        @grep -nE '\<free \(\(' $$($(CVS_LIST_EXCEPT)) &&               \
          { echo '$(ME): don'\''t cast free argument' 1>&2;             \

diff --git a/qemud/qemud.c b/qemud/qemud.c
index a40dfcbbb0041a2009ee7495f348f7e05ef0b6d0..d53a813e8c6a2c728662f263b6ca544136431550 100644 (file)
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -118,7 +118,7 @@ static void sig_handler(int sig) {
         return;
 
     origerrno = errno;
-    r = write(sigwrite, &sigc, 1);
+    r = safewrite(sigwrite, &sigc, 1);
     if (r == -1) {
         sig_errors++;
         sig_lasterrno = errno;
@@ -1360,11 +1360,9 @@ static int qemudClientWriteBuf(struct qemud_server *server,
                                const char *data, int len) {
     int ret;
     if (!client->tlssession) {
-        if ((ret = write(client->fd, data, len)) == -1) {
-            if (errno != EAGAIN) {
-                qemudLog (QEMUD_ERR, _("write: %s"), strerror (errno));
-                qemudDispatchClientFailure(server, client);
-            }
+        if ((ret = safewrite(client->fd, data, len)) == -1) {
+            qemudLog (QEMUD_ERR, _("write: %s"), strerror (errno));
+            qemudDispatchClientFailure(server, client);
             return -1;
         }
     } else {

diff --git a/src/conf.c b/src/conf.c
index e0ecdea9981f3ac1ee6bfeb49aa18eb2a362838b..53ea99351115e6e3ebf26d4e70b290028460d500 100644 (file)
--- a/src/conf.c
+++ b/src/conf.c
@@ -904,7 +904,7 @@ __virConfWriteFile(const char *filename, virConfPtr conf)
        goto error;
     }
 
-    ret = write(fd, buf->content, buf->use);
+    ret = safewrite(fd, buf->content, buf->use);
     close(fd);
     if (ret != (int) buf->use) {
         virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"), 0);

diff --git a/src/console.c b/src/console.c
index 02a9c7f33013a75c9e583f95c58b0856e9476fbb..079d4efea603d468d89739119a1751629a8256d6 100644 (file)
--- a/src/console.c
+++ b/src/console.c
@@ -1,7 +1,7 @@
 /*
  * console.c: A dumb serial console client
  *
- * Copyright (C) 2007 Red Hat, Inc.
+ * Copyright (C) 2007, 2008 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -37,6 +37,7 @@
 
 #include "console.h"
 #include "internal.h"
+#include "util-lib.h"
 
 /* ie  Ctrl-]  as per telnet */
 #define CTRL_CLOSE_BRACKET '\35'
@@ -161,7 +162,8 @@ int vshRunConsole(const char *tty) {
 
                 while (sent < got) {
                     int done;
-                    if ((done = write(destfd, buf + sent, got - sent)) <= 0) {
+                    if ((done = safewrite(destfd, buf + sent, got - sent))
+                        <= 0) {
                         fprintf(stderr, _("failure writing output: %s\n"),
                                 strerror(errno));
                         goto cleanup;

diff --git a/src/proxy_internal.c b/src/proxy_internal.c
index bc94763235612df00316741a0095ddb3ba0f4716..c3e50c6936437f53f3f0c73707d72c38b3374ada 100644 (file)
--- a/src/proxy_internal.c
+++ b/src/proxy_internal.c
@@ -1,7 +1,7 @@
 /*
  * proxy_client.c: client side of the communication with the libvirt proxy.
  *
- * Copyright (C) 2006 Red Hat, Inc.
+ * Copyright (C) 2006, 2008 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -26,6 +26,7 @@
 #include "internal.h"
 #include "driver.h"
 #include "proxy_internal.h"
+#include "util.h"
 #include "xen_unified.h"
 
 #define STANDALONE
@@ -345,17 +346,10 @@ virProxyWriteClientSocket(int fd, const char *data, int len) {
     if ((fd < 0) || (data == NULL) || (len < 0))
         return(-1);
 
-retry:
-    ret = write(fd, data, len);
+    ret = safewrite(fd, data, len);
     if (ret < 0) {
-        if (errno == EINTR) {
-           if (debug > 0)
-               fprintf(stderr, "write socket %d, %d bytes interrupted\n",
-                       fd, len);
-           goto retry;
-       }
         fprintf(stderr, _("Failed to write to socket %d\n"), fd);
-       return(-1);
+        return(-1);
     }
     if (debug)
        fprintf(stderr, "wrote %d bytes to socket %d\n",

diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ac1cc1eb32bf701c6d3260f8ce2cc5057dffe060..fc82a6d47c31ba73df6e61a6767610d57e390139 100644 (file)
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1866,7 +1866,7 @@ static int qemudSaveConfig(virConnectPtr conn,
     }
 
     towrite = strlen(xml);
-    if (write(fd, xml, towrite) != towrite) {
+    if (safewrite(fd, xml, towrite) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                          "cannot write config file %s: %s",
                          vm->configFile, strerror(errno));
@@ -2089,7 +2089,7 @@ static int qemudSaveNetworkConfig(virConnectPtr conn,
     }
 
     towrite = strlen(xml);
-    if (write(fd, xml, towrite) != towrite) {
+    if (safewrite(fd, xml, towrite) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                          "cannot write config file %s: %s",
                          network->configFile, strerror(errno));

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 15cd52c86db654560b5ab04a20bc03afe360a8fa..85d042f9ba5e3e6c7c9a26b3c725774f30f592af 100644 (file)
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -539,11 +539,9 @@ static int qemudWaitForMonitor(virConnectPtr conn,
                                      "console");
 
     buf[sizeof(buf)-1] = '\0';
- retry:
-    if (write(vm->logfile, buf, strlen(buf)) < 0) {
+
+    if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
         /* Log, but ignore failures to write logfile for VM */
-        if (errno == EINTR)
-            goto retry;
         qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s"),
                  strerror(errno));
     }
@@ -656,15 +654,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
 
     tmp = argv;
     while (*tmp) {
-        if (write(vm->logfile, *tmp, strlen(*tmp)) < 0)
+        if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
             qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"),
                      errno, strerror(errno));
-        if (write(vm->logfile, " ", 1) < 0)
+        if (safewrite(vm->logfile, " ", 1) < 0)
             qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"),
                      errno, strerror(errno));
         tmp++;
     }
-    if (write(vm->logfile, "\n", 1) < 0)
+    if (safewrite(vm->logfile, "\n", 1) < 0)
         qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"),
                  errno, strerror(errno));
 
@@ -733,11 +731,8 @@ static int qemudVMData(struct qemud_driver *driver ATTRIBUTE_UNUSED,
         }
         buf[ret] = '\0';
 
-    retry:
-        if (write(vm->logfile, buf, ret) < 0) {
+        if (safewrite(vm->logfile, buf, ret) < 0) {
             /* Log, but ignore failures to write logfile for VM */
-            if (errno == EINTR)
-                goto retry;
             qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s"),
                      strerror(errno));
         }
@@ -1113,7 +1108,7 @@ qemudEnableIpForwarding(void)
     if ((fd = open(PROC_IP_FORWARD, O_WRONLY|O_TRUNC)) == -1)
         return 0;
 
-    if (write(fd, "1\n", 2) < 0)
+    if (safewrite(fd, "1\n", 2) < 0)
         ret = 0;
 
     close (fd);

diff --git a/src/test.c b/src/test.c
index f860e495f841f5eeacf288c818a5caccb9af6ea3..1d064d3c416829e491d34ced2d5ddab4dc989fb3 100644 (file)
--- a/src/test.c
+++ b/src/test.c
@@ -42,6 +42,7 @@
 #include "test.h"
 #include "xml.h"
 #include "buf.h"
+#include "util.h"
 #include "uuid.h"
 
 /* Flags that determine the action to take on a shutdown or crash of a domain
@@ -1293,19 +1294,19 @@ static int testDomainSave(virDomainPtr domain,
         return (-1);
     }
     len = strlen(xml);
-    if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC)) {
+    if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write header");
         close(fd);
         return (-1);
     }
-    if (write(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
+    if (safewrite(fd, (char*)&len, sizeof(len)) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write metadata length");
         close(fd);
         return (-1);
     }
-    if (write(fd, xml, len) != len) {
+    if (safewrite(fd, xml, len) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write metadata");
         free(xml);
@@ -1398,7 +1399,7 @@ static int testDomainCoreDump(virDomainPtr domain,
                   "cannot save domain core");
         return (-1);
     }
-    if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC)) {
+    if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write header");
         close(fd);

diff --git a/src/virsh.c b/src/virsh.c
index 5a5f04e4fc0a7f93c37472075bf46be73f3d544b..356c4963612fdf8a13522f94ab4942fbc291958f 100644 (file)
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -46,9 +46,10 @@
 #include <readline/history.h>
 #endif
 
+#include "buf.h"
 #include "console.h"
 #include "util.h"
-#include "buf.h"
+#include "util-lib.h"
 
 static char *progname;
 
@@ -6170,7 +6171,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, va_list
         snprintf(msg_buf + strlen(msg_buf), sizeof(msg_buf) - strlen(msg_buf), "\n");
 
     /* write log */
-    if (write(ctl->log_fd, msg_buf, strlen(msg_buf)) == -1) {
+    if (safewrite(ctl->log_fd, msg_buf, strlen(msg_buf)) < 0) {
         vshCloseLogFile(ctl);
         vshError(ctl, FALSE, "%s", _("failed to write the log file"));
     }