Re: [PATCH] mergesort: avoid left shift overflow

From: René Scharfe <l.s.r@web.de>
Date: Nov 19, 2021, 17:15 UTC
Message-ID: <fb70a801-369b-c1b1-457b-bab06688b2bc@web.de>
In-Reply-To: <nycvar.QRO.7.76.6.2111191750400.63@tvgsbejvaqbjf.bet>

Am 19.11.21 um 17:51 schrieb Johannes Schindelin:

Show 11 quoted lines

> Hi Philip,
>
> On Thu, 18 Nov 2021, Philip Oakley wrote:
>
>> I already had this locally as part of an MSVC (Visual Studio) fix for
>> the various C4334 warnings.
>>
>> The other cases are in object-file.c, diffcore-delta.c (2 occurrences) ,
>> and repack.c.
>>
>> My patches are in https://github.com/PhilipOakley/git/tree/oneU_t

Good warning, that.  And scary stuff.

builtin/repack.c shifts by 5, diffcore-delta.c by at most 17 IIUC, and object-file.c by at most 31, which should all be safe. Shifting by 32 or more would push the 1 off the right end, leaving only a surprising 0. Phew! Definitely worth fixing regardless.

> How about rebasing the remaining patches from
> https://github.com/git-for-windows/git/compare/main...PhilipOakley:oneU_t
> on top of `rs/mergesort` and then submitting them, to avoid duplicate
> efforts?

That would be nice. I'd also don't mind if Junio took the whole set incl. the mergesort.c change instead.

René