Re: [PATCH 2/2] config.mak.dev: alternative workaround to gcc 12 warning in http.c

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 15, 2022, 23:34 UTC
Message-ID: <xmqqh76taplg.fsf@gitster.g>
In-Reply-To: <20220415231342.35980-3-carenas@gmail.com>

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

Show 21 quoted lines

> This provides a "no code change needed" option to the "fix" currently
> queued as part of ab/http-gcc-12-workaround and therefore should be
> reverted once that gets merged.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  config.mak.dev | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index 335efd46203..c3104f400b2 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -68,6 +68,7 @@ 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

Hmph, this might be an acceptable workaround to squelch the compiler that complains "you stored an onstack pointer in a structure and then you are leaving the scope". We should do something more like the attached patch, with or without the gcc warning, I think. We may have smuggled the pointer to finished in slot->finished pointer that survives the current stackframe out of the function in the original code, so that is what we rectify by clearing the member when it has the value we stored.

 http.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git c/http.c w/http.c
index 229da4d148..85437b1980 100644
--- c/http.c
+++ w/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;
 }
 
 static void release_active_slot(struct active_request_slot *slot)