changeset 17924:68876b89b3ad

poll: fixes for large fds * lib/poll.c (poll): Don't check directly for NFD too large. Don't rely on undefined behavior in FD_SET when an arg exceeds FD_SETSIZE. Always set revents afterwards, even if to zero. * tests/test-poll.c (poll1): Set revents to -1 instead of 0, as that makes the test a bit stricter.
author Paul Eggert <eggert@cs.ucla.edu>
date Fri, 20 Feb 2015 10:37:49 -0800
parents 2b5c3699ac6f
children b76a3e3d3926
files ChangeLog lib/poll.c tests/test-poll.c
diffstat 3 files changed, 31 insertions(+), 43 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2015-02-20  Paul Eggert  <eggert@cs.ucla.edu>
+
+	poll: fixes for large fds
+	* lib/poll.c (poll): Don't check directly for NFD too large.
+	Don't rely on undefined behavior in FD_SET when an arg exceeds
+	FD_SETSIZE.  Always set revents afterwards, even if to zero.
+	* tests/test-poll.c (poll1): Set revents to -1 instead of 0,
+	as that makes the test a bit stricter.
+
 2015-02-19  Kevin Cernekee  <cernekee@google.com>
 
 	fcntl: Fix cross compiling
--- a/lib/poll.c
+++ b/lib/poll.c
@@ -334,26 +334,15 @@
   int maxfd, rc;
   nfds_t i;
 
-# ifdef _SC_OPEN_MAX
-  static int sc_open_max = -1;
-
-  if (nfd < 0
-      || (nfd > sc_open_max
-          && (sc_open_max != -1
-              || nfd > (sc_open_max = sysconf (_SC_OPEN_MAX)))))
+  if (nfd < 0)
     {
       errno = EINVAL;
       return -1;
     }
-# else /* !_SC_OPEN_MAX */
-#  ifdef OPEN_MAX
-  if (nfd < 0 || nfd > OPEN_MAX)
-    {
-      errno = EINVAL;
-      return -1;
-    }
-#  endif /* OPEN_MAX -- else, no check is needed */
-# endif /* !_SC_OPEN_MAX */
+  /* Don't check directly for NFD too large.  Any practical use of a
+     too-large NFD is caught by one of the other checks below, and
+     checking directly for getdtablesize is too much of a portability
+     and/or performance and/or correctness hassle.  */
 
   /* EFAULT is not necessary to implement, but let's do it in the
      simplest case. */
@@ -394,10 +383,17 @@
     {
       if (pfd[i].fd < 0)
         continue;
-
+      if (maxfd < pfd[i].fd)
+        {
+          maxfd = pfd[i].fd;
+          if (FD_SETSIZE <= maxfd)
+            {
+              errno = EINVAL;
+              return -1;
+            }
+        }
       if (pfd[i].events & (POLLIN | POLLRDNORM))
         FD_SET (pfd[i].fd, &rfds);
-
       /* see select(2): "the only exceptional condition detectable
          is out-of-band data received on a socket", hence we push
          POLLWRBAND events onto wfds instead of efds. */
@@ -405,18 +401,6 @@
         FD_SET (pfd[i].fd, &wfds);
       if (pfd[i].events & (POLLPRI | POLLRDBAND))
         FD_SET (pfd[i].fd, &efds);
-      if (pfd[i].fd >= maxfd
-          && (pfd[i].events & (POLLIN | POLLOUT | POLLPRI
-                               | POLLRDNORM | POLLRDBAND
-                               | POLLWRNORM | POLLWRBAND)))
-        {
-          maxfd = pfd[i].fd;
-          if (maxfd > FD_SETSIZE)
-            {
-              errno = EOVERFLOW;
-              return -1;
-            }
-        }
     }
 
   /* examine fd sets */
@@ -427,18 +411,13 @@
   /* establish results */
   rc = 0;
   for (i = 0; i < nfd; i++)
-    if (pfd[i].fd < 0)
-      pfd[i].revents = 0;
-    else
-      {
-        int happened = compute_revents (pfd[i].fd, pfd[i].events,
-                                        &rfds, &wfds, &efds);
-        if (happened)
-          {
-            pfd[i].revents = happened;
-            rc++;
-          }
-      }
+    {
+      pfd[i].revents = (pfd[i].fd < 0
+                        ? 0
+                        : compute_revents (pfd[i].fd, pfd[i].events,
+                                           &rfds, &wfds, &efds));
+      rc += pfd[i].revents != 0;
+    }
 
   return rc;
 #else
--- a/tests/test-poll.c
+++ b/tests/test-poll.c
@@ -166,7 +166,7 @@
 
   pfd.fd = fd;
   pfd.events = ev;
-  pfd.revents = 0;
+  pfd.revents = -1;
   r = poll (&pfd, 1, time);
   if (r < 0)
     return r;