On Thu, 2013-01-24 at 12:10 +0100, Svante Signell wrote:
> On Wed, 2013-01-23 at 17:52 +0100, Svante Signell wrote:
> > On Wed, 2013-01-23 at 00:52 +0100, Samuel Thibault wrote:
> > > Svante Signell, le Tue 22 Jan 2013 20:53:06 +0100, a écrit :

Now the restructuring is complete (and without excessive code
duplication).

This patch does the following:
1) Introduce the three switch(ispoll) cases: DELAY, POLL, SELECT

The patch is made against hurdselect_step1_3.c. Everything is prepared
for fixing the POLL bugs and POSIX compliance.

Hopefully the restructured code will have the same behaviour as the old
code. A number of tests have been performed with different examples, but
of course the new code is not exclusively tested.

--- hurdselect_step1_3.c	2013-01-24 11:40:13.000000000 +0100
+++ hurdselect_step2.c	2013-01-24 13:40:59.000000000 +0100
@@ -331,8 +331,27 @@ _hurd_select (int nfds,
   if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset))
     return -1;
 
-  if (pollfds)
+  err = 0;
+  got = 0;
+
+  /* Send them all io_select request messages.  */
+
+  switch (ispoll)
     {
+    case DELAY:
+    /* But not if there were no ports to deal with at all.
+       We are just a pure timeout.  */
+      portset = __mach_reply_port ();
+      firstfd = lastfd = -1;
+
+      got = _wait_for_replies (nfds, firstfd, lastfd, to, 
+			       err, portset, d,
+			       timeout, sigmask, &oset);
+      if (got == -1)
+	return -1;
+      break;
+
+    case POLL:
       /* Collect interesting descriptors from the user's `pollfd' array.
 	 We do a first pass that reads the user's array before taking
 	 any locks.  The second pass then only touches our own stack,
@@ -371,6 +390,7 @@ _hurd_select (int nfds,
 		  continue;
 	      }
 
+	    /* FIXME: This is a bug */
 	    /* If one descriptor is bogus, we fail completely.  */
 	    while (i-- > 0)
 	      if (d[i].type != 0)
@@ -392,9 +412,39 @@ _hurd_select (int nfds,
 
       lastfd = i - 1;
       firstfd = i == 0 ? lastfd : 0;
-    } /* POLL */
-  else
-    {
+
+      got =  _io_select_request (nfds, firstfd, lastfd,
+				 err, &portset, d);
+      if (got == -1)
+	return -1;
+
+      got = _wait_for_replies (nfds, firstfd, lastfd, to, 
+			       err, portset, d,
+			       timeout, sigmask, &oset);
+      if (got == -1)
+	return -1;
+
+      /* Fill in the `revents' members of the user's array.  */
+      for (i = 0; i < nfds; ++i)
+	{
+	  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;
+	    }
+
+	  pollfds[i].revents = revents;
+	}
+      break;
+
+    case SELECT:
       /* Collect interested descriptors from the user's fd_set arguments.
 	 Use local copies so we can't crash from user bogosity.  */
 
@@ -458,51 +508,18 @@ _hurd_select (int nfds,
 	  errno = EBADF;
 	  return -1;
 	}
-    } /* SELECT */
-
-
-  err = 0;
-  got = 0;
-
-  /* Send them all io_select request messages.  */
-
-  if (firstfd == -1) /* DELAY */
-    /* But not if there were no ports to deal with at all.
-       We are just a pure timeout.  */
-    portset = __mach_reply_port ();
-  else /* POLL || SELECT */
-    got =  _io_select_request (nfds, firstfd, lastfd,
-			       err, &portset, d);
-  if (got == -1)
-    return -1;
-
-  got = _wait_for_replies (nfds, firstfd, lastfd, to, 
-			   err, portset, d,
-			   timeout, sigmask, &oset);
-  if (got == -1)
-    return -1;
 
-  if (pollfds) /* POLL */
-    /* Fill in the `revents' members of the user's array.  */
-    for (i = 0; i < nfds; ++i)
-      {
-	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;
-	  }
+      got =  _io_select_request (nfds, firstfd, lastfd,
+				 err, &portset, d);
+      if (got == -1)
+	return -1;
+
+      got = _wait_for_replies (nfds, firstfd, lastfd, to, 
+			       err, portset, d,
+			       timeout, sigmask, &oset);
+      if (got == -1)
+	return -1;
 
-	pollfds[i].revents = revents;
-      }
-  else /* SELECT */
-    {
       /* Below we recalculate GOT to include an increment for each operation
 	 allowed on each fd.  */
       got = 0;
@@ -530,7 +547,8 @@ _hurd_select (int nfds,
 	    else if (exceptfds)
 	      FD_CLR (i, exceptfds);
 	  }
-    } /* SELECT */
+      break;
+    } /* switch (ispoll) */
 
   if (sigmask && __sigprocmask (SIG_SETMASK, &oset, NULL))
     return -1;

Reply via email to