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;