7:53 a.m.
Use safewrite in place of write, in many cases.
And 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.
However, there remain invalid uses of write in both virsh.c and
console.c. I would have replaced those, too, but util.c isn't
linked with them, so it would have made this change more involved.
xend_internal.c has one use, but it's ok, and not easily replaced.
Note: the change to libvirt_proxy.c removes a diagnostic that
would have warned about an EINTR-interrupted (and retried) write,
but that seems ok.
* qemud/qemud.c (sig_handler):
* src/conf.c (__virConfWriteFile):
* src/proxy_internal.c (virProxyWriteClientSocket):
* src/qemu_conf.c (qemudSaveConfig, qemudSaveNetworkConfig):
* src/qemu_driver.c (qemudWaitForMonitor, qemudStartVMDaemon)
(qemudVMData, PROC_IP_FORWARD):
* src/test.c (testDomainSave, testDomainCoreDump):
* proxy/libvirt_proxy.c (proxyWriteClientSocket):
* proxy/Makefile.am (libvirt_proxy_SOURCES): Add src/util.c.
* Makefile.maint (sc_avoid_write): New rule.
* .x-sc_avoid_write: New file. Record the two legitimate
exemptions, and two that are temporarily grandfathered in.
Signed-off-by: Jim Meyering <meyering(a)redhat.com>
---
.x-sc_avoid_write | 4 ++++
Makefile.maint | 11 +++++++++++
proxy/Makefile.am | 5 +++--
proxy/libvirt_proxy.c | 15 ++++-----------
qemud/qemud.c | 10 ++++------
src/conf.c | 2 +-
src/proxy_internal.c | 13 +++----------
src/qemu_conf.c | 4 ++--
src/qemu_driver.c | 19 +++++++------------
src/test.c | 8 ++++----
10 files changed, 43 insertions(+), 48 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 0000000..0933e20
--- /dev/null
+++ b/.x-sc_avoid_write
@@ -0,0 +1,4 @@
+^src/util\.c$
+^src/xend_internal\.c$
+^src/virsh\.c$
+^src/console\.c$
diff --git a/Makefile.maint b/Makefile.maint
index 4b54baf..afddddb 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 '\<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/proxy/Makefile.am b/proxy/Makefile.am
index ff9a3eb..6827f36 100644
--- a/proxy/Makefile.am
+++ b/proxy/Makefile.am
@@ -12,11 +12,12 @@ libexec_PROGRAMS = libvirt_proxy
libvirt_proxy_SOURCES = libvirt_proxy.c @top_srcdir(a)/src/xend_internal.c \
@top_srcdir(a)/src/xen_internal.c @top_srcdir(a)/src/virterror.c \
@top_srcdir(a)/src/sexpr.c @top_srcdir(a)/src/xml.c \
- @top_srcdir(a)/src/xs_internal.c @top_srcdir(a)/src/buf.c
@top_srcdir(a)/src/uuid.c
+ @top_srcdir(a)/src/xs_internal.c @top_srcdir(a)/src/buf.c \
+ @top_srcdir(a)/src/util.c @top_srcdir(a)/src/uuid.c
libvirt_proxy_LDFLAGS = $(WARN_CFLAGS)
libvirt_proxy_DEPENDENCIES =
libvirt_proxy_LDADD =
install-exec-hook:
chmod u+s $(DESTDIR)$(libexecdir)/libvirt_proxy
-endif
\ No newline at end of file
+endif
diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c
index d96d3db..12d0f35 100644
--- a/proxy/libvirt_proxy.c
+++ b/proxy/libvirt_proxy.c
@@ -2,7 +2,7 @@
* proxy_svr.c: root suid proxy server for Xen access to APIs with no
* side effects from unauthenticated clients.
*
- * Copyright (C) 2006, 2007 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -317,19 +317,12 @@ proxyWriteClientSocket(int nr, virProxyPacketPtr req) {
return(-1);
}
-retry:
- ret = write(pollInfos[nr].fd, (char *) req, req->len);
+ ret = safewrite(pollInfos[nr].fd, (char *) req, req->len);
if (ret < 0) {
- if (errno == EINTR) {
- if (debug > 0)
- fprintf(stderr, "write socket %d to client %d interrupted\n",
- pollInfos[nr].fd, nr);
- goto retry;
- }
fprintf(stderr, "write %d bytes to socket %d from client %d failed\n",
req->len, pollInfos[nr].fd, nr);
- proxyCloseClientSocket(nr);
- return(-1);
+ proxyCloseClientSocket(nr);
+ return(-1);
}
if (ret == 0) {
if (debug)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index 3a5e44c..269e9fe 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 e0ecdea..53ea993 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/proxy_internal.c b/src/proxy_internal.c
index bc94763..5a14e54 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
*
@@ -345,17 +345,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 e39c7bc..ff6a92e 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 15cd52c..85d042f 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 003d6b7..3dd1007 100644
--- a/src/test.c
+++ b/src/test.c
@@ -1293,19 +1293,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 +1398,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);
--
1.5.4.2.134.g82883