On 6/7/2021 11:52 AM, Ævar Arnfjörð Bjarmason wrote: > > 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. 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
