Skip to content

Commit 07647c9

Browse files
ttaylorrgitster
authored andcommitted
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 7cc8f97 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 94830fc commit 07647c9

File tree

4 files changed

+159
-123
lines changed

4 files changed

+159
-123
lines changed

builtin/pack-objects.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,7 @@ static void write_pack_file(void)
12451245
uint32_t nr_remaining = nr_result;
12461246
time_t last_mtime = 0;
12471247
struct object_entry **write_order;
1248+
struct bitmap_writer bitmap_writer;
12481249

12491250
if (progress > pack_to_stdout)
12501251
progress_state = start_progress(_("Writing objects"), nr_result);
@@ -1339,8 +1340,9 @@ static void write_pack_file(void)
13391340
hash_to_hex(hash));
13401341

13411342
if (write_bitmap_index) {
1342-
bitmap_writer_set_checksum(hash);
1343-
bitmap_writer_build_type_index(
1343+
bitmap_writer_init(&bitmap_writer);
1344+
bitmap_writer_set_checksum(&bitmap_writer, hash);
1345+
bitmap_writer_build_type_index(&bitmap_writer,
13441346
&to_pack, written_list, nr_written);
13451347
}
13461348

@@ -1358,11 +1360,16 @@ static void write_pack_file(void)
13581360
strbuf_addstr(&tmpname, "bitmap");
13591361
stop_progress(&progress_state);
13601362

1361-
bitmap_writer_show_progress(progress);
1362-
bitmap_writer_select_commits(indexed_commits, indexed_commits_nr, -1);
1363-
if (bitmap_writer_build(&to_pack) < 0)
1363+
bitmap_writer_show_progress(&bitmap_writer,
1364+
progress);
1365+
bitmap_writer_select_commits(&bitmap_writer,
1366+
indexed_commits,
1367+
indexed_commits_nr,
1368+
-1);
1369+
if (bitmap_writer_build(&bitmap_writer, &to_pack) < 0)
13641370
die(_("failed to write bitmap index"));
1365-
bitmap_writer_finish(written_list, nr_written,
1371+
bitmap_writer_finish(&bitmap_writer,
1372+
written_list, nr_written,
13661373
tmpname.buf, write_bitmap_options);
13671374
write_bitmap_index = 0;
13681375
strbuf_setlen(&tmpname, tmpname_len);

midx-write.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,7 @@ static int write_midx_bitmap(const char *midx_name,
798798
{
799799
int ret, i;
800800
uint16_t options = 0;
801+
struct bitmap_writer writer;
801802
struct pack_idx_entry **index;
802803
char *bitmap_name = xstrfmt("%s-%s.bitmap", midx_name,
803804
hash_to_hex(midx_hash));
@@ -819,8 +820,10 @@ static int write_midx_bitmap(const char *midx_name,
819820
for (i = 0; i < pdata->nr_objects; i++)
820821
index[i] = &pdata->objects[i].idx;
821822

822-
bitmap_writer_show_progress(flags & MIDX_PROGRESS);
823-
bitmap_writer_build_type_index(pdata, index, pdata->nr_objects);
823+
bitmap_writer_init(&writer);
824+
bitmap_writer_show_progress(&writer, flags & MIDX_PROGRESS);
825+
bitmap_writer_build_type_index(&writer, pdata, index,
826+
pdata->nr_objects);
824827

825828
/*
826829
* bitmap_writer_finish expects objects in lex order, but pack_order
@@ -838,13 +841,14 @@ static int write_midx_bitmap(const char *midx_name,
838841
for (i = 0; i < pdata->nr_objects; i++)
839842
index[pack_order[i]] = &pdata->objects[i].idx;
840843

841-
bitmap_writer_select_commits(commits, commits_nr, -1);
842-
ret = bitmap_writer_build(pdata);
844+
bitmap_writer_select_commits(&writer, commits, commits_nr, -1);
845+
ret = bitmap_writer_build(&writer, pdata);
843846
if (ret < 0)
844847
goto cleanup;
845848

846-
bitmap_writer_set_checksum(midx_hash);
847-
bitmap_writer_finish(index, pdata->nr_objects, bitmap_name, options);
849+
bitmap_writer_set_checksum(&writer, midx_hash);
850+
bitmap_writer_finish(&writer, index, pdata->nr_objects, bitmap_name,
851+
options);
848852

849853
cleanup:
850854
free(index);

0 commit comments

Comments
 (0)