Hi

2011/5/4 Eric Wong <normalperson / yhbt.net>:
> 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)
>>  ¨Ââßäåæéîåßãïîóô¨òâßãÏâêåãô¬ ¢ÒÂß×ÁÉÔÆÄßÉ΢ÉÎÔ²ÎÕͨÒÂß×ÁÉÔÆÄßÉΩ©>>  ¨Ââßäåæéîåßãïîóô¨òâßãÏâêåãô¬ ¢ÒÂß×ÁÉÔÆÄßÏÕÔ¢¬ ÉÎÔ²ÎÕͨÒÂß×ÁÉÔÆÄßÏÕÔ©©»
>>  ¨Ââßäåæéîåßãïîóô¨òâßãÏâêåãô¬ ¢ÒÂß×ÁÉÔÆÄßÐÒÉ¢¬ ÉÎÔ²ÎÕͨÒÂß×ÁÉÔÆÄßÐÒÉ©©»
>> +  ¨Ââßäåæéîåßãïîóô¨òâßãÏâêåãô¬ ¢ÉÎÔßÍÁØ¢¬ ÉÎÔ²ÎÕͨÉÎÔßÍÁØ©©»
>>  ¨Ââßäåæéîåßóéîçìåôïîßíåôèïä¨òâßãÉÏ¢÷áéôßæïòßóéîçìåßæä¢>> wait_for_single_fd, 3);
>>
>> Strongly disagree. Any language change should be passed matz review.
>
> Huh?  ¨Âøô¯­ôåóô­¯éó ïîìù ìïáäåä äõòéîç ôåóôáîîåöåéîóôáììåä®
> No users see anything in ext/-test-/*

Oops, my bad. I misunderstood your diff. ok, I'll commit it.

>
>> 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.  ¨Â§ì÷ïòë ïî ôèáô
> later or tomorrow.

ok.

>
>>  ¨Â¨òåóõì°© >> - /* 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
>> +   ¨Âåñõåóôåä åöåîôó éî ôèòåóõìô áîæïòãôèãáììåò ôï íáëå áî
>> +   ¨Âøôòá óùóãáì¨å®çòåáä¯÷òéôå¯óåîä¯òåãöôï çåôèåòòïò®
>> + /
>> 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.

Yes.

>
> 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.
>
>  ¨Âîô åöåîôó ÒÂß×ÁÉÔÆÄßÉÎ ÒÂß×ÁÉÔÆÄßÏÕÔ»
>  ¨Âîô òåöåîôòâß÷áéôßæïòßóéîçìåßæä¨æäåöåîôóÎÕÌÌ©»
> * poll() itself may return POLLERR, but we prevent it from being in
> * revents since select() can't return that */
>  ¨Â¨òåöåîôó ÒÂß×ÁÉÔÆÄßÉÎ> * since we don't know POLLERR, we fall back to fail here */
>  ¨Â¨òåáä¨æä®®®© °©
>  ¨Ââßóùóßæáé쨰©»
>  ¨Â
>  ¨Â¨òåöåîôó ÒÂß×ÁÉÔÆÄßÏÕÔ© > * since we don't know POLLERR, we fall back to fail here */
>  ¨Â¨÷òéôå¨æä¬ ®®®© °©
>  ¨Ââßóùóßæáé쨰©»
>  ¨Â
> * user code shouldn't care about anything else since it only
> * requested RB_WAITFD_IN|RB_WAITFD_OUT */

Then, correct way is

/* copyed from linux */
#define POLLIN_SET (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR)
#define POLLOUT_SET (POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR)
#define POLLEX_SET (POLLPRI)

int revent_filter(int revents)
{
  int ret = 0;

  if (revents & POLLIN_SET)
        ret |= RB_WAITFD_IN;
  if (revents & POLLOUT_SET)
        ret |= RB_WAITFD_OUT;
  if (revents & POLLEX_SET)
        ret |= RB_WAITFD_PRI;
}

this code don't make false positive. I'll commit it.