On Thu, Jul 13, 2023 at 09:21:55AM +0100, Phillip Wood wrote:
> > diff --git a/packfile.c b/packfile.c
> > index 89220f0e03..70acf1694b 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -186,7 +186,7 @@ int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
> > */
> > (sizeof(off_t) <= 4))
> > return error("pack too large for current definition of off_t in %s", path);
> > - p->crc_offset = 8 + 4 * 256 + nr * hashsz;
> > + p->crc_offset = st_add(8 + 4 * 256, st_mult(nr, hashsz));
>
> p->crc_offset is a uint32_t so we're still prone to truncation here unless
> we change the crc_offset member of struct packed_git to be a size_t. I
> haven't checked if the other users of crc_offset would need adjusting if we
> change its type.
Thanks for spotting. Luckily, this should be a straightforward change:
$ git grep crc_offset
builtin/index-pack.c: idx1 = (((const uint32_t *)((const uint8_t *)p->index_data + p->crc_offset))
object-store-ll.h: uint32_t crc_offset;
packfile.c: p->crc_offset = st_add(8 + 4 * 256, st_mult(nr, hashsz));
The single usage in index-pack is OK, so we only need to change its type
to a size_t.
I could see an argument that this should be an off_t, since it is an
offset into a file. But since we memory map the whole thing anyway, I
think we are equally OK to treat it as a pointer offset. A similar
argument is made in f86f769550e (compute pack .idx byte offsets using
size_t, 2020-11-13), so I am content to leave this as a size_t.
Thanks,
Taylor
