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

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

On Mon, Jun 07 2021, Derrick Stolee wrote:

quoted

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.

That's right. I misremembered where this trace2 stuff was.

The idea we didn't pursue was to create the progress struct
unconditionally, and just leave it as "quiet" based on an
input parameter. That would keep the trace2 regions consistent.

But I'm wrong and there is no behavior change here.

Thanks,
-Stolee