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

From: Torsten Bögershausen <tboegi@web.de>
Date: May 8, 2026, 19:09 UTC
Message-ID: <20260508190947.GA25792@tb-raspi4>
In-Reply-To: <fa39d84b-ddbc-3943-5cca-078fb18db80d@gmx.de>

On Fri, May 08, 2026 at 09:36:53AM +0200, Johannes Schindelin wrote:

Show 27 quoted lines

> 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 <johannes.schindelin@gmx.de>
> > > 
> > > [...]
> > > @@ -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.

I think I was not very clear here. De-referencing a long (or size_t) from any address is something I would try to avoid: Some processors do not like to read a 32 or 64 bit value from an uneven address (and throw an exeption). x86 processors just handle it, using more than one bus cycle.

In short: Please keep the up-cast and simply read an uint8_t.