Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size

["Torsten Bögershausen" <tboegi@web.de>]

On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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 <https://github.com/LordKiRon>,
> who preferred not to reveal their real name and therefore agreed that I
> take over authorship.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  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
> 
>