threads / patch / 56474

patch, 2 parts-Wunused-parameter clenaups on tb/multi-pack-bitmaps

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

## tl;dr

4 messages between Sep 9, 2021 and Sep 9, 2021. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jeff King· Sep 9, 2021, 19:55 UTC · lore

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
Jeff King· Sep 9, 2021, 19:56 UTC · re: Jeff King · lore

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

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(-)
Show changes to 3 files +3 −5

midx.c, pack-bitmap.c, pack-bitmap.h

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
Jeff King· Sep 9, 2021, 19:57 UTC · re: Jeff King · lore

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

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(-)
Show changes to pack-bitmap.c +2 −3
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
Taylor Blau· Sep 9, 2021, 20:46 UTC · re: Jeff King · lore

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

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

← back to recent threads