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

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

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

> I.e. the assignment earlier in the function is unconditional, why
> wouldn't the clearing of the data correspond to that assignment and
> clear it unconditionally?

The original problem description that introduced .finished member
indicates that inside the while() loop, the same slot object can be
completed (by feeding it to finish_active_slot(), which would also
clears its in_use thus making it reusable) and then later be reused
(by using it for a different request).

The dispatching is done by calling step_active_slots() repeatedly
inside the loop and I do not think there is any multi-threaded
concurrency to worry about here.  The protection is against a case
where such a slot, which was originally ours and pointed at our
on-stack finished variable with its finished member, is reused for a
different request, and its finished member is used in a similar way
to point at the on-stack finish variable in somebody else's
stackframe in the future code.  If the slot instance we were using
as ours upon the entry of this function is being used for another
request already (the fix that required the .finished member is an
enough explanation that it is a real concern), after we leave the
loop, the slot instance is no longer ours, so we need to be careful
when we clear it.

At the entry of this function, the story is vastly different. The
slot instance belongs to us---the caller chose the slot and decided
to use it to service a particular request and threw the slot
instance at us, so there is nothing wrong to unconditionally use the
.finished member of the slot and point it at a variable in our
stackframe.  But after the loop leaves, and the slot may or may not
be already reused to hold another request.  If .finished is set and
it is the value that points at the variable in our stackframe, then
we are the only one who could have set that and it is safe to clear.
Any other value other than NULL, we do not know at that point who
set it, and it is being used for a request that we have nothing to
do with.  That is why we want to refrain from touching it when it is
not clearly ours.