From 07647c92ffabbb639436de8b5634244cbdfd6ef2 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 14 May 2024 15:56:56 -0400 Subject: pack-bitmap: avoid use of static `bitmap_writer` The pack-bitmap machinery uses a structure called 'bitmap_writer' to collect the data necessary to write out .bitmap files. Since its introduction in 7cc8f971085 (pack-objects: implement bitmap writing, 2013-12-21), there has been a single static bitmap_writer structure, which is responsible for all bitmap writing-related operations. In practice, this is OK, since we are only ever writing a single .bitmap file in a single process (e.g., `git multi-pack-index write --bitmap`, `git pack-objects --write-bitmap-index`, `git repack -b`, etc.). However, having a single static variable makes issues like data ownership unclear, when to free variables, what has/hasn't been initialized unclear. Refactor this code to be written in terms of a given bitmap_writer structure instead of relying on a static global. Note that this exposes the structure definition of the bitmap_writer at the pack-bitmap.h level. We could work around this by, e.g., forcing callers to declare their writers as: struct bitmap_writer *writer; bitmap_writer_init(&bitmap_writer); and then declaring `bitmap_writer_init()` as taking in a double-pointer like so: void bitmap_writer_init(struct bitmap_writer **writer); which would avoid us having to expose the definition of the structure itself. This patch takes a different approach, since future patches (like for the ongoing pseudo-merge bitmaps work) will want to modify the innards of this structure (in the previous example, via pseudo-merge.c). Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'builtin/pack-objects.c') diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index baf0090fc8..ba4c93d241 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1245,6 +1245,7 @@ static void write_pack_file(void) uint32_t nr_remaining = nr_result; time_t last_mtime = 0; struct object_entry **write_order; + struct bitmap_writer bitmap_writer; if (progress > pack_to_stdout) progress_state = start_progress(_("Writing objects"), nr_result); @@ -1339,8 +1340,9 @@ static void write_pack_file(void) hash_to_hex(hash)); if (write_bitmap_index) { - bitmap_writer_set_checksum(hash); - bitmap_writer_build_type_index( + bitmap_writer_init(&bitmap_writer); + bitmap_writer_set_checksum(&bitmap_writer, hash); + bitmap_writer_build_type_index(&bitmap_writer, &to_pack, written_list, nr_written); } @@ -1358,11 +1360,16 @@ static void write_pack_file(void) strbuf_addstr(&tmpname, "bitmap"); stop_progress(&progress_state); - bitmap_writer_show_progress(progress); - bitmap_writer_select_commits(indexed_commits, indexed_commits_nr, -1); - if (bitmap_writer_build(&to_pack) < 0) + bitmap_writer_show_progress(&bitmap_writer, + progress); + bitmap_writer_select_commits(&bitmap_writer, + indexed_commits, + indexed_commits_nr, + -1); + if (bitmap_writer_build(&bitmap_writer, &to_pack) < 0) die(_("failed to write bitmap index")); - bitmap_writer_finish(written_list, nr_written, + bitmap_writer_finish(&bitmap_writer, + written_list, nr_written, tmpname.buf, write_bitmap_options); write_bitmap_index = 0; strbuf_setlen(&tmpname, tmpname_len); -- cgit v1.3