Re: [PATCH 1/2] read-cache.c: don't guard calls to progress.c API

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Jun 07 2021, Derrick Stolee wrote:

> On 6/7/2021 10:43 AM, Ævar Arnfjörð Bjarmason wrote:
>> Don't guard the calls to the progress.c API with "if (progress)". The
>> API itself will check this. This doesn't change any behavior, but
>> makes this code consistent with the rest of the codebase.
>
> Since stop_progress() closes a trace2 region, this actually
> does make a change in behavior, I think. In a good way.

I don't see the behavior change.

Yes start_delayed_progress() will call start_progress_delay() which
mallocs and enters the trace2 region, and then if you don't call
stop_progress() at all you won't leave it.

But in read-cache.c both before & after my change we only malloc & only
enter the region if we're actually displaying the progress, there's an
isatty() guard on it.

Once we start that progress bar we will leave the trace2 region, via
stop_progress(), but note that stop_progress() will exit early if the
pointer you dereference is NULL and not do that, and since we'll have
"*progress = NULL" in the case of not wanting the progress bar we won't
leave the region we didn't enter in the first place.

The change here is just to remove the needless nano-optimization of
guarding the calls with a NULL check of "progress", which the API itself
does, no?