Re: [PATCH] http.c: clear the 'finished' member once we are done with it

["Carlo Marcelo Arenas Belón" <carenas@gmail.com>]

On Fri, May 06, 2022 at 02:17:01PM -0700, Junio C Hamano wrote:
> diff --git a/http.c b/http.c
> index 229da4d148..85437b1980 100644
> --- a/http.c
> +++ b/http.c
> @@ -1367,6 +1367,9 @@ void run_active_slot(struct active_request_slot *slot)
>  			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
>  		}
>  	}
> +
> +	if (slot->finished == &finished)
> +		slot->finished = NULL;

I am not completely sure yet (since I looked at it long ago and got
sidetracked) but I think this might be optimized out (at least by gcc12)
since it is technically UB, which is why it never "fixed" the warning.

the "correct" way to implement this would be to make "finished" a thread
local static, which is finally one good reason to support C99, but the
syntax to do so with Windows broke my first attempt at doing so and now
I can even find the code I used then which required a per platform macro
and was better looking than the following

Carlo
--- >8 ----
Date: Wed, 20 Apr 2022 23:25:55 -0700
Subject: [PATCH] http: avoid using out of scope pointers

baa7b67d091 (HTTP slot reuse fixes, 2006-03-10) introduces a way
to notify a curl thread that its slot is finished by using a pointer
to a stack variable from run_active_slot(), but then gcc 12 was
released and started rightfully to complain about it (-Wdangling-pointer).

Use instead a thread storage static variable which is safe to use
between threads since C99 and doesn't go out of scope, while being
functionally equivalent to the original code, and also remove the
workaround from 9c539d1027d (config.mak.dev: alternative workaround
to gcc 12 warning in http.c, 2022-04-15)

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 config.mak.dev | 1 -
 http.c         | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/config.mak.dev b/config.mak.dev
index c3104f400b..335efd4620 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -68,7 +68,6 @@ endif
 # https://bugzilla.redhat.com/show_bug.cgi?id=2075786
 ifneq ($(filter gcc12,$(COMPILER_FEATURES)),)
 DEVELOPER_CFLAGS += -Wno-error=stringop-overread
-DEVELOPER_CFLAGS += -Wno-error=dangling-pointer
 endif
 
 GIT_TEST_PERL_FATAL_WARNINGS = YesPlease
diff --git a/http.c b/http.c
index 229da4d148..cb9acfca19 100644
--- a/http.c
+++ b/http.c
@@ -1327,7 +1327,7 @@ void run_active_slot(struct active_request_slot *slot)
 	fd_set excfds;
 	int max_fd;
 	struct timeval select_timeout;
-	int finished = 0;
+	static __thread int finished;
 
 	slot->finished = &finished;
 	while (!finished) {
-- 
2.36.0.352.g0cd7feaf86f