httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t
Date Mon, 14 Mar 2016 08:32:29 GMT
On Mon, Mar 14, 2016 at 12:41 AM, Graham Leggett <minfrin@sharp.fm> wrote:
> On 13 Mar 2016, at 10:55 PM, Eric Covener <covener@gmail.com> wrote:
>
>> I also meant the original feature never made it, so we can whatever we
>> want to it.
>
> What do you think of this?

Looks good and indeed very valuable to me, s/socket/pollfd/ is a great idea.

Maybe a little nit-picking below...

>
> Index: include/ap_mpm.h
> ===================================================================
> --- include/ap_mpm.h    (revision 1734657)
> +++ include/ap_mpm.h    (working copy)
> @@ -207,50 +207,48 @@
>                                                         void *baton);
>
>  /**
> - * Register a callback on the readability or writability on a group of sockets
> - * @param s Null-terminated list of sockets
> + * Register a callback on the readability or writability on a group of
> + * sockets/pipes.
> + * @param pds Null-terminated list of apr_pollfd_t
>   * @param p pool for use between registration and callback
> - * @param for_read Whether the sockets are monitored for read or writability
>   * @param cbfn The callback function
>   * @param baton userdata for the callback function
> - * @return APR_SUCCESS if all sockets could be added to a pollset,
> + * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
>   * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
> - * @remark When activity is found on any 1 socket in the list, all are removed
> + * @remark When activity is found on any 1 socket/pipe in the list, all are removed
>   * from the pollset and only 1 callback is issued.
>   */
>
> -AP_DECLARE(apr_status_t) ap_mpm_register_socket_callback(apr_socket_t **s,
> -                                                         apr_pool_t *p,
> -                                                         int for_read,
> -                                                         ap_mpm_callback_fn_t *cbfn,
> -                                                         void *baton);
> +AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(apr_pollfd_t **pds,
> +                                                       apr_pool_t *p,
> +                                                       ap_mpm_callback_fn_t *cbfn,
> +                                                       void *baton);

Since apr_pollfd_t is not opaque (unlike apr_socket_t), maybe we could
remove the indirection here (and in the code below) with somthing like
(apr_pollfd_t *pfds, size_t npfds, ...).
That would allow a single allocation (all pfds in once) and possibly
make things easier for the caller.

Regards,
Yann.

Mime
View raw message