httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Eissing <stefan.eiss...@greenbytes.de>
Subject Re: Proposed change to mpm_register_socket_callback(): apr_socket_t -> apr_pollfd_t
Date Mon, 14 Mar 2016 09:13:34 GMT

> Am 14.03.2016 um 09:32 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
> 
> 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.

Like the direction this is going as well.

Do we need a MPM Query for detecting support before one actually has a handle
for registration?

> 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