apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: apr_poll* changes
Date Tue, 05 Oct 2004 00:00:00 GMT
On Mon, Oct 04, 2004 at 01:34:29PM -0700, Paul Querna wrote:
> Attached is a patch that combines my previous patches:
> - Adds an  APR_POLLSET_THREADSAFE Flag. When this flag is passed, there 
> is thread safety between _add() and _remove().
> - Uses APR Rings to store Pollfds in the Pollsets for KQueue and EPoll 
> back ends.
> - Splits the poll and  pollset implementation  into one file for  each 
> type (epoll, kqueue, poll, and select).

Thanks for persevering with this.  Some review below - which of the
implementations have you tested with this?

Quite a lot of unnecessary whitespace changes, e.g:

>      if (event & APR_POLLIN)
> -        rv |= POLLIN;        
> +        rv |= POLLIN;
>      if (event & APR_POLLPRI)

And s/ * / */g in function prototypes:

> -APR_DECLARE(apr_status_t) apr_poll(apr_pollfd_t *aprset, apr_int32_t num,
> -                      apr_int32_t *nsds, apr_interval_time_t timeout)
> +APR_DECLARE(apr_status_t) apr_poll(apr_pollfd_t * aprset, apr_int32_t num,
> +                                   apr_int32_t * nsds,
> +                                   apr_interval_time_t timeout)

Lack of error handling and s,if(,if (,g

> +#define pollset_lock_rings() \
> +    if(pollset->flags & APR_POLLSET_THREADSAFE) \
> +        apr_thread_mutex_lock(pollset->ring_lock);
> +#define pollset_unlock_rings() \
> +    if(pollset->flags & APR_POLLSET_THREADSAFE) \
> +        apr_thread_mutex_unlock(pollset->ring_lock);

Would be good to apr_ prefix this structure to avoid any possible
conflicts:

> +typedef struct pfd_elem_t pfd_elem_t;
> +
> +struct pfd_elem_t {
> +    APR_RING_ENTRY(pfd_elem_t) link;
> +    apr_pollfd_t pfd;
> +};


Mime
View raw message