apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@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 10:47:48 GMT


On 02/13/2009 01:24 PM, mturk@apache.org wrote:
> Author: mturk
> Date: Fri Feb 13 12:24:00 2009
> New Revision: 744095
> 
> URL: http://svn.apache.org/viewvc?rev=744095&view=rev
> Log:
> Implement providers for apr_pollset and apr_pollcb
> 
> Modified:
>     apr/apr/trunk/CHANGES
>     apr/apr/trunk/NWGNUmakefile
>     apr/apr/trunk/apr.dsp
>     apr/apr/trunk/include/apr_poll.h
>     apr/apr/trunk/include/arch/unix/apr_arch_poll_private.h
>     apr/apr/trunk/libapr.dsp
>     apr/apr/trunk/poll/os2/pollset.c
>     apr/apr/trunk/poll/unix/epoll.c
>     apr/apr/trunk/poll/unix/kqueue.c
>     apr/apr/trunk/poll/unix/poll.c
>     apr/apr/trunk/poll/unix/port.c
>     apr/apr/trunk/poll/unix/select.c
> 

> Modified: apr/apr/trunk/poll/unix/epoll.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/epoll.c?rev=744095&r1=744094&r2=744095&view=diff
> ==============================================================================
> --- apr/apr/trunk/poll/unix/epoll.c (original)
> +++ apr/apr/trunk/poll/unix/epoll.c Fri Feb 13 12:24:00 2009

> @@ -384,54 +318,46 @@
>      return rv;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollset_wakeup(apr_pollset_t *pollset)
> -{
> -    if (pollset->flags & APR_POLLSET_WAKEABLE)
> -        return apr_file_putc(1, pollset->wakeup_pipe[1]);
> -    else
> -        return APR_EINIT;
> -}
> -
> -struct apr_pollcb_t {
> -    apr_pool_t *pool;
> -    apr_uint32_t nalloc;
> -    struct epoll_event *pollset;
> -    int epoll_fd;
> +static apr_pollset_provider_t impl = {
> +    impl_pollset_create,
> +    impl_pollset_add,
> +    impl_pollset_remove,
> +    impl_pollset_poll,
> +    impl_pollset_cleanup,
> +    "epool"

Guess this should be epoll instead of epool above :-).

>  };
>  

> Modified: apr/apr/trunk/poll/unix/poll.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/poll.c?rev=744095&r1=744094&r2=744095&view=diff
> ==============================================================================
> --- apr/apr/trunk/poll/unix/poll.c (original)
> +++ apr/apr/trunk/poll/unix/poll.c Fri Feb 13 12:24:00 2009
             }
> @@ -359,45 +270,134 @@
>              rv = APR_SUCCESS;
>      }
>      if (descriptors && (*num))
> -        *descriptors = pollset->result_set;
> +        *descriptors = pollset->p->result_set;
>      return rv;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollset_wakeup(apr_pollset_t *pollset)
> -{
> -    if (pollset->flags & APR_POLLSET_WAKEABLE)
> -        return apr_file_putc(1, pollset->wakeup_pipe[1]);
> -    else
> -        return APR_EINIT;
> +static apr_pollset_provider_t impl = {
> +    impl_pollset_create,
> +    impl_pollset_add,
> +    impl_pollset_remove,
> +    impl_pollset_poll,
> +    NULL,
> +    "pool"

Guess this should be poll instead of pool above :-).

> +};
> +
> +apr_pollset_provider_t *apr_pollset_provider_poll = &impl;
> +
> +/* Poll method pollcb.
> + * This is probably usable only for WIN32 having WSAPoll
> + */
> +static apr_status_t impl_pollcb_create(apr_pollcb_t *pollcb,
> +                                       apr_uint32_t size,
> +                                       apr_pool_t *p,
> +                                       apr_uint32_t flags)
> +{
> +    pollcb->fd = -1;
> +    pollcb->pollset.ps = apr_palloc(p, size * sizeof(struct pollfd));
> +    pollcb->copyset = apr_palloc(p, size * sizeof(apr_pollfd_t *));
> +
> +    return APR_SUCCESS;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollcb_create(apr_pollcb_t **pollcb,
> -                                            apr_uint32_t size,
> -                                            apr_pool_t *p,
> -                                            apr_uint32_t flags)
> +static apr_status_t impl_pollcb_add(apr_pollcb_t *pollcb,
> +                                    apr_pollfd_t *descriptor)
>  {
> -    return APR_ENOTIMPL;
> +    if (pollcb->nelts == pollcb->nalloc) {
> +        return APR_ENOMEM;
> +    }
> +
> +    if (descriptor->desc_type == APR_POLL_SOCKET) {
> +        pollcb->pollset.ps[pollcb->nelts].fd = descriptor->desc.s->socketdes;
> +    }
> +    else {
> +        pollcb->pollset.ps[pollcb->nelts].fd = descriptor->desc.f->filedes;
> +    }
> +
> +    pollcb->pollset.ps[pollcb->nelts].events =
> +        get_event(descriptor->reqevents);
> +    pollcb->copyset[pollcb->nelts] = descriptor;
> +    pollcb->nelts++;
> +    
> +    return APR_SUCCESS;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollcb_add(apr_pollcb_t *pollcb,
> -                                         apr_pollfd_t *descriptor)
> +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.

> +            apr_uint32_t dst = i;
> +            apr_uint32_t old_nelts = pollcb->nelts;
> +            pollcb->nelts--;
> +            for (i++; i < old_nelts; i++) {
> +                if (descriptor->desc.s == pollcb->copyset[i]->desc.s) {
> +                    pollcb->nelts--;
> +                }
> +                else {
> +                    pollcb->pollset.ps[dst] = pollcb->pollset.ps[i];
> +                    pollcb->copyset[dst] = pollcb->copyset[i];
> +                    dst++;
> +                }
> +            }
> +            return APR_SUCCESS;
> +        }
> +    }
> +
> +    return APR_NOTFOUND;
>  }
>  

> Modified: apr/apr/trunk/poll/unix/port.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/poll/unix/port.c?rev=744095&r1=744094&r2=744095&view=diff
> ==============================================================================
> --- apr/apr/trunk/poll/unix/port.c (original)
> +++ apr/apr/trunk/poll/unix/port.c Fri Feb 13 12:24:00 2009

> @@ -431,61 +363,52 @@
>      pollset_lock_rings();
>  
>      /* Shift all PFDs in the Dead Ring to be Free Ring */
> -    APR_RING_CONCAT(&(pollset->free_ring), &(pollset->dead_ring), pfd_elem_t,
link);
> +    APR_RING_CONCAT(&(pollset->p->free_ring), &(pollset->p->dead_ring),
pfd_elem_t, link);
>  
>      pollset_unlock_rings();
>  
>      return rv;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollset_wakeup(apr_pollset_t *pollset)
> -{
> -    if (pollset->flags & APR_POLLSET_WAKEABLE)
> -        return apr_file_putc(1, pollset->wakeup_pipe[1]);
> -    else
> -        return APR_EINIT;
> -}
> -
> -struct apr_pollcb_t {
> -    apr_pool_t *pool;
> -    apr_uint32_t nalloc;
> -    port_event_t *port_set;
> -    int port_fd;
> +static apr_pollset_provider_t impl = {
> +    impl_pollset_create,
> +    impl_pollset_add,
> +    impl_pollset_remove,
> +    impl_pollset_poll,
> +    impl_pollset_cleanup,
> +    "port"
>  };
>  
> +apr_pollset_provider_t *apr_pollset_provider_port = &impl;
> +
>  static apr_status_t cb_cleanup(void *p_)
>  {
>      apr_pollcb_t *pollcb = (apr_pollcb_t *) p_;
> -    close(pollcb->port_fd);
> +    close(pollcb->fd);
>      return APR_SUCCESS;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollcb_create(apr_pollcb_t **pollcb,
> -                                            apr_uint32_t size,
> -                                            apr_pool_t *p,
> -                                            apr_uint32_t flags)
> +static apr_status_t impl_pollcb_create(apr_pollcb_t *pollcb,
> +                                       apr_uint32_t size,
> +                                       apr_pool_t *p,
> +                                       apr_uint32_t flags)
>  {
>      int fd;
>  
>      fd = port_create();
>  
>      if (fd < 0) {
> -        *pollcb = NULL;
>          return apr_get_netos_error();
>      }
>  
> -    *pollcb = apr_palloc(p, sizeof(**pollcb));
> -    (*pollcb)->nalloc = size;
> -    (*pollcb)->pool = p;
> -    (*pollcb)->port_fd = fd;
> -    (*pollcb)->port_set = apr_palloc(p, size * sizeof(port_event_t));
> -    apr_pool_cleanup_register(p, *pollcb, cb_cleanup, cb_cleanup);
> +    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?

>  
>      return APR_SUCCESS;
>  }
>  
> -APR_DECLARE(apr_status_t) apr_pollcb_add(apr_pollcb_t *pollcb,
> -                                         apr_pollfd_t *descriptor)
> +static apr_status_t impl_pollcb_add(apr_pollcb_t *pollcb,
> +                                    apr_pollfd_t *descriptor)
>  {
>      int ret, fd;
>  

Regards

RĂ¼diger

Mime
View raw message