Re: [PATCH] Fix delta integer overflows
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 7, 2017, 21:10 UTC
- Message-ID
- <alpine.DEB.2.21.1.1708072307410.4271@virtualbox>
- In-Reply-To
- <xmqqshh3p3b0.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 7 Aug 2017, Junio C Hamano wrote:
Show 22 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> The patch obviously makes the code better and self consistent in > >> that "struct delta_index" has src_size as ulong, and this function > >> takes trg_size as ulong, and it was plain wrong for the code to > >> assume that "i", which is uint, can receive it safely. > >> > >> In the longer term we might want to move to size_t or even > >> uintmax_t, as the ulong on a platform may not be long enough in > >> order to express the largest file size the platform can have, but > >> this patch (1) is good even without such a change, and (2) gives a > >> good foundation to build on if we want such a change on top. > >> > >> Thanks. Will queue. > > > > This is sad. There is no "may not be long enough". We already know a > > platform where unsigned long is not long enough, don't we? Why leave this > > patch in this intermediate state? > > This is a good foundation to build on, and I never said no further > update on top of this patch is desired. Look for "(2)" in what you > quoted.
So are you saying that starting with v2.14.0, you accept patches into `pu` for which you would previously have required multiple iterations before even considering it for `pu`?
Frankly, I am a bit surprised that this obvious change from `unsigned long` to `size_t` is not required in this case before queuing, but if the rules have changed to lower the bar for patch submissions, I am all for it. I always felt that we are wasting contributors' time a little too freely and too deliberately.
Ciao, Dscho