KOSAKI Motohiro <kosaki.motohiro / gmail.com> wrote:
> diff --git a/ext/-test-/wait_for_single_fd/wait_for_single_fd.c
> b/ext/-test-/wait_for_single_fd/wait_for_single_fd.c
> index d406724..6efd1af 100644
> --- a/ext/-test-/wait_for_single_fd/wait_for_single_fd.c
> +++ b/ext/-test-/wait_for_single_fd/wait_for_single_fd.c
> @@ -25,6 +25,7 @@ Init_wait_for_single_fd(void)
>      rb_define_const(rb_cObject, "RB_WAITFD_IN", INT2NUM(RB_WAITFD_IN));
>      rb_define_const(rb_cObject, "RB_WAITFD_OUT", INT2NUM(RB_WAITFD_OUT));
>      rb_define_const(rb_cObject, "RB_WAITFD_PRI", INT2NUM(RB_WAITFD_PRI));
> +    rb_define_const(rb_cObject, "INT_MAX", INT2NUM(INT_MAX));
>      rb_define_singleton_method(rb_cIO, "wait_for_single_fd",
>                                 wait_for_single_fd, 3);
> 
> Strongly disagree. Any language change should be passed matz review.

Huh?  ext/-test-/* is only loaded during tests and never installed.
No users see anything in ext/-test-/*

> 1) use ppoll(2) if available. and use INT_MAX if unavailable. or
> 2) fallback select(2)
> 
> 1) is safe because linux has ppol(2).

OK, good point about ppoll(), I forgot that exists.  I'll work on that
later or tomorrow.

>      if (result > 0) {
> -       /* remain compatible with select(2)-based implementation */
> +       /*
> +        * Remain compatible with the select(2)-based implementation:
> +        * 1) mask out poll()-only revents such as POLLHUP/POLLERR
> +        * 2) In case revents only consists of masked-out events, return all
> +        *    requested events in the result and force the caller to make an
> +        *    extra syscall (e.g. read/write/send/recv) to get the error.
> +        */
>         result = (int)(fds.revents & fds.events);
>         return result == 0 ? events : result;
>      }
> 
> I don't understand this. Why does this behavior help to compatible?
> When do we use it?

We need to ensure rb_wait_for_single_fd(fd, events, timeval) returns
only a subset of its +events+ argument because that's all select() is
capable of.

If poll() returns POLLHUP/POLLERR, we should not expose those flags to
callers of rb_wait_for_single_fd() since it would then behave
differently if poll() or select() were used.

    int events = RB_WAITFD_IN | RB_WAITFD_OUT;
    int revents = rb_wait_for_single_fd(fd, events, NULL);
    /* poll() itself may return POLLERR, but we prevent it from being in
     * revents since select() can't return that */
    if (revents & RB_WAITFD_IN) {
	/* since we don't know POLLERR, we fall back to fail here */
	if (read(fd, ...) < 0)
	    rb_sys_fail(0);
    }
    if (revents & RB_WAITFD_OUT) {
	/* since we don't know POLLERR, we fall back to fail here */
	if (write(fd, ...) < 0)
	    rb_sys_fail(0);
    }
    /* user code shouldn't care about anything else since it only
     * requested RB_WAITFD_IN|RB_WAITFD_OUT */

-- 
Eric Wong