Show 57 quoted lines
> On 28 Jun 2025, at 2:06 AM, Carlo Marcelo Arenas Belón <carenas@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 01:19:18PM -0800, Junio C Hamano wrote:
>> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
>>
>>> On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:
>>>>
>>>>> On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:
>>>>>>
>>>>>> diff --git a/daemon.c b/daemon.c
>>>>>> index d1be61fd57..f113839781 100644
>>>>>> --- a/daemon.c
>>>>>> +++ b/daemon.c
>>>>>> @@ -1145,6 +1145,7 @@ static int service_loop(struct socketlist *socklist)
>>>>>> for (size_t i = 0; i < socklist->nr; i++) {
>>>>>> if (pfd[i].revents & POLLIN) {
>>>>>> + int incoming;
>>>>>> union {
>>>>>> struct sockaddr sa;
>>>>>> struct sockaddr_in sai;
>>>>>> @@ -1153,11 +1154,19 @@ static int service_loop(struct socketlist *socklist)
>>>>>> #endif
>>>>>> } ss;
>>>>>> socklen_t sslen = sizeof(ss);
>>>>>> - int incoming = accept(pfd[i].fd, &ss.sa, &sslen);
>>>>>
>>>>> Why is the declaration of incoming moved but retry is declared here?
>>>
>>> Separating the declaration and assignment for incoming is needed so we can
>>> insert a label for goto; moving it up just removes distractions so the rest
>>> of the logic is clearly in view.
>>>
>>> Obviously that includes the definition and assignment for retry.
>>>
>>> How would you suggest to arrange this better?
>>
>> I think what Phillip meant was more like this, perhaps.
>>
>> socklen_t sslen = sizeof(ss);
>> - int incoming = accept(pfd[i].fd, &ss.sa, &sslen);
>> + int incoming;
>> + int retry = 3;
>> +
>> + incoming = accept(pfd[i].fd, &ss.sa, &sslen);
>> if (incoming < 0) {
>> ...
>
> That seems unnecessarily restrictive just to minimize churn and leaves the
> deflaration of incoming strangely sitting in between two assignments, which
> while it doesn't trigger -Wdeclaration-after-statement seems to go against
> its spirit.
>
> Will include in a v3 with all other suggestions, but frankly think that the
> original was overall cleaner.
>
> Carlo
>