From b59d9c85f1bea94a748fd296e559d806e09193b5 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 22 Feb 2008 15:55:04 +0000 Subject: [PATCH] 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. --- .x-sc_avoid_write | 2 ++ ChangeLog | 19 +++++++++++++++++++ Makefile.maint | 11 +++++++++++ qemud/qemud.c | 10 ++++------ src/conf.c | 2 +- src/console.c | 6 ++++-- src/proxy_internal.c | 14 ++++---------- src/qemu_conf.c | 4 ++-- src/qemu_driver.c | 19 +++++++------------ src/test.c | 9 +++++---- src/virsh.c | 5 +++-- 11 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 .x-sc_avoid_write diff --git a/.x-sc_avoid_write b/.x-sc_avoid_write new file mode 100644 index 0000000000..72a196d138 --- /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 8700dd302d..daeabf8ba8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,24 @@ Fri Feb 22 13:32:11 CET 2008 Jim Meyering + 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 39320df088..fe964a5713 100644 --- 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 '\&2; exit 1; } || :; \ + else :; \ + fi + sc_cast_of_argument_to_free: @grep -nE '\&2; \ diff --git a/qemud/qemud.c b/qemud/qemud.c index a40dfcbbb0..d53a813e8c 100644 --- 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 e0ecdea998..53ea993511 100644 --- 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 02a9c7f330..079d4efea6 100644 --- 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 bc94763235..c3e50c6936 100644 --- 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 ac1cc1eb32..fc82a6d47c 100644 --- 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 15cd52c86d..85d042f9ba 100644 --- 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 f860e495f8..1d064d3c41 100644 --- 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 5a5f04e4fc..356c496361 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -46,9 +46,10 @@ #include #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")); } -- GitLab