On Fri, Jun 27, 2025 at 01:19:18PM -0800, Junio C Hamano wrote:
> Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> 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
