[PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps

[PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps

[Jeff King]

Now that tb/multi-pack-bitmaps is in "next", my local -Wunused-parameter
topic noticed a few possible cleanups. Neither is a big deal, and I can
definitely hold them for a larger cleanup. But I thought it might be
worth doing them while they're fresh in our minds.

  [1/2]: pack-bitmap: drop repository argument from prepare_midx_bitmap_git()
  [2/2]: pack-bitmap: drop bitmap_index argument from try_partial_reuse()

 midx.c        | 2 +-
 pack-bitmap.c | 8 +++-----
 pack-bitmap.h | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)

-Peff

[PATCH 1/2] pack-bitmap: drop repository argument from prepare_midx_bitmap_git()

[Jeff King]

We never look at the repository argument which is passed. This makes
sense, since the multi_pack_index struct already tells us everything we
need to access the files in its associated object directory.

Signed-off-by: Jeff King <peff@peff.net>
---
 midx.c        | 2 +-
 pack-bitmap.c | 3 +--
 pack-bitmap.h | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/midx.c b/midx.c
index ccdc3e5702..864034a6ad 100644
--- a/midx.c
+++ b/midx.c
@@ -1144,7 +1144,7 @@ static int write_midx_internal(const char *object_dir,
 		int bitmap_exists;
 		int want_bitmap = flags & MIDX_WRITE_BITMAP;
 
-		bitmap_git = prepare_midx_bitmap_git(the_repository, ctx.m);
+		bitmap_git = prepare_midx_bitmap_git(ctx.m);
 		bitmap_exists = bitmap_git && bitmap_is_midx(bitmap_git);
 		free_bitmap_index(bitmap_git);
 
diff --git a/pack-bitmap.c b/pack-bitmap.c
index fa69ed7a6d..f2a7994a9c 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -517,8 +517,7 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
 	return NULL;
 }
 
-struct bitmap_index *prepare_midx_bitmap_git(struct repository *r,
-					     struct multi_pack_index *midx)
+struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 {
 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
 
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 81664f933f..469090bad2 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -44,8 +44,7 @@ typedef int (*show_reachable_fn)(
 struct bitmap_index;
 
 struct bitmap_index *prepare_bitmap_git(struct repository *r);
-struct bitmap_index *prepare_midx_bitmap_git(struct repository *r,
-					     struct multi_pack_index *midx);
+struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
 void count_bitmap_commit_list(struct bitmap_index *, uint32_t *commits,
 			      uint32_t *trees, uint32_t *blobs, uint32_t *tags);
 void traverse_bitmap_commit_list(struct bitmap_index *,
-- 
2.33.0.621.ga797e945d8

[PATCH 2/2] pack-bitmap: drop bitmap_index argument from try_partial_reuse()

[Jeff King]

Starting in commit 0f533c7284 (pack-bitmap: read multi-pack bitmaps,
2021-08-31), we no longer look at the "struct bitmap_index" passed to
try_partial_reuse(). This is because we only handle verbatim reuse from
a single pack: either the pack whose bitmap we're looking at, or the
"preferred" pack of a midx bitmap. And thus the primary item we look at
is the "pack" parameter added by that same commit, and not the
bitmap_git->pack parameter (which would be NULL for a midx bitmap). It's
our caller, reuse_partial_packfile_from_bitmap(), which decides which
pack to use and passes it in to us.

Drop the unused parameter to prevent confusion.

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-bitmap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index f2a7994a9c..8504110a4d 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1328,8 +1328,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
  * -1 means "stop trying further objects"; 0 means we may or may not have
  * reused, but you can keep feeding bits.
  */
-static int try_partial_reuse(struct bitmap_index *bitmap_git,
-			     struct packed_git *pack,
+static int try_partial_reuse(struct packed_git *pack,
 			     size_t pos,
 			     struct bitmap *reuse,
 			     struct pack_window **w_curs)
@@ -1476,7 +1475,7 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 				break;
 
 			offset += ewah_bit_ctz64(word >> offset);
-			if (try_partial_reuse(bitmap_git, pack, pos + offset,
+			if (try_partial_reuse(pack, pos + offset,
 					      reuse, &w_curs) < 0) {
 				/*
 				 * try_partial_reuse indicated we couldn't reuse
-- 
2.33.0.621.ga797e945d8

Re: [PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps

[Taylor Blau]

On Thu, Sep 09, 2021 at 03:55:31PM -0400, Jeff King wrote:
> Now that tb/multi-pack-bitmaps is in "next", my local -Wunused-parameter
> topic noticed a few possible cleanups. Neither is a big deal, and I can
> definitely hold them for a larger cleanup. But I thought it might be
> worth doing them while they're fresh in our minds.

Thanks, I'd be happy to see this picked up while the topic is still in
next. Both of the spots you touched looked good to me (and don't
suddenly become necessary in any of the later series that I'm sitting on).

    Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor