On 11/1/22 7:14 PM, Matthew John Cheetham wrote:
> On 2022-10-28 12:14, Jeff Hostetler wrote:
>> On 10/28/22 11:08 AM, Derrick Stolee wrote:
>>>> +static void kill_some_child(void)
>>>
>>>> +static void check_dead_children(void)
>>>
>>> These technically sound methods have unfortunate names.
>>> Using something like "connection" over "child" might
>>> alleviate some of the horror. (I initially wanted to
>>> suggest "subprocess" but you compare live_children to
>>> max_connections in the next method, so connection seemed
>>> appropriate.)
>>
>> These names were inherited from `daemon.c` IIRC. I wouldn't change
>> them since it'll just introduce noise when diffing. Especially,
>> if we do the copy commit first.
>
> Indeed. These functions are untouched from daemon.c. I do plan to split
> this mega-patch up however in to a single 'add the boilerplate' based on
> git-daemon patch, then add the extra pieces like HTTP request parsing and
> the auth pieces in a v3.
If these are copied from daemon.c, it may be worth trying
to lib-ify these data structures and code so they can be
shared across the two places. That can also come up as a
cleanup later, too.
For now, don't bother changing the names since they exist
somewhere else.
>> [...]
>>>> +static struct strvec cld_argv = STRVEC_INIT;
>>>> +static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
>>>> +{
>>>> + struct child_process cld = CHILD_PROCESS_INIT;
>>>> +
>>>> + if (max_connections && live_children >= max_connections) {
>>>> + kill_some_child();
>>>> + sleep(1); /* give it some time to die */
>>>> + check_dead_children();
>>>> + if (live_children >= max_connections) {
>>>> + close(incoming);
>>>> + logerror("Too many children, dropping connection");
>>>> + return;
>>>> + }
>>>> + }
>>>
>>> Do we anticipate exercising concurrent requests in our
>>> tests? Perhaps it's not worth putting a cap on the
>>> connection count so we can keep the test helpers simple.
>>
>> again, this code was inherited from `daemon.c`, so we could leave it.
I wonder how much could be extracted from daemon.c using a
copy into a 'daemon-lib.c' with methods defined in 'daemon-lib.h'
then consumed from this file instead. Not sure it's worth the
churn to daemon.c, though.
Thanks,
-Stolee
