From 3ae5fa0768f7f9781b40b1d40cb2f9f4c753bad4 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 7 Jun 2018 12:04:13 -0700 Subject: pack-bitmap: remove bitmap_git global variable Remove the bitmap_git global variable. Instead, generate on demand an instance of struct bitmap_index for code that needs to access it. This allows us significant control over the lifetime of instances of struct bitmap_index. In particular, packs can now be closed without worrying if an unnecessarily long-lived "pack" field in struct bitmap_index still points to it. The bitmap API is also clearer in that we need to first obtain a struct bitmap_index, then we use it. This patch raises two potential issues: (1) memory for the struct bitmap_index is allocated without being freed, and (2) prepare_bitmap_git() and prepare_bitmap_walk() can reuse a previously loaded bitmap. For (1), this will be dealt with in a subsequent patch in this patch set that also deals with freeing the contents of the struct bitmap_index (which were not freed previously, because they have global scope). For (2), current bitmap users only load the bitmap once at most (note that pack-objects can use bitmaps or write bitmaps, but not both at the same time), so support for reuse has no effect - and future users can pass around the struct bitmap_index * obtained if they need to do 2 or more things with the same bitmap. Helped-by: Stefan Beller Signed-off-by: Jonathan Tan Helped-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'builtin/pack-objects.c') diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 71056d8294..d064f944b0 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2929,11 +2929,13 @@ static int pack_options_allow_reuse(void) static int get_object_list_from_bitmap(struct rev_info *revs) { - if (prepare_bitmap_walk(revs) < 0) + struct bitmap_index *bitmap_git; + if (!(bitmap_git = prepare_bitmap_walk(revs))) return -1; if (pack_options_allow_reuse() && !reuse_partial_packfile_from_bitmap( + bitmap_git, &reuse_packfile, &reuse_packfile_objects, &reuse_packfile_offset)) { @@ -2942,7 +2944,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs) display_progress(progress_state, nr_result); } - traverse_bitmap_commit_list(&add_object_entry_from_bitmap); + traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap); return 0; } -- cgit v1.2.1 From f3c23db2d7e764b247f7d76a8d0ba180811e9525 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 7 Jun 2018 12:04:14 -0700 Subject: pack-bitmap: add free function Add a function to free struct bitmap_index instances, and use it where needed (except when rebuild_existing_bitmaps() is used, since it creates references to the bitmaps within the struct bitmap_index passed to it). Note that the hashes field in struct bitmap_index is not freed because it points to another field within the same struct. The documentation for that field has been updated to clarify that. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin/pack-objects.c') diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index d064f944b0..896e413007 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -2945,6 +2945,7 @@ static int get_object_list_from_bitmap(struct rev_info *revs) } traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap); + free_bitmap_index(bitmap_git); return 0; } -- cgit v1.2.1