apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mladen Turk <mt...@apache.org>
Subject Re: svn commit: r744095 [1/2] - in /apr/apr/trunk: ./ include/ include/arch/unix/ poll/os2/ poll/unix/
Date Sat, 14 Feb 2009 17:53:16 GMT
Ruediger Pluem wrote:
> 
>> +    impl_pollset_cleanup,
>> +    "epool"
> 
> Guess this should be epoll instead of epool above :-).
>

Right :)

>> +static apr_status_t impl_pollcb_remove(apr_pollcb_t *pollcb,
>> +                                       apr_pollfd_t *descriptor)
>>  {
>> -    return APR_ENOTIMPL;
>> +    int fd;
>> +    apr_uint32_t i;
>> +
>> +    if (descriptor->desc_type == APR_POLL_SOCKET)
>> +        fd = descriptor->desc.s->socketdes;
>> +    else
>> +        fd = descriptor->desc.f->filedes;
>> +
>> +    for (i = 0; i < pollcb->nelts; i++) {
>> +        if (descriptor->desc.s == pollcb->copyset[i]->desc.s) {
>> +            /* Found an instance of the fd: remove this and any other copies */
> 
> I don't grok this algorithm. Why do we determine the fd above and work only with
> socket fd's in the loop? We should either compare the descriptors itself or if
> this is unreliable we need to check the fd against the correct counterpart.
>

With pollcb checking descriptors is by design reliable.
I've removed the dormant code.

>> +    pollcb->pollset.port = apr_palloc(p, size * sizeof(port_event_t));
>> +    apr_pool_cleanup_register(p, pollcb, cb_cleanup, cb_cleanup);
> 
> Why don't you save fd to pollcb->fd?
>

Makes sense, I'll do that.


Thanks
-- 
^(TM)

Mime
View raw message