| Message ID | ac1f46aa1f0dbc7fba45229555b11b390fe104a0.1624314293.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | multi-pack reachability bitmaps | expand |
On Mon, Jun 21, 2021 at 06:25:26PM -0400, Taylor Blau wrote: > static int load_bitmap_entries_v1(struct bitmap_index *index) > { > uint32_t i; > @@ -242,9 +249,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) > xor_offset = read_u8(index->map, &index->map_pos); > flags = read_u8(index->map, &index->map_pos); > > - if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0) > - return error("corrupt ewah bitmap: commit index %u out of range", > - (unsigned)commit_idx_pos); > + nth_bitmap_object_oid(index, &oid, commit_idx_pos); Oops. I was reading code in this area and noticed that this patch drops the check introduced by c6b0c3910c (pack-bitmap.c: check reads more aggressively when loading, 2020-12-08). I fixed it up locally by restoring the check (but on the new function nth_bitmap_object_oid() instead), and will send it in a reroll. Thanks, Taylor
On Mon, Jun 21, 2021 at 06:25:26PM -0400, Taylor Blau wrote: > A subsequent patch to support reading MIDX bitmaps will be less noisy > after extracting a generic function to fetch the nth OID contained in > the bitmap. Makes sense, but... > +static void nth_bitmap_object_oid(struct bitmap_index *index, > + struct object_id *oid, > + uint32_t n) > +{ > + nth_packed_object_id(oid, index->pack, n); > +} > + > static int load_bitmap_entries_v1(struct bitmap_index *index) > { > uint32_t i; > @@ -242,9 +249,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) > xor_offset = read_u8(index->map, &index->map_pos); > flags = read_u8(index->map, &index->map_pos); > > - if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0) > - return error("corrupt ewah bitmap: commit index %u out of range", > - (unsigned)commit_idx_pos); > + nth_bitmap_object_oid(index, &oid, commit_idx_pos); What happened to our error check here? Should nth_bitmap_object_oid() be returning the value from nth_packed_object_id()? -Peff
On Wed, Jul 21, 2021 at 06:37:41AM -0400, Jeff King wrote: > > @@ -242,9 +249,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) > > xor_offset = read_u8(index->map, &index->map_pos); > > flags = read_u8(index->map, &index->map_pos); > > > > - if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0) > > - return error("corrupt ewah bitmap: commit index %u out of range", > > - (unsigned)commit_idx_pos); > > + nth_bitmap_object_oid(index, &oid, commit_idx_pos); > > What happened to our error check here? > > Should nth_bitmap_object_oid() be returning the value from > nth_packed_object_id()? Ah, sorry, I just saw your followup message. I'll look for the fix in the re-roll. -Peff
diff --git a/pack-bitmap.c b/pack-bitmap.c index 2dc135d34a..9757cd0fbb 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -223,6 +223,13 @@ static inline uint8_t read_u8(const unsigned char *buffer, size_t *pos) #define MAX_XOR_OFFSET 160 +static void nth_bitmap_object_oid(struct bitmap_index *index, + struct object_id *oid, + uint32_t n) +{ + nth_packed_object_id(oid, index->pack, n); +} + static int load_bitmap_entries_v1(struct bitmap_index *index) { uint32_t i; @@ -242,9 +249,7 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) xor_offset = read_u8(index->map, &index->map_pos); flags = read_u8(index->map, &index->map_pos); - if (nth_packed_object_id(&oid, index->pack, commit_idx_pos) < 0) - return error("corrupt ewah bitmap: commit index %u out of range", - (unsigned)commit_idx_pos); + nth_bitmap_object_oid(index, &oid, commit_idx_pos); bitmap = read_bitmap_1(index); if (!bitmap) @@ -844,8 +849,8 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git, off_t ofs = pack_pos_to_offset(pack, pos); if (packed_object_info(the_repository, pack, ofs, &oi) < 0) { struct object_id oid; - nth_packed_object_id(&oid, pack, - pack_pos_to_index(pack, pos)); + nth_bitmap_object_oid(bitmap_git, &oid, + pack_pos_to_index(pack, pos)); die(_("unable to get size of %s"), oid_to_hex(&oid)); } } else {
A subsequent patch to support reading MIDX bitmaps will be less noisy after extracting a generic function to fetch the nth OID contained in the bitmap. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- pack-bitmap.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)