Re: [PATCH]: multiple problem with the poll module

[Paolo Bonzini]

> The previous patch had an issue. Here is an updated version.

Ok, here is what I'm going to commit.  I simplified a bit the logic to 
not using ioctl on hosts other than Mac OS X.

The patch is -b to simplify reading it.

Thanks,

Paolo

2007-01-03  Paolo Bonzini  <address@hidden>
            Yoann Vandoorselaere <address@hidden>

        * poll.c (poll): Use recv on Mac OS X to distinguish connected
        sockets, server sockets, and other file descriptors.  Count errors
        to compute the return value.  Reorder the code a bit to be easier
        to follow.  Don't set event bits that were not requested (except
        POLLERR and POLLHUP).

Index: poll.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/poll.c,v
retrieving revision 1.5
diff -u -b -r1.5 poll.c
--- poll.c      28 Sep 2006 19:58:33 -0000      1.5
+++ poll.c      3 Jan 2007 10:46:09 -0000
@@ -63,7 +63,7 @@
 {
   fd_set rfds, wfds, efds;
   struct timeval tv, *ptv;
-  int maxfd, rc, happened;
+  int maxfd, rc;
   nfds_t i;
 
 #ifdef _SC_OPEN_MAX
@@ -143,65 +143,60 @@
 
   /* examine fd sets */
   rc = select (maxfd + 1, &rfds, &wfds, &efds, ptv);
+  if (rc < 0)
+    return rc;
 
   /* establish results */
-  if (rc > 0)
-    {
       rc = 0;
       for (i = 0; i < nfd; i++)
-       {
-         pfd[i].revents = 0;
          if (pfd[i].fd < 0)
-           continue;
-
-         happened = 0;
+      pfd[i].revents = 0;
+    else
+      {
+       int happened = 0, sought = pfd[i].events;
          if (FD_ISSET (pfd[i].fd, &rfds))
            {
              int r;
-             long avail = -1;
-             /* support for POLLHUP.  */
+           
 #if defined __MACH__ && defined __APPLE__
-             /* There is a bug in Mac OS X that causes it to ignore MSG_PEEK 
for
-                some kinds of descriptors.  Use FIONREAD to emulate POLLHUP.
-                It is still not completely POSIX compliant (it does not fully
-                work on TTYs), but at least it does not delete data!  For other
-                platforms, we still use MSG_PEEK because it was proved to be
-                reliable, and I a leery of changing it.  */
-             do
-               r = ioctl (pfd[i].fd, FIONREAD, &avail);
-             while (r == -1 && (errno == EAGAIN || errno == EINTR));
-             if (avail < 0)
-               avail = 0;
+           /* There is a bug in Mac OS X that causes it to ignore MSG_PEEK
+              for some kinds of descriptors.  Detect if this descriptor is a
+              connected socket, a server socket, or something else using a
+              0-byte recv, and use ioctl(2) to detect POLLHUP.  */
+           r = recv (pfd[i].fd, NULL, 0, MSG_PEEK);
+           if (r == 0 || errno == ENOTSOCK)
+             ioctl(pfd[i].fd, FIONREAD, &r);
 #else
              char data[64];
-             r = recv (pfd[i].fd, data, 64, MSG_PEEK);
-             if (r == -1)
-               {
-                 avail = (errno == ESHUTDOWN || errno == ECONNRESET ||
-                          errno == ECONNABORTED || errno == ENETRESET) ? 0 : 
-1;
-                 errno = 0;
-               }
-             else
-               avail = r;
+           r = recv (pfd[i].fd, data, sizeof (data), MSG_PEEK);
 #endif
+           if (r == 0)
+             happened |= POLLHUP;
+           
+           /* If the event happened on an unconnected server socket,
+              that's fine. */
+           else if (r > 0 || ( /* (r == -1) && */ errno == ENOTCONN))
+             happened |= (POLLIN | POLLRDNORM) & sought;
+           
+           /* Distinguish hung-up sockets from other errors.  */
+           else if (errno == ESHUTDOWN || errno == ECONNRESET
+                    || errno == ECONNABORTED || errno == ENETRESET)
+             happened |= POLLHUP;
 
-             /* An hung up descriptor does not increase the return value! */
-             if (avail == 0)
-               pfd[i].revents |= POLLHUP;
-             else if (avail == -1)
-               pfd[i].revents |= POLLERR;
              else
-               happened |= POLLIN | POLLRDNORM;
+             happened |= POLLERR;
            }
 
          if (FD_ISSET (pfd[i].fd, &wfds))
-           happened |= POLLOUT | POLLWRNORM | POLLWRBAND;
+         happened |= (POLLOUT | POLLWRNORM | POLLWRBAND) & sought;
 
          if (FD_ISSET (pfd[i].fd, &efds))
-           happened |= POLLPRI | POLLRDBAND;
+         happened |= (POLLPRI | POLLRDBAND) & sought;
 
-         pfd[i].revents |= pfd[i].events & happened;
-         rc += (happened > 0);
+       if (happened)
+         {
+           pfd[i].revents = happened;
+           rc++;
        }
     }