Fix poll and select POSIX compliancy details about errors

This fixes the following:

- Poll must not return immediately on error, including EBADF, and instead
  report POLLHUP/POLLERR/POLLNVAL
- Select must report EBADF if some set contains an invalid FD.

The idea is to move error management to after all select calls, in the
poll/select final treatment. The error is instead recorded in a new `error'
field, and a new SELECT_ERROR bit set.

Thanks Svante Signell for the initial version of the patch.

* hurd/hurdselect.c (SELECT_ERROR): New macro.
(_hurd_select):
- Add `error' field to `d' structures array.
- If a poll descriptor is bogus, set EBADF, but continue.
- Go through the whole fd_set, not only until _hurd_dtablesize. Return EBADF
there is any bit set above _hurd_dtablesize.
- Do not request io_select on bogus descriptors (SELECT_ERROR).
- On io_select request error, record the error.
- On io_select bogus reply, use EIO error code.
- On io_select bogus or error reply, record the error.
- Do not destroy reply port for bogus FDs.
- On error, make poll set POLLHUP in the EPIPE case, POLLNVAL in the EBADF
case, or else POLLERR.
- On error, make select simulated readiness.

1 files changed, 64 insertions, 41 deletions

diff --git a/hurd/hurdselect.c b/hurd/hurdselect.c
index d0214cb6df..19d4ae1519 100644
--- a/hurd/hurdselect.c
+++ b/hurd/hurdselect.c
@@ -34,6 +34,7 @@
 /* Used to record that a particular select rpc returned.  Must be distinct
    from SELECT_ALL (which better not have the high bit set).  */
 #define SELECT_RETURNED ((SELECT_ALL << 1) & ~SELECT_ALL)
+#define SELECT_ERROR (SELECT_RETURNED << 1)
 
 /* Check the first NFDS descriptors either in POLLFDS (if nonnnull) or in
    each of READFDS, WRITEFDS, EXCEPTFDS that is nonnull.  If TIMEOUT is not
@@ -61,6 +62,7 @@ _hurd_select (int nfds,
       mach_port_t io_port;
       int type;
       mach_port_t reply_port;
+      int error;
     } d[nfds];
   sigset_t oset;
 
@@ -156,25 +158,14 @@ _hurd_select (int nfds,
 		  continue;
 	      }
 
-	    /* If one descriptor is bogus, we fail completely.  */
-	    while (i-- > 0)
-	      if (d[i].type != 0)
-		_hurd_port_free (&d[i].cell->port,
-				 &d[i].ulink, d[i].io_port);
-	    break;
+	    /* Bogus descriptor, make it EBADF already.  */
+	    d[i].error = EBADF;
+	    d[i].type = SELECT_ERROR;
 	  }
 
       __mutex_unlock (&_hurd_dtable_lock);
       HURD_CRITICAL_END;
 
-      if (i < nfds)
-	{
-	  if (sigmask)
-	    __sigprocmask (SIG_SETMASK, &oset, NULL);
-	  errno = EBADF;
-	  return -1;
-	}
-
       lastfd = i - 1;
       firstfd = i == 0 ? lastfd : 0;
     }
@@ -199,9 +190,6 @@ _hurd_select (int nfds,
       HURD_CRITICAL_BEGIN;
       __mutex_lock (&_hurd_dtable_lock);
 
-      if (nfds > _hurd_dtablesize)
-	nfds = _hurd_dtablesize;
-
       /* Collect the ports for interesting FDs.  */
       firstfd = lastfd = -1;
       for (i = 0; i < nfds; ++i)
@@ -216,9 +204,12 @@ _hurd_select (int nfds,
 	  d[i].type = type;
 	  if (type)
 	    {
-	      d[i].cell = _hurd_dtable[i];
-	      d[i].io_port = _hurd_port_get (&d[i].cell->port, &d[i].ulink);
-	      if (d[i].io_port == MACH_PORT_NULL)
+	      if (i < _hurd_dtablesize)
+		{
+		  d[i].cell = _hurd_dtable[i];
+		  d[i].io_port = _hurd_port_get (&d[i].cell->port, &d[i].ulink);
+		}
+	      if (i >= _hurd_dtablesize || d[i].io_port == MACH_PORT_NULL)
 		{
 		  /* If one descriptor is bogus, we fail completely.  */
 		  while (i-- > 0)
@@ -243,6 +234,9 @@ _hurd_select (int nfds,
 	  errno = EBADF;
 	  return -1;
 	}
+
+      if (nfds > _hurd_dtablesize)
+	nfds = _hurd_dtablesize;
     }
 
 
@@ -260,7 +254,7 @@ _hurd_select (int nfds,
       portset = MACH_PORT_NULL;
 
       for (i = firstfd; i <= lastfd; ++i)
-	if (d[i].type)
+	if (d[i].type & ~SELECT_ERROR)
 	  {
 	    int type = d[i].type;
 	    d[i].reply_port = __mach_reply_port ();
@@ -294,11 +288,10 @@ _hurd_select (int nfds,
 	      }
 	    else
 	      {
-		/* No error should happen.  Callers of select
-		   don't expect to see errors, so we simulate
-		   readiness of the erring object and the next call
-		   hopefully will get the error again.  */
-		d[i].type |= SELECT_RETURNED;
+		/* No error should happen, but record it for later
+		   processing.  */
+		d[i].error = err;
+		d[i].type |= SELECT_ERROR;
 		++got;
 	      }
 	    _hurd_port_free (&d[i].cell->port, &d[i].ulink, d[i].io_port);
@@ -398,9 +391,10 @@ _hurd_select (int nfds,
 #endif
 		  msg.head.msgh_size != sizeof msg.success)
 		{
-		  /* Error or bogus reply.  Simulate readiness.  */
+		  /* Error or bogus reply.  */
+		  if (!msg.error.err)
+		    msg.error.err = EIO;
 		  __mach_msg_destroy (&msg.head);
-		  msg.success.result = SELECT_ALL;
 		}
 
 	      /* Look up the respondent's reply port and record its
@@ -412,9 +406,18 @@ _hurd_select (int nfds,
 		    if (d[i].type
 			&& d[i].reply_port == msg.head.msgh_local_port)
 		      {
-			d[i].type &= msg.success.result;
-			if (d[i].type)
-			  ++ready;
+			if (msg.error.err)
+			  {
+			    d[i].error = msg.error.err;
+			    d[i].type = SELECT_ERROR;
+			    ++ready;
+			  }
+			else
+			  {
+			    d[i].type &= msg.success.result;
+			    if (d[i].type)
+			      ++ready;
+			  }
 
 			d[i].type |= SELECT_RETURNED;
 			++got;
@@ -452,7 +455,7 @@ _hurd_select (int nfds,
 
   if (firstfd != -1)
     for (i = firstfd; i <= lastfd; ++i)
-      if (d[i].type)
+      if (d[i].type & ~SELECT_ERROR)
 	__mach_port_destroy (__mach_task_self (), d[i].reply_port);
   if (firstfd == -1 || (firstfd != lastfd && portset != MACH_PORT_NULL))
     /* Destroy PORTSET, but only if it's not actually the reply port for a
@@ -474,15 +477,29 @@ _hurd_select (int nfds,
 	int type = d[i].type;
 	int_fast16_t revents = 0;
 
-	if (type & SELECT_RETURNED)
-	  {
-	    if (type & SELECT_READ)
-	      revents |= POLLIN;
-	    if (type & SELECT_WRITE)
-	      revents |= POLLOUT;
-	    if (type & SELECT_URG)
-	      revents |= POLLPRI;
-	  }
+	if (type & SELECT_ERROR)
+	  switch (d[i].error)
+	    {
+	      case EPIPE:
+		revents = POLLHUP;
+		break;
+	      case EBADF:
+		revents = POLLNVAL
+		break;
+	      default:
+		revents = POLLERR;
+		break;
+	    }
+	else
+	  if (type & SELECT_RETURNED)
+	    {
+	      if (type & SELECT_READ)
+		revents |= POLLIN;
+	      if (type & SELECT_WRITE)
+		revents |= POLLOUT;
+	      if (type & SELECT_URG)
+		revents |= POLLPRI;
+	    }
 
 	pollfds[i].revents = revents;
       }
@@ -502,6 +519,12 @@ _hurd_select (int nfds,
 	    if ((type & SELECT_RETURNED) == 0)
 	      type = 0;
 
+	    /* Callers of select don't expect to see errors, so we simulate
+	       readiness of the erring object and the next call hopefully
+	       will get the error again.  */
+	    if (type & SELECT_ERROR)
+	      type = SELECT_ALL;
+
 	    if (type & SELECT_READ)
 	      ready++;
 	    else if (readfds)