[ruby-core:35992] Re: [Ruby 1.9 - Feature #4531] [PATCH 0/7] use poll() instead of select() in certain cases

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.



@@ -2723,7 +2736,14 @@ rb_wait_for_single_fd(int fd, int events,
struct timeval *tv)
     struct pollfd fds;
     int result, lerrno;
     double start;
-    int timeout = tv ? tv->tv_sec * 1000 + (tv->tv_usec + 500) / 1000 : -1;
+    int timeout = -1;
+
+    if (tv) {
+       timeout = timeval2msec(tv);
+       if (timeout == -1) {
+           return -1;
+       }
+    }

No good solusion. backend change should be transparent from caller.
I recommend following two way.

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).


     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?


2) rb_wait_for_single_fd: have poll()-using impl set EBADF
is good fix. but a testcase of it introduce ruby level new method.
It's unacceptable. I'll commit only code fix.