Add a properly-safe version of select(2).

Do the timeout handling the right and cross-platform way. The code
that was in QProcess worked only on Linux, where the kernel sets the
remainder in the returned timeval structure.

Reviewed-By: ossi

3 files changed, 159 insertions, 43 deletions

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index c81da44..fafce07 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -128,6 +128,13 @@ static void qt_sa_sigchld_handler(int signum)
         qt_sa_old_sigchld_handler(signum);
 }
 
+static inline void add_fd(int &nfds, int fd, fd_set *fdset)
+{
+    FD_SET(fd, fdset);
+    if ((fd) > nfds)
+        nfds = fd;
+}
+
 struct QProcessInfo {
     QProcess *process;
     int deathPipe;
@@ -845,17 +852,15 @@ void QProcessPrivate::killProcess()
         ::kill(pid_t(pid), SIGKILL);
 }
 
-static int qt_safe_select(fd_set *fdread, fd_set *fdwrite, int timeout)
+static int select_msecs(int nfds, fd_set *fdread, fd_set *fdwrite, int timeout)
 {
+    if (timeout < 0)
+        return qt_safe_select(nfds, fdread, fdwrite, 0, 0);
+
     struct timeval tv;
     tv.tv_sec = timeout / 1000;
     tv.tv_usec = (timeout % 1000) * 1000;
-
-    int ret;
-    do {
-        ret = select(FD_SETSIZE, fdread, fdwrite, 0, timeout < 0 ? 0 : &tv);
-    } while (ret < 0 && (errno == EINTR));
-    return ret;
+    return qt_safe_select(nfds, fdread, fdwrite, 0, &tv);
 }
 
 /*
@@ -883,11 +888,7 @@ bool QProcessPrivate::waitForStarted(int msecs)
     fd_set fds;
     FD_ZERO(&fds);
     FD_SET(childStartedPipe[0], &fds);
-    int ret;
-    do {
-        ret = qt_safe_select(&fds, 0, msecs);
-    } while (ret < 0 && errno == EINTR);
-    if (ret == 0) {
+    if (select_msecs(childStartedPipe[0] + 1, &fds, 0, msecs) == 0) {
         processError = QProcess::Timedout;
         q->setErrorString(QProcess::tr("Process operation timed out"));
 #if defined (QPROCESS_DEBUG)
@@ -920,24 +921,23 @@ bool QProcessPrivate::waitForReadyRead(int msecs)
         FD_ZERO(&fdread);
         FD_ZERO(&fdwrite);
 
+        int nfds = deathPipe[0];
+        FD_SET(deathPipe[0], &fdread);
+
         if (processState == QProcess::Starting)
-            FD_SET(childStartedPipe[0], &fdread);
+            add_fd(nfds, childStartedPipe[0], &fdread);
 
         if (stdoutChannel.pipe[0] != -1)
-            FD_SET(stdoutChannel.pipe[0], &fdread);
+            add_fd(nfds, stdoutChannel.pipe[0], &fdread);
         if (stderrChannel.pipe[0] != -1)
-            FD_SET(stderrChannel.pipe[0], &fdread);
-
-        FD_SET(deathPipe[0], &fdread);
+            add_fd(nfds, stderrChannel.pipe[0], &fdread);
 
         if (!writeBuffer.isEmpty() && stdinChannel.pipe[1] != -1)
-            FD_SET(stdinChannel.pipe[1], &fdwrite);
+            add_fd(nfds, stdinChannel.pipe[1], &fdwrite);
 
         int timeout = qt_timeout_value(msecs, stopWatch.elapsed());
-        int ret = qt_safe_select(&fdread, &fdwrite, timeout);
+        int ret = select_msecs(nfds + 1, &fdread, &fdwrite, timeout);
         if (ret < 0) {
-            if (errno == EINTR)
-                continue;
             break;
         }
         if (ret == 0) {
@@ -993,24 +993,24 @@ bool QProcessPrivate::waitForBytesWritten(int msecs)
         FD_ZERO(&fdread);
         FD_ZERO(&fdwrite);
 
+        int nfds = deathPipe[0];
+        FD_SET(deathPipe[0], &fdread);
+
         if (processState == QProcess::Starting)
-            FD_SET(childStartedPipe[0], &fdread);
+            add_fd(nfds, childStartedPipe[0], &fdread);
 
         if (stdoutChannel.pipe[0] != -1)
-            FD_SET(stdoutChannel.pipe[0], &fdread);
+            add_fd(nfds, stdoutChannel.pipe[0], &fdread);
         if (stderrChannel.pipe[0] != -1)
-            FD_SET(stderrChannel.pipe[0], &fdread);
+            add_fd(nfds, stderrChannel.pipe[0], &fdread);
 
-        FD_SET(deathPipe[0], &fdread);
 
         if (!writeBuffer.isEmpty() && stdinChannel.pipe[1] != -1)
-            FD_SET(stdinChannel.pipe[1], &fdwrite);
+            add_fd(nfds, stdinChannel.pipe[1], &fdwrite);
 
 	int timeout = qt_timeout_value(msecs, stopWatch.elapsed());
-	int ret = qt_safe_select(&fdread, &fdwrite, timeout);
+	int ret = select_msecs(nfds + 1, &fdread, &fdwrite, timeout);
         if (ret < 0) {
-            if (errno == EINTR)
-                continue;
             break;
         }
 
@@ -1056,29 +1056,28 @@ bool QProcessPrivate::waitForFinished(int msecs)
     forever {
         fd_set fdread;
         fd_set fdwrite;
+        int nfds = -1;
 
         FD_ZERO(&fdread);
         FD_ZERO(&fdwrite);
 
         if (processState == QProcess::Starting)
-            FD_SET(childStartedPipe[0], &fdread);
+            add_fd(nfds, childStartedPipe[0], &fdread);
 
         if (stdoutChannel.pipe[0] != -1)
-            FD_SET(stdoutChannel.pipe[0], &fdread);
+            add_fd(nfds, stdoutChannel.pipe[0], &fdread);
         if (stderrChannel.pipe[0] != -1)
-            FD_SET(stderrChannel.pipe[0], &fdread);
+            add_fd(nfds, stderrChannel.pipe[0], &fdread);
 
         if (processState == QProcess::Running)
-            FD_SET(deathPipe[0], &fdread);
+            add_fd(nfds, deathPipe[0], &fdread);
 
         if (!writeBuffer.isEmpty() && stdinChannel.pipe[1] != -1)
-            FD_SET(stdinChannel.pipe[1], &fdwrite);
+            add_fd(nfds, stdinChannel.pipe[1], &fdwrite);
 
 	int timeout = qt_timeout_value(msecs, stopWatch.elapsed());
-	int ret = qt_safe_select(&fdread, &fdwrite, timeout);
+	int ret = select_msecs(nfds + 1, &fdread, &fdwrite, timeout);
         if (ret < 0) {
-            if (errno == EINTR)
-                continue;
             break;
         }
 	if (ret == 0) {
@@ -1113,12 +1112,7 @@ bool QProcessPrivate::waitForWrite(int msecs)
     fd_set fdwrite;
     FD_ZERO(&fdwrite);
     FD_SET(stdinChannel.pipe[1], &fdwrite);
-
-    int ret;
-    do {
-        ret = qt_safe_select(0, &fdwrite, msecs < 0 ? 0 : msecs) == 1;
-    } while (ret < 0 && errno == EINTR);
-    return ret == 1;
+    return select_msecs(stdinChannel.pipe[1] + 1, 0, &fdwrite, msecs < 0 ? 0 : msecs) == 1;
 }
 
 void QProcessPrivate::findExitCode()
diff --git a/src/corelib/kernel/kernel.pri b/src/corelib/kernel/kernel.pri
index 6f28029..8759578 100644
--- a/src/corelib/kernel/kernel.pri
+++ b/src/corelib/kernel/kernel.pri
@@ -88,6 +88,7 @@ mac {
 
 unix {
 	SOURCES += \
+                kernel/qcore_unix.cpp \
                 kernel/qcrashhandler.cpp \
                 kernel/qsharedmemory_unix.cpp \
                 kernel/qsystemsemaphore_unix.cpp
diff --git a/src/corelib/kernel/qcore_unix.cpp b/src/corelib/kernel/qcore_unix.cpp
new file mode 100644
index 0000000..ffaf958
--- /dev/null
+++ b/src/corelib/kernel/qcore_unix.cpp
@@ -0,0 +1,121 @@
+/****************************************************************************
+**
+** Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies).
+** Contact: Qt Software Information (qt-info@nokia.com)
+**
+** This file is part of the QtCore module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** No Commercial Usage
+** This file contains pre-release code and may not be distributed.
+** You may use this file in accordance with the terms and conditions
+** contained in the either Technology Preview License Agreement or the
+** Beta Release License Agreement.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Nokia gives you certain
+** additional rights. These rights are described in the Nokia Qt LGPL
+** Exception version 1.0, included in the file LGPL_EXCEPTION.txt in this
+** package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+** If you are unsure which license is appropriate for your use, please
+** contact the sales department at qt-sales@nokia.com.
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "qcore_unix_p.h"
+
+#include <sys/select.h>
+#include <sys/time.h>
+#include <stdlib.h>
+
+#include "qeventdispatcher_unix_p.h" // for the timeval operators
+
+#if !defined(QT_NO_CLOCK_MONOTONIC)
+# if defined(QT_BOOTSTRAPPED)
+#  define QT_NO_CLOCK_MONOTONIC
+# endif
+#endif
+
+QT_BEGIN_NAMESPACE
+
+static inline timeval gettime()
+{
+    timeval tv;
+#ifndef QT_NO_CLOCK_MONOTONIC
+    // use the monotonic clock
+    static volatile bool monotonicClockDisabled = false;
+    struct timespec ts;
+    if (!monotonicClockDisabled) {
+        if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1) {
+            monotonicClockDisabled = true;
+        } else {
+            tv.tv_sec = ts.tv_sec;
+            tv.tv_usec = ts.tv_nsec / 1000;
+            return tv;
+        }
+    }
+#endif
+    // use gettimeofday
+    ::gettimeofday(&tv, 0);
+    return tv;
+}
+
+static inline bool time_update(struct timeval *tv, const struct timeval &start,
+                               const struct timeval &timeout)
+{
+    struct timeval now = gettime();
+    if (now < start) {
+        // clock reset, give up
+        return false;
+    }
+
+    *tv = timeout + start - now;
+    return true;
+}
+
+int qt_safe_select(int nfds, fd_set *fdread, fd_set *fdwrite, fd_set *fdexcept,
+                   const struct timeval *orig_timeout)
+{
+    if (!orig_timeout) {
+        // no timeout -> block forever
+        register int ret;
+        EINTR_LOOP(ret, select(nfds, fdread, fdwrite, fdexcept, 0));
+        return ret;
+    }
+
+    timeval start = gettime();
+    timeval timeout = *orig_timeout;
+
+    // loop and recalculate the timeout as needed
+    int ret;
+    forever {
+        ret = ::select(nfds, fdread, fdwrite, fdexcept, &timeout);
+        if (ret != -1 || errno != EINTR)
+            return ret;
+
+        // recalculate the timeout
+        if (!time_update(&timeout, start, *orig_timeout)) {
+            // clock reset, fake timeout error
+            return 0;
+        }
+    }
+}
+
+QT_END_NAMESPACE