Re: [PATCH] Fix delta integer overflows

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 8, 2017, 11:38 UTC
Message-ID: <alpine.DEB.2.21.1.1708081336410.4271@virtualbox>
In-Reply-To: <20170808062047.GA4091@mail.zuhause>

Hi Martin,

On Tue, 8 Aug 2017, Martin Koegler wrote:

Show 7 quoted lines

> On Mon, Aug 07, 2017 at 09:39:12PM +0200, Johannes Schindelin wrote:
> > If you want to work on data in memory, then size_t is the appropriate data
> > type. We already use it elsewhere. Let's use it here, too, without the
> > intermediate bump from the incorrect `int` to the equally incorrect
> > `long`.
> 
> I disagree with "We already use it elsewhere.".

By "it" I meant "size_t".

> The whole delta code uses "unsigned long" - look at delta.h. Look at
> unpack-objects.c. Or cache.h. Or pack-objects.c. Or index-pack.c.

I know that. It is a major bug in the source code.

> Other possible cases:
> git grep "unsigned long" |grep size

Yes, even more bugs.

> So the codebase still suggests, that "unsigned long" is the data type
> for storing object sizes.

And it would be wrong, and we know it already. Most importantly, Junio knows it already.

> I would be fine with resubmitting a patch using size_t/off_t for the
> touched parts - changing the whole core code is a too invasive change
> for a bug fix.

Sorry, my mistake: I never meant to burden you with the invasive change. I only wanted you to change the `int` to `size_t` right away.

Thanks, Johannes