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

From: Johannes Schindelin <[email-masked]>
Date: Fri, 8 May 2026 09:36:53 +0200 (CEST)

Hi Torsten,

On Tue, 5 May 2026, Torsten Bögershausen wrote:

> On Mon, May 04, 2026 at 05:08:18PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <[email-masked]>
> > 
> > [...]
> > @@ -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 ?

Good point.

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

Almost. It should be a `size_t`, so that we don't have to cast it when
shifting it. I'll include a fix in the next iteration.

Thank you!
Johannes

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