# [PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps 4 messages from 2021-09-09 to 2021-09-09. Participants: Jeff King, Taylor Blau. Thread: https://gitlist.dev/t/56474 ## Jeff King, 2021-09-09 19:55 Subject: [PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps Message-ID: URL: https://gitlist.dev/e/YTpms9UnVsx%2Buu%2F2%40coredump.intra.peff.net ``` 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, 2021-09-09 19:56 Subject: [PATCH 1/2] pack-bitmap: drop repository argument from prepare_midx_bitmap_git() Message-ID: URL: https://gitlist.dev/e/YTpnCsqXdzeFp8Uz%40coredump.intra.peff.net In-Reply-To: ``` 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 --- 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 ``` ## Jeff King, 2021-09-09 19:57 Subject: [PATCH 2/2] pack-bitmap: drop bitmap_index argument from try_partial_reuse() Message-ID: URL: https://gitlist.dev/e/YTpnIQSiYfSYlexy%40coredump.intra.peff.net In-Reply-To: ``` 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 --- 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 ``` ## Taylor Blau, 2021-09-09 20:46 Subject: Re: [PATCH 0/2] -Wunused-parameter clenaups on tb/multi-pack-bitmaps Message-ID: URL: https://gitlist.dev/e/YTpyp8ul6ipgDAhY%40nand.local In-Reply-To: ``` 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 Thanks, Taylor ```