On Tue, Jan 12, 2021 at 03:47:47AM -0500, Jeff King wrote:
> > @@ -436,10 +436,13 @@ static off_t write_reuse_object(struct hashfile *f, struct object_entry *entry,
> > type, entry_size);
> >
> > offset = entry->in_pack_offset;
> > - revidx = find_pack_revindex(p, offset);
> > - datalen = revidx[1].offset - offset;
> > + if (offset_to_pack_pos(p, offset, &pos) < 0)
> > + die(_("write_reuse_object: could not locate %s"),
> > + oid_to_hex(&entry->idx.oid));
>
> If we believe the offset is bogus, should we print that in the error
> message, too? Something like:
>
> die("could not locate %s, expected at offset %"PRIuMAX" in pack %s",
> oid_to_hex(&entry->idx.oid), (uintmax_t)offset, p->pack_name);
Good idea, thanks.
> > + datalen = pack_pos_to_offset(p, pos + 1) - offset;
>
> This "pos + 1" means we may be looking one past the end of the array.
> That's OK (at least for now), because our revindex always puts in an
> extra dummy value exactly for computing these kinds of byte-distances.
> That might be worth documenting in the API header.
Yeah, I made sure to document that when I was touching up the last
patch. FWIW, that's a behavior that we're going to carry over even when
the reverse index is stored on-disk (not by writing four extra bytes
into the .rev file, but by handling queries for pos == p->num_objects
separately.)
> -Peff
Thanks,
Taylor
