From: Torsten Bögershausen Date: Tue, 05 May 2026 19:11:00 GMT Subject: Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size Message-ID: <20260505191100.GA12275@tb-raspi4> In-Reply-To: On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin > > When unpacking objects from a packfile, the object size is decoded > from a variable-length encoding. On platforms where unsigned long is > 32-bit (such as Windows, even in 64-bit builds), the shift operation > overflows when decoding sizes larger than 4GB. The result is a > truncated size value, causing the unpacked object to be corrupted or > rejected. > > Fix this by changing the size variable to size_t, which is 64-bit on > 64-bit platforms, and ensuring the shift arithmetic occurs in 64-bit > space. > > This was originally authored by LordKiRon , > who preferred not to reveal their real name and therefore agreed that I > take over authorship. > > Signed-off-by: Johannes Schindelin > --- > builtin/index-pack.c | 9 +++++---- > builtin/unpack-objects.c | 5 +++-- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/builtin/index-pack.c b/builtin/index-pack.c > index ca7784dc2c..cc660582e9 100644 > --- a/builtin/index-pack.c > +++ b/builtin/index-pack.c > @@ -37,7 +37,7 @@ static const char index_pack_usage[] = > > struct object_entry { > struct pack_idx_entry idx; > - unsigned long size; > + size_t size; > unsigned char hdr_size; > signed char type; > signed char real_type; > @@ -469,7 +469,7 @@ static int is_delta_type(enum object_type type) > return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA); > } > > -static void *unpack_entry_data(off_t offset, unsigned long size, > +static void *unpack_entry_data(off_t offset, size_t size, > enum object_type type, struct object_id *oid) > { > static char fixed_buf[8192]; > @@ -524,7 +524,8 @@ static void *unpack_raw_entry(struct object_entry *obj, > struct object_id *oid) > { > unsigned char *p; > - unsigned long size, c; > + size_t size; > + unsigned long c; Does this look a little bit strange ? p points to an unsigned char (better would be *uint8_t) then it is dereferenced into an "unsigned long". Then it is masked with 0x7f In short: should "c" be declared as uint8_t ? > off_t base_offset; > unsigned shift; > void *data; > @@ -542,7 +543,7 @@ static void *unpack_raw_entry(struct object_entry *obj, > p = fill(1); > c = *p; > use(1); > - size += (c & 0x7f) << shift; > + size += ((size_t)c & 0x7f) << shift; > shift += 7; > } > obj->size = size; > diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c > index e01cf6e360..59a36c2481 100644 > --- a/builtin/unpack-objects.c > +++ b/builtin/unpack-objects.c > @@ -533,7 +533,8 @@ static void unpack_one(unsigned nr) > { > unsigned shift; > unsigned char *pack; > - unsigned long size, c; > + size_t size; > + unsigned long c; > enum object_type type; > > obj_list[nr].offset = consumed_bytes; > @@ -548,7 +549,7 @@ static void unpack_one(unsigned nr) > pack = fill(1); > c = *pack; > use(1); > - size += (c & 0x7f) << shift; > + size += ((size_t)c & 0x7f) << shift; > shift += 7; > } > > -- > gitgitgadget > >