Hi,

I have been experiencing several problem with the poll module,
especially under MacOS X.

The module make the following assumption (quoting from the source): "An
hung up descriptor does not increase the return value".

- However, POSIX state:
Upon  successful  completion,  poll() shall return a non-negative value.
A positive value indicates the total number of file descriptors that
have been selected (that is, file descriptors for which the revents
member is non-zero). A value of 0 indicates that the call timed  out
and  no  file descriptors have been selected. Upon failure, poll() shall
return -1 and set errno to indicate the error.

- More specifically, the Linux manpage state:
On  success, a positive number is returned; this is the number of
structures which have non-zero revents fields (in other words, those
descriptors with events ***or errors reported***). [...]


The attached patch fix this issue (as well as a bzero() warning, due to
string.h not being included).


There is a second problem where poll() would fail with a non connected
socket (example: server socket polled for incoming connection event).

The attached patch fix this issue in the following way:
- [ALL except OSX]: if recv() return -1 with errno set to ENOTCONN, then
trigger a POLLIN.

- [OSX]: The situation is more complicated since recv() was not used due
to a OSX bug where data would be consumed despite the MSG_PEEK flag
being set. FIONREAD was used in place of the recv() call.

However, FIONREAD does not permit to determine whether the connection
has been closed, or if we are working with a non connected socket (it
will return 0/set available data to 0 in both case). 

Instead of using FIONREAD, I modified the check to use recv() with a 0
length (which is, AFAIK, not portable, but seems to work under OSX).
This doesn't consume any data, but permit to make the distinction
between both case.

If anyone has a better idea on how to fix the above issues, any input is
welcome.

Regards, 

-- 
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)8 70 70 21 58                  Fax: +33(0)4 78 42 21 58
http://www.prelude-ids.com
Index: lib/poll.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/poll.c,v
retrieving revision 1.5
diff -u -r1.5 poll.c
--- lib/poll.c	28 Sep 2006 19:58:33 -0000	1.5
+++ lib/poll.c	18 Dec 2006 19:23:49 -0000
@@ -28,6 +28,7 @@
 #include <sys/socket.h>
 #include <sys/select.h>
 #include <unistd.h>
+#include <string.h>
 
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
@@ -158,40 +159,33 @@
 	  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;
+                some kinds of descriptors. Use a length of 0. */
+
+              char *data = NULL;
+              size_t len = 0;
 #else
-	      char data[64];
-	      r = recv (pfd[i].fd, data, 64, MSG_PEEK);
-	      if (r == -1)
+              char data[64];
+              size_t len = sizeof(data);
+#endif
+	      r = recv (pfd[i].fd, data, len, MSG_PEEK);
+	      if (r > 0)
+	        happened = POLLIN|POLLRDNORM;
+	        
+	      else if (r == 0)
+	        happened = (len == 0 /* Mac OSX */ ) ? POLLIN|POLLRDNORM : POLLHUP;
+	        
+	      else if (r < 0)
 		{
-		  avail = (errno == ESHUTDOWN || errno == ECONNRESET ||
-	                   errno == ECONNABORTED || errno == ENETRESET) ? 0 : -1;
+		  if (errno == ENOTCONN)
+		    happened = POLLIN|POLLRDNORM; /* Event happening on an unconnected server socket. */
+		  else
+		    happened = (errno == ESHUTDOWN || errno == ECONNRESET ||
+	                        errno == ECONNABORTED || errno == ENETRESET) ? POLLHUP : POLLERR;
 		  errno = 0;
 		}
-	      else
-	        avail = r;
-#endif
-
-	      /* 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;
 	    }
 
 	  if (FD_ISSET (pfd[i].fd, &wfds))
@@ -200,7 +194,7 @@
 	  if (FD_ISSET (pfd[i].fd, &efds))
 	    happened |= POLLPRI | POLLRDBAND;
 
-	  pfd[i].revents |= pfd[i].events & happened;
+	  pfd[i].revents |= happened;
 	  rc += (happened > 0);
 	}
     }

Reply via email to