Jeff King <peff@xxxxxxxx> writes:
> On Wed, Feb 17, 2021 at 09:48:44PM +0000, Jeff Hostetler via GitGitGadget wrote:
>
>> @@ -106,7 +108,10 @@ int unix_stream_listen(const char *path)
>> if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
>> goto fail;
>>
>> - if (listen(fd, 5) < 0)
>> + backlog = opts->listen_backlog_size;
>> + if (backlog <= 0)
>> + backlog = DEFAULT_UNIX_STREAM_LISTEN_BACKLOG;
>> + if (listen(fd, backlog) < 0)
>> goto fail;
Luckily there is no "pass 0 and the platforms will choose an
appropriate backlog value", so "pass 0 to get the default Git
chooses" is OK, but do we even want to allow passing any negative
value? Shouldn't it be diagnosed as an error instead?
> OK, so we still have the fallback-on-zero here, which is good...
>
>> +struct unix_stream_listen_opts {
>> + int listen_backlog_size;
>> +};
>> +
>> +#define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
>> +
>> +#define UNIX_STREAM_LISTEN_OPTS_INIT \
>> +{ \
>> + .listen_backlog_size = DEFAULT_UNIX_STREAM_LISTEN_BACKLOG, \
>> +}
>
> ...but I thought the plan was to drop this initialization in favor of a
> zero-initialization. What you have certainly wouldn't do the wrong
> thing, but it just seems weirdly redundant. Unless some caller really
> wants to know what the default will be?
Very true. The code knows the exact value input 0 has to fall back
to; we shouldn't have to initialize to that same exact value and I
do not offhand see why the DEFAULT_UNIX_STREAM_LISTEN_BACKLOG needs
to be a public constant.
Thanks.
