A future commit will want to deal with bit positions instead of pack positions from within builtin/pack-objects.c::write_reused_pack_one(). That function at present takes a pack position, so one approach to accommodating the new functionality would be to add a secondary bit position parameter, making the function's declaration look something like: static void write_reused_pack_one(struct packed_git *reuse_packfile, size_t pack_pos, size_t bitmap_pos, struct hashfile *out, off_t pack_start, struct pack_window **w_curs); But because the pack-relative position can be easily derived from the bit position, it makes senes to just pass the latter and let the function itself translate it into a pack-relative position. To do this, extract a new function `bitmap_to_pack_pos()` from the existing `write_reused_pack()` function. This new routine is responsible for performing the conversion from bitmap- to pack-relative positions. Instead of performing that translation in `write_reused_pack()`, instead call the new function from within `write_reused_pack_one()` so that we can just pass a single bit position to it. Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx> --- builtin/pack-objects.c | 78 ++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 097bb5ac2ca..7f50d58a235 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1017,6 +1017,42 @@ static off_t find_reused_offset(off_t where) return reused_chunks[lo-1].difference; } +static uint32_t bitmap_to_pack_pos(struct packed_git *reuse_packfile, + size_t pos) +{ + if (bitmap_is_midx(bitmap_git)) { + /* + * When doing multi-pack reuse on a + * non-preferred pack, translate bit positions + * from the MIDX pseudo-pack order back to their + * pack-relative positions before attempting + * reuse. + */ + struct multi_pack_index *m = bitmap_midx(bitmap_git); + uint32_t midx_pos, pack_pos; + off_t pack_ofs; + + if (!m) + BUG("non-zero bitmap position without MIDX"); + + midx_pos = pack_pos_to_midx(m, pos); + pack_ofs = nth_midxed_offset(m, midx_pos); + + if (offset_to_pack_pos(reuse_packfile, pack_ofs, &pack_pos) < 0) + BUG("could not find expected object at offset %"PRIuMAX" in pack %s", + (uintmax_t)pack_ofs, pack_basename(reuse_packfile)); + + return pack_pos; + } else { + /* + * Can use bit positions directly, even for MIDX + * bitmaps. See comment in try_partial_reuse() + * for why. + */ + return pos; + } +} + static void write_reused_pack_one(struct packed_git *reuse_packfile, size_t pos, struct hashfile *out, off_t pack_start, @@ -1025,9 +1061,10 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile, off_t offset, next, cur; enum object_type type; unsigned long size; + uint32_t pack_pos = bitmap_to_pack_pos(reuse_packfile, pos); - offset = pack_pos_to_offset(reuse_packfile, pos); - next = pack_pos_to_offset(reuse_packfile, pos + 1); + offset = pack_pos_to_offset(reuse_packfile, pack_pos); + next = pack_pos_to_offset(reuse_packfile, pack_pos + 1); record_reused_object(offset, offset - (hashfile_total(out) - pack_start)); @@ -1191,7 +1228,6 @@ static void write_reused_pack(struct bitmapped_pack *reuse_packfile, size_t pos = (i * BITS_IN_EWORD); for (offset = 0; offset < BITS_IN_EWORD; ++offset) { - uint32_t pack_pos; if ((word >> offset) == 0) break; @@ -1201,40 +1237,8 @@ static void write_reused_pack(struct bitmapped_pack *reuse_packfile, if (pos + offset >= reuse_packfile->bitmap_pos + reuse_packfile->bitmap_nr) goto done; - if (bitmap_is_midx(bitmap_git)) { - /* - * When doing multi-pack reuse on a - * non-preferred pack, translate bit positions - * from the MIDX pseudo-pack order back to their - * pack-relative positions before attempting - * reuse. - */ - struct multi_pack_index *m = bitmap_midx(bitmap_git); - uint32_t midx_pos; - off_t pack_ofs; - - if (!m) - BUG("non-zero bitmap position without MIDX"); - - midx_pos = pack_pos_to_midx(m, pos + offset); - pack_ofs = nth_midxed_offset(m, midx_pos); - - if (offset_to_pack_pos(reuse_packfile->p, - pack_ofs, &pack_pos) < 0) - BUG("could not find expected object at offset %"PRIuMAX" in pack %s", - (uintmax_t)pack_ofs, - pack_basename(reuse_packfile->p)); - } else { - /* - * Can use bit positions directly, even for MIDX - * bitmaps. See comment in try_partial_reuse() - * for why. - */ - pack_pos = pos + offset; - } - - write_reused_pack_one(reuse_packfile->p, pack_pos, f, - pack_start, &w_curs); + write_reused_pack_one(reuse_packfile->p, pos + offset, + f, pack_start, &w_curs); display_progress(progress_state, ++written); } } -- 2.47.0.11.g487258bca34