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

From: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Date: May 24, 2022, 20:16 UTC
Message-ID: <20220524201639.2gucdkzponddk5qt@carlos-mbp.lan>
In-Reply-To: <xmqqleuqj1gy.fsf@gitster.g>

On Tue, May 24, 2022 at 10:15:57AM -0700, Junio C Hamano wrote:

Show 6 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?

Assuming that some overly clever compiler might optimize that out (either because it might think it is Undefined Behaviour or for other unknown reasons) then Ævar's version would be better for clearing the "warning".

But your patch fixed the "bug" that a probably overeager compiler was "detecting".

> What did I miss?  I must be missing something, as I can explain how
> the current "(*slot->finished) = 1" with "while (finished)"
> correctly works, but I cannot quite explain why the original "while
> (slot->in_use)" would not, which is annoying.

My guess is that there is a curl version somewhere that is patched to use threads more extensible than upstream and where this code is stil needed. I think it is also safe to assume (like you did) that this is a 16 year bug that was already fixed and reverting that code would be an alternative too.

Carlo