Re: [PATCH] http API: fix dangling pointer issue noted by GCC 12.0

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> If the motivation behind the original "do not get fooled by a reused
>> slot still working on somebody else's request---instead return when
>> our request is done" was indeed what I speculated, then the pointer
>> slot->finished when we leave this function should not matter to
>> anybody.  Would the following patch make the compiler realize that
>> we never smuggle a local variable's address out of this function via
>> a pointer in the structure?
>>
>>  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)
>
> Yes, that does quiet it. The GCC warning is specifically about pointers
> that survive the exit of the function.

Thanks for a quick testing.

The real question is if this breaks anything.  By definition, if
this changes the behaviour, it is an indication that the code has
been somehow depending on having a pointer to a stackframe that has
already gone out of scope, which is a more serious problem.  I am
wondering if we need to dig further to find it out, and if so, how.

In any case, it looks like a more correct fix to address the "early
GCC 12" problem, than reverting baa7b67d (HTTP slot reuse fixes,
2006-03-10), at least to me.

Unless we devise another way to address the "slot reuse" issue, or
we come up with an explanation that the "slot reuse" issue is no
longer possible in todays code, that is.

Thanks.