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 Tue, 15 Mar 2016 11:09:09 GMT
On Mon, Mar 14, 2016 at 11:29 PM, Graham Leggett <minfrin@sharp.fm> wrote:
> On 14 Mar 2016, at 11:17 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote
> :
>>> Having looked at this in more detail this isn’t as simple.
>>>
>>> The sticking point is the cleanup, which needs to be passed a single struct to
do it’s work. To pass both the pfds and the npdfs these two need to be wrapped in a structure
of some kind, which the user has to feed in from the outside common to both register and unregister.
This structure probably should be an apr_array_header_t.
>>
>> Hmm right, either this or, say, the ending pfd is the one with
>> ->reqevents=0, or ->p=NULL, or probably better
>> ->desc_type=APR_NO_DESC.
>> Up to the caller to ensure this, it could possibly be documented.
>>
>>>
>>> Is it worth making the change from apr_pollfd_t **pfds to apr_array_header_t
*pfds?
>>
>> That would still require some "complex" initialization, so I'd prefer
>> the above if it doesn't sound to hacky…
>
> I just gave it a try and it was a lot more elegant than I thought.
>
> You create an array the precise size you need, and off you go.

That's indeed elegant enough, but I think I prefer the plain array
solution, still :p

Don't you like?
{
    apr_pollfd_t pfds[3];
    /* or */
    apr_pollfd_t *pfds = apr_pcalloc(p, 3 * apr_pollfd_t);

    pfds[0].desc_type = APR_POLL_SOCKET;
    pfds[0].desc.s = s0;
    ...
    pfds[1].desc_type = APR_POLL_SOCKET;
    pfds[1].desc.s = s1;
    ...
    pfds[2].desc_type = APR_NO_DESC;

    ap_mpm_register_poll_callback_timeout(pfds, pool, ...);
}

And the callbacks/cleanups could then loop with:
{
    apr_pollfd_t *pfd;
    for (pfd = pfds; pfd->desc_type != APR_NO_DESC; pfd++) {
        ...
    }
}

Since APR_NO_DESC == 0, we could even omit it above (both for
initializing pfds[2] thanks to calloc, and for the != APR_NO_DESC in
the loop)...

No struct needed for cleanup_register() either, we can pass the pfds
(plain) array as is (terminated by .desc_type == APR_NO_DESC instead
of the NULL-pointer of your first/indirect version, we can likewise
require this from the user).

But, after all, I'm not really against using an apr_array either...

Regards,
Yann.

Mime
View raw message