From f9f6c34c5ca8446d4b90b02525524dac35d05b46 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 2 Mar 2010 09:35:01 -0700
Subject: [PATCH] util: ensure safe{read,write,zero} return is checked

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.
---
 daemon/event.c             |  5 +++--
 src/remote/remote_driver.c | 22 ++++++++++++++++------
 src/util/util.h            |  9 ++++++---
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/daemon/event.c b/daemon/event.c
index 2218a3e489..69714091dc 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 f514e1d5dc..ebcfcd86b8 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8877,7 +8877,11 @@ remoteIOReadBuffer(struct private_data *priv,
 
                 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"));
+                        return -1;
+                    }
                 }
 
                 virReportSystemError(errno,
@@ -8886,7 +8890,12 @@ remoteIOReadBuffer(struct private_data *priv,
             } else {
                 char errout[1024] = "\0";
                 if (priv->errfd != -1) {
-                    saferead(priv->errfd, errout, sizeof(errout));
+                    if (saferead(priv->errfd, errout, sizeof(errout)) < 0) {
+                        remoteError(VIR_ERR_SYSTEM_ERROR,
+                                    _("server closed connection: %s"),
+                                    virStrerror(errno, errout, sizeof errout));
+                        return -1;
+                    }
                 }
 
                 remoteError(VIR_ERR_SYSTEM_ERROR,
@@ -9499,7 +9508,7 @@ remoteIOEventLoop(virConnectPtr conn,
         sigaddset (&blockedsigs, SIGWINCH);
         sigaddset (&blockedsigs, SIGCHLD);
         sigaddset (&blockedsigs, SIGPIPE);
-        ignore_value (pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask));
+        ignore_value(pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask));
 #endif
 
     repoll:
@@ -9508,14 +9517,15 @@ remoteIOEventLoop(virConnectPtr conn,
             goto repoll;
 
 #ifdef HAVE_PTHREAD_H
-        ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
+        ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
 #endif
 
         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) {
@@ -9659,7 +9669,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 c25611763e..4f0b23300e 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,
-- 
GitLab