On Tue, Jan 12, 2021 at 11:31:40AM -0500, Taylor Blau wrote: > > > + 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.) I think the only reason to look past the end like that is to compute the size of the final entry. So we _could_ abstract that away from the callers with a separate function like: off_t pack_pos_to_size(struct packed_git *p, uint32_t pos); But as long as the behavior of passing p->num_objects is documented, I do not mind overly mind spelling it one way or the other. -Peff
