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. --- Changes from previous attempt: rework remote_driver to no longer trigger 'make syntax-check' failures. This can wait until after 0.7.7. daemon/event.c | 5 +++-- src/remote/remote_driver.c | 18 ++++++++++++++---- src/util/util.h | 9 ++++++--- 3 files changed, 23 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,13 +1,13 @@ /* * 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 * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -30,16 +30,17 @@ #include <errno.h> #include <unistd.h> #include "threads.h" #include "logging.h" #include "event.h" #include "memory.h" #include "util.h" +#include "ignore-value.h" #define EVENT_DEBUG(fmt, ...) DEBUG(fmt, __VA_ARGS__) static int virEventInterruptLocked(void); /* State for a single file handle being monitored */ struct virEventHandle { int watch; @@ -625,17 +626,17 @@ error_unlocked: static void virEventHandleWakeup(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { char c; virEventLock(); - saferead(fd, &c, sizeof(c)); + ignore_value (saferead(fd, &c, sizeof(c))); virEventUnlock(); } int virEventInit(void) { if (pthread_mutex_init(&eventLoop.lock, NULL) != 0) return -1; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b7b2e09..d9cae8b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7868,26 +7868,35 @@ remoteIOReadBuffer(virConnectPtr conn, if (ret == -1) { if (errno == EINTR) goto resend; if (errno == EWOULDBLOCK) return 0; char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + virReportSystemError(errno, "%s", + _("cannot recv data: unknown reason")); + return -1; + } } virReportSystemError(errno, _("cannot recv data: %s"), errout); } else { char errout[1024] = "\0"; if (priv->errfd != -1) { - saferead(priv->errfd, errout, sizeof(errout)); + if (saferead(priv->errfd, errout, sizeof(errout)) < 0) { + errorf (in_open ? NULL : conn, + VIR_ERR_SYSTEM_ERROR, + _("server closed connection: unknown reason")); + return -1; + } } errorf (in_open ? NULL : conn, VIR_ERR_SYSTEM_ERROR, _("server closed connection: %s"), errout); } return -1; } @@ -8490,17 +8499,18 @@ remoteIOEventLoop(virConnectPtr conn, goto repoll; ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); remoteDriverLock(priv); 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) { if (errno == EWOULDBLOCK) continue; virReportSystemError(errno, "%s", _("poll on socket failed")); goto error; @@ -8634,17 +8644,17 @@ remoteIO(virConnectPtr conn, while (tmp && tmp->next) tmp = tmp->next; if (tmp) tmp->next = thiscall; else 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... */ if (virCondWait(&thiscall->cond, &priv->lock) < 0) { if (priv->waitDispatch == thiscall) { priv->waitDispatch = thiscall->next; } else { tmp = priv->waitDispatch; 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 @@ -1,12 +1,13 @@ /* * utils.h: common, generic utility functions * + * Copyright (C) 2010 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * @@ -26,19 +27,21 @@ #define __VIR_UTIL_H__ #include "verify.h" #include "internal.h" #include <unistd.h> #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, VIR_EXEC_NONBLOCK = (1 << 0), VIR_EXEC_DAEMON = (1 << 1), VIR_EXEC_CLEAR_CAPS = (1 << 2), }; -- 1.6.6.1