From: Torsten Bögershausen <tboegi@web.de>
Date: Sun, 10 May 2026 09:14:36 GMT
Subject: Re: [PATCH v2 01/11] index-pack, unpack-objects: use size_t for object size
Message-ID: <20260510091436.GA5880@tb-raspi4>
In-Reply-To: <xmqqik8w9eb8.fsf@gitster.g>

> 
> Hmph, I do not think there is "up-cast" to keep.  And we do not
> dereference a random pointer that would be suitable for unsigned
> char * as if it were "unsigned long *" or "size_t *" in this code.
> 
> This came from commit 48fb7deb5bbd87933e7d314b73d7c1b52667f80f
> 
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Wed Jun 17 17:22:27 2009 -0700
> 
>     Fix big left-shifts of unsigned char
>     
>     Shifting 'unsigned char' or 'unsigned short' left can result in sign
>     extension errors, since the C integer promotion rules means that the
>     unsigned char/short will get implicitly promoted to a signed 'int' due to
>     the shift (or due to other operations).
>     
>     This normally doesn't matter, but if you shift things up sufficiently, it
>     will now set the sign bit in 'int', and a subsequent cast to a bigger type
>     (eg 'long' or 'unsigned long') will now sign-extend the value despite the
>     original expression being unsigned.
>     
>     One example of this would be something like
>     
>             unsigned long size;
>             unsigned char c;
>     
>             size += c << 24;
>     
>     where despite all the variables being unsigned, 'c << 24' ends up being a
>     signed entity, and will get sign-extended when then doing the addition in
>     an 'unsigned long' type.
> 
> 
> You could rewrite Linus's example to
> 
> 	unsigned char *cp;
> 	unsigned long size;
> 	unsigned char c;
> 
> 	c = *cp;
> 	size += ((unsigned long)c) << 24;
> 
> While I am sympathetic to that position, I also would not mind
> 
> 	unsigned char *cp;
> 	unsigned long size;
> 	unsigned long c;
> 
> 	c = *cp;
> 	size += c << 24;
> 
> all that much.  In any case, such a "clean-up" has little to do with
> the topic under discussion, and itshould be discussed separately on
> its own merit, most likely when the dust settles after this topic
> lands.  Let's not contaminate the patches that is "a trivial rewrite
> that is so obviously correct to fix the assumption that ulong and
> size_t are of the same size everywhere" with unrelated clean-up.
> 
> Thanks.
> 

Sorry for the confusion and noise.
My brain insisted to read
c = *cp; // fetch 8 bits from memory, upcast to unsigned long
as if we have written
c = *(long*)cp; // fetch 32/64 bits from memory
which is a completely different thing.

In short: all is good.
Thanks for digging and the patience.