Re: [PATCH v7 6/7] pack-bitmap-write.c: don't return without stop_progress()

From
Johannes Altmanninger <hidden>
Date
2021-12-27 01:11:53
Reply to
Ævar Arnfjörð Bjarmason, 2021-12-17
On Fri, Dec 17, 2021 at 05:25:01AM +0100, Ævar Arnfjörð Bjarmason wrote:
Fix a bug that's been here since 7cc8f971085 (pack-objects: implement
bitmap writing, 2013-12-21), we did not call stop_progress() if we
reached the early exit in this function.

We could call stop_progress() before we return, but better yet is to
defer calling start_progress() until we need it.
Looks great.
This will matter in a subsequent commit where we BUG(...) out if this happens,
"this" = "call start without stop"? Let's maybe be explicit here

(I guess the BUG() commit was postponed, but it doesn't hurt to mention it.)
and matters now e.g. because we don't have a corresponding
"region_end" for the progress trace2 event.
s/region_end/region_leave/
quoted hunk ↗ jump to hunk
Suggested-by: SZEDER Gábor <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 pack-bitmap-write.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 9c55c1531e1..cab3eaa2acd 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -575,15 +575,15 @@ void bitmap_writer_select_commits(struct commit **indexed_commits,
 
 	QSORT(indexed_commits, indexed_commits_nr, date_compare);
 
-	if (writer.show_progress)
-		writer.progress = start_progress("Selecting bitmap commits", 0);
-
 	if (indexed_commits_nr < 100) {
 		for (i = 0; i < indexed_commits_nr; ++i)
 			push_bitmapped_commit(indexed_commits[i]);
 		return;
 	}
 
+	if (writer.show_progress)
+		writer.progress = start_progress("Selecting bitmap commits", 0);
+
 	for (;;) {
 		struct commit *chosen = NULL;
 
-- 
2.34.1.1119.g7a3fc8778ee
Thread (15 messages) 15 messages, 2 authors, last reply 2021-12-27
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help