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;
> +};
