10:40 a.m.
Based on a warning from coverity. The safe* functions
guarantee complete transactions on success, but don't guarantee
freedom from failure.
* src/util/util.h (saferead, safewrite, safezero): Add
ATTRIBUTE_RETURN_CHECK.
* src/remote/remote_driver.c (remoteIO, remoteIOEventLoop): Ignore
some failures.
(remoteIOReadBuffer): Adjust error messages on read failure.
* daemon/event.c (virEventHandleWakeup): Ignore read failure.
---
Coverity claimed that safewrite was checked 37 out of 46 times,
but only flagged one instance of a CHECKED_RETURN report,
against remote_driver.c. I didn't spot the 9 unchecked calls
that coverity's numbers would imply. Meanwhile, adding
the attribute ensures that we don't forget this in the future.
Please double-check if my use of ignore_value on some of the
previously unchecked calls makes sense, or if we need a more
invasive patch to deal with real failure.
daemon/event.c | 5 +++--
src/remote/remote_driver.c | 13 +++++++++----
src/util/util.h | 9 ++++++---
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/daemon/event.c b/daemon/event.c
index 2218a3e..36e9dd3 100644
--- a/daemon/event.c
+++ b/daemon/event.c
@@ -1,8 +1,8 @@
/*
* event.c: event loop for monitoring file handles
*
+ * Copyright (C) 2007, 2010 Red Hat, Inc.
* Copyright (C) 2007 Daniel P. Berrange
- * Copyright (C) 2007 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
@@ -35,6 +35,7 @@
#include "event.h"
#include "memory.h"
#include "util.h"
+#include "ignore-value.h"
#define EVENT_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__)
@@ -630,7 +631,7 @@ static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED,
{
char c;
virEventLock();
- saferead(fd, &c, sizeof(c));
+ ignore_value (saferead(fd, &c, sizeof(c)));
virEventUnlock();
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index def4617..47d8c56 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7809,7 +7809,9 @@ remoteIOReadBuffer(virConnectPtr conn,
char errout[1024] = "\0";
if (priv->errfd != -1) {
- saferead(priv->errfd, errout, sizeof(errout));
+ if (saferead(priv->errfd, errout, sizeof(errout)) < 0) {
+ strncpy (errout, _("unknown reason"), sizeof errout);
+ }
}
virReportSystemError(errno,
@@ -7818,7 +7820,9 @@ remoteIOReadBuffer(virConnectPtr conn,
} else {
char errout[1024] = "\0";
if (priv->errfd != -1) {
- saferead(priv->errfd, errout, sizeof(errout));
+ if (saferead(priv->errfd, errout, sizeof(errout)) < 0) {
+ strncpy (errout, _("unknown reason"), sizeof errout);
+ }
}
errorf (in_open ? NULL : conn,
@@ -8431,7 +8435,8 @@ remoteIOEventLoop(virConnectPtr conn,
if (fds[1].revents) {
DEBUG0("Woken up from poll by other thread");
- saferead(priv->wakeupReadFD, &ignore, sizeof(ignore));
+ ignore_value (saferead(priv->wakeupReadFD, &ignore,
+ sizeof(ignore)));
}
if (ret < 0) {
@@ -8575,7 +8580,7 @@ remoteIO(virConnectPtr conn,
priv->waitDispatch = thiscall;
/* Force other thread to wakup from poll */
- safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore));
+ ignore_value (safewrite(priv->wakeupSendFD, &ignore, sizeof(ignore)));
DEBUG("Going to sleep %d %p %p", thiscall->proc_nr,
priv->waitDispatch, thiscall);
/* Go to sleep while other thread is working... */
diff --git a/src/util/util.h b/src/util/util.h
index cc05abe..34b77fa 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -2,6 +2,7 @@
/*
* utils.h: common, generic utility functions
*
+ * Copyright (C) 2010 Red Hat, Inc.
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
*
@@ -31,9 +32,11 @@
#include <sys/select.h>
#include <sys/types.h>
-int saferead(int fd, void *buf, size_t count);
-ssize_t safewrite(int fd, const void *buf, size_t count);
-int safezero(int fd, int flags, off_t offset, off_t len);
+int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
+ssize_t safewrite(int fd, const void *buf, size_t count)
+ ATTRIBUTE_RETURN_CHECK;
+int safezero(int fd, int flags, off_t offset, off_t len)
+ ATTRIBUTE_RETURN_CHECK;
enum {
VIR_EXEC_NONE = 0,
--
1.6.6.1