Re: [PATCH 02/20] write_reuse_object(): convert to new revindex API

From: Jeff King <peff@peff.net>
Date: Jan 13, 2021, 13:02 UTC
Message-ID: <X/7vb9Dnrfa1Yaut@coredump.intra.peff.net>
In-Reply-To: <X/3O7D18gRLfSNJ8@nand.local>

On Tue, Jan 12, 2021 at 11:31:40AM -0500, Taylor Blau wrote:

Show 12 quoted lines

> > > +	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