On Fri, Jun 27, 2025 at 01:19:18PM -0800, Junio C Hamano wrote:

Carlo Marcelo Arenas Belón [off-list ref] writes:

quoted

On Fri, Jun 27, 2025 at 09:38:47AM -0800, Phillip Wood wrote:

quoted

On 26/06/2025 18:21, Carlo Marcelo Arenas Belón wrote:

quoted

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