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

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2022, 22:28 UTC
Message-ID: <xmqqleuqefa4.fsf@gitster.g>
In-Reply-To: <220524.86r14ivewt.gmgdl@evledraar.gmail.com>

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

Show 10 quoted lines

>> 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.