Junio C Hamano <gitster@xxxxxxxxx> writes:
> Derrick Stolee <stolee@xxxxxxxxx> writes:
> ...
>> This trace2_region_leave() needs to be conditional on the progress
>> being non-null. The pattern used by consumers of the progress API is:
>>
>> if (my_progress_condition)
>> start_progress(&progress);
>>
>> do {
>> display_progress(&progress, count);
>> } while (condition);
>>
>> stop_progress(&progress);
>>
>> The condition to show progress or not is conditional on an option that
>> is external to the progress API.
>>
>> The fix for this patch is simple: make the trace2_region_leave() be
>> conditional on "p_progress && *p_progress".
Oops. That means that we need to apply the fix before -rc1 to
'master' X-<.
Something like this?
-- >8 --
Subject: progress: call trace2_region_leave() only after calling _enter()
From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
A user of progress API calls start_progress() conditionally and
depends on the display_progress() and stop_progress() functions to
become no-op when start_progress() hasn't been called.
As we added a call to trace2_region_enter() to start_progress(), the
calls to other trace2 API calls from the progress API functions must
make sure that these trace2 calls are skipped when start_progress()
hasn't been called on the progress struct. Specifically, do not
call trace2_region_leave() from stop_progress() when we haven't
called start_progress(), which would have called the matching
trace2_region_enter().
Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
progress.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/progress.c b/progress.c
index 6d2dcff0b6..3eda914518 100644
--- a/progress.c
+++ b/progress.c
@@ -329,13 +329,9 @@ void stop_progress(struct progress **p_progress)
trace2_data_intmax("progress", the_repository,
"total_bytes",
(*p_progress)->throughput->curr_total);
- }
- trace2_region_leave("progress",
- p_progress && *p_progress
- ? (*p_progress)->title
- : NULL,
- the_repository);
+ trace2_region_leave("progress", (*p_progress)->title, the_repository);
+ }
stop_progress_msg(p_progress, _("done"));
}
