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

On Mon, Jun 07 2021, Derrick Stolee wrote:

On 6/7/2021 10:43 AM, Ævar Arnfjörð Bjarmason wrote:

quoted

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?