Re: [PATCH v2] daemon: correctly handle soft accept() errors in service_loop

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 27, 2025, 20:19 UTC
Message-ID: <xmqq34bl7xa1.fsf@gitster.g>
In-Reply-To: <u4scxaxivz62fsljo7okkfdvcle3zdby6h2sdzd6ed5n6wi5xb@5ekxdycixwxe>

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

Show 30 quoted lines

> 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) {
			...