From: Samuel Thibault Subject: Fix poll and select POSIX compliancy details about errors This fixes the following: - On error, poll must not return without polling, 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 with a zero timeout. - 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. --- hurd/hurdselect.c | 124 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 41 deletions(-) --- 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,33 @@ _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; + + /* And set timeout to 0. */ + { + struct timeval now; + err = __gettimeofday(&now, NULL); + if (err) + { + err = errno; + while (i-- > 0) + if (d[i].type & ~SELECT_ERROR != 0) + _hurd_port_free (&d[i].cell->port, &d[i].ulink, + d[i].io_port); + errno = err; + return -1; + } + ts.tv_sec = now.tv_sec; + ts.tv_nsec = now.tv_usec * 1000; + reply_msgid = IO_SELECT_TIMEOUT_REPLY_MSGID; + } } __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 +209,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 +223,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 +253,9 @@ _hurd_select (int nfds, errno = EBADF; return -1; } + + if (nfds > _hurd_dtablesize) + nfds = _hurd_dtablesize; } @@ -260,7 +273,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 +307,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); @@ -404,9 +416,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 @@ -418,9 +431,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; @@ -461,7 +483,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 @@ -483,15 +505,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; } @@ -511,6 +547,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)