[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv2] 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.
---
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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]