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 :
> Since there was a lot of concerns about code duplication I introduced a
> helper function _wait_for_replies(), splitting out the union definitions
> and the _mach_msg() call.
Introduced another helper function: _io_select_request()
The patch does the following:
1) Create another helper function: _io_select_request() (good enough name?)
2) Sets got to -1 in case of errors and always return got in both helper
functions.
(Hopefully I got the call by value and call by reference issues right)
The patch is made against hurdselect_step1_2.c.
Next step will be to introduce the three cases: DELAY POLL, SELECT
before adding the needed changes for POLL.
--- hurdselect_step1_2.c 2013-01-23 17:23:30.000000000 +0100
+++ hurdselect_step1_3.c 2013-01-24 11:40:13.000000000 +0100
@@ -42,7 +42,77 @@ struct dfd
mach_port_t reply_port;
};
-/* Helper function */
+/* Helper functions */
+int _io_select_request (int nfds, int firstfd, int lastfd,
+ error_t err, mach_port_t *portset, struct dfd *d)
+{
+ *portset = MACH_PORT_NULL;
+ int i, got = 0;
+
+ for (i = firstfd; i <= lastfd; ++i)
+ if (d[i].type)
+ {
+ int type = d[i].type;
+ d[i].reply_port = __mach_reply_port ();
+ err = __io_select (d[i].io_port, d[i].reply_port, 0, &type);
+ switch (err)
+ {
+ case MACH_RCV_TIMED_OUT:
+ /* No immediate response. This is normal. */
+ err = 0;
+ if (firstfd == lastfd)
+ /* When there's a single descriptor, we don't need a
+ portset, so just pretend we have one, but really
+ use the single reply port. */
+ *portset = d[i].reply_port;
+ else if (got == 0)
+ /* We've got multiple reply ports, so we need a port set to
+ multiplex them. */
+ {
+ /* We will wait again for a reply later. */
+ if (*portset == MACH_PORT_NULL)
+ /* Create the portset to receive all the replies on. */
+ err = __mach_port_allocate (__mach_task_self (),
+ MACH_PORT_RIGHT_PORT_SET,
+ portset);
+ if (! err)
+ /* Put this reply port in the port set. */
+ __mach_port_move_member (__mach_task_self (),
+ d[i].reply_port, *portset);
+ }
+ break;
+
+ default:
+ /* No other 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. */
+ type = SELECT_ALL;
+ /* FALLTHROUGH */
+
+ case 0:
+ /* We got an answer. */
+ if ((type & SELECT_ALL) == 0)
+ /* Bogus answer; treat like an error, as a fake positive. */
+ type = SELECT_ALL;
+
+ /* This port is already ready already. */
+ d[i].type &= type;
+ d[i].type |= SELECT_RETURNED;
+ ++got;
+ break;
+ } /* switch (err) */
+ _hurd_port_free (&d[i].cell->port, &d[i].ulink, d[i].io_port);
+ } /* if (d[i].type) */
+
+ if (err)
+ {
+ errno = err;
+ got = -1;
+ }
+ return got;
+} /* _io_select_request() */
+
int _wait_for_replies (int nfds, int firstfd, int lastfd, mach_msg_timeout_t to,
error_t err, mach_port_t portset, struct dfd *d,
const struct timespec *timeout, const sigset_t *sigmask,
@@ -200,10 +270,9 @@ int _wait_for_replies (int nfds, int fir
if (sigmask)
__sigprocmask (SIG_SETMASK, oset, NULL);
errno = err;
- return -1;
+ got = -1;
}
- else
- return got;
+ return got;
} /* _wait_for_replies() */
/* Check the first NFDS descriptors either in POLLFDS (if nonnnull) or in
@@ -402,65 +471,10 @@ _hurd_select (int nfds,
We are just a pure timeout. */
portset = __mach_reply_port ();
else /* POLL || SELECT */
- {
- portset = MACH_PORT_NULL;
-
- for (i = firstfd; i <= lastfd; ++i)
- if (d[i].type)
- {
- int type = d[i].type;
- d[i].reply_port = __mach_reply_port ();
- err = __io_select (d[i].io_port, d[i].reply_port, 0, &type);
- switch (err)
- {
- case MACH_RCV_TIMED_OUT:
- /* No immediate response. This is normal. */
- err = 0;
- if (firstfd == lastfd)
- /* When there's a single descriptor, we don't need a
- portset, so just pretend we have one, but really
- use the single reply port. */
- portset = d[i].reply_port;
- else if (got == 0)
- /* We've got multiple reply ports, so we need a port set to
- multiplex them. */
- {
- /* We will wait again for a reply later. */
- if (portset == MACH_PORT_NULL)
- /* Create the portset to receive all the replies on. */
- err = __mach_port_allocate (__mach_task_self (),
- MACH_PORT_RIGHT_PORT_SET,
- &portset);
- if (! err)
- /* Put this reply port in the port set. */
- __mach_port_move_member (__mach_task_self (),
- d[i].reply_port, portset);
- }
- break;
-
- default:
- /* No other 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. */
- type = SELECT_ALL;
- /* FALLTHROUGH */
-
- case 0:
- /* We got an answer. */
- if ((type & SELECT_ALL) == 0)
- /* Bogus answer; treat like an error, as a fake positive. */
- type = SELECT_ALL;
-
- /* This port is already ready already. */
- d[i].type &= type;
- d[i].type |= SELECT_RETURNED;
- ++got;
- break;
- }
- _hurd_port_free (&d[i].cell->port, &d[i].ulink, d[i].io_port);
- }
- } /* 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,
