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

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

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

>> I _think_ we can even get away by not doing anything to
>> slot->finished at the end of run_active_slot(), as we are not
>> multi-threaded and the callee only returns to the caller, but if it
>> helps pleasing the warning compiler, I'd prefer the simplest
>> workaround, perhaps with an unconditional clearing there?
>
> I'll admit I haven't fully looked into this again, but does anything in
> the subsequent analysis suggest that my original patch wouldn't be a
> working solution to this, still:
> https://lore.kernel.org/git/patch-1.1-1cec367e805-20220126T212921Z-avarab@gmail.com/ ?

I traced _one_ code path as a demonstration to show why the current
"slot->finished = &finished" based solution works.  

But I think what we need is to demonstrate a code path in the old
version that shows why the old slot->in_use would not have worked
and the slot->finished was needed, and demonstrate why it NO LONGER
is the case in today's code.  Without that, especially with the
latter, I cannot take the "just revert 16-year old bugfix because a
new compiler throws a warning related to multi-threaded code to it,
even though we are strictly single-threaded" as a serious solution.

And because I do not think I've seen anybody has done that necessary
digging, I would still prefer the "if the compiler somehow cares,
then let's clear the finished member once we are done with it" much
better than "we do not know why but we somehow think we can do
without this bugfix, even though we wouldn't be making noises about
this piece of code if a new compiler did not start emitting a
warning".

Thanks.