Re: [RFC PATCH 00/10] range-diff: fix segfault due to integer overflow
- From
Philip Oakley <philipoakley@iee.email>
- Date
- Dec 21, 2021, 23:22 UTC
- Message-ID
- <59ec39af-fdb1-a86a-d2be-37e5954e245f@iee.email>
- In-Reply-To
- <nycvar.QRO.7.76.6.2112101528200.90@tvgsbejvaqbjf.bet>
Sorry for the late comment..
On 10/12/2021 14:31, Johannes Schindelin wrote:
Show 14 quoted lines
> Hi Ævar,
>
> On Thu, 9 Dec 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> The difference between "master" and "git-for-windows/main" is large
>> enough that comparing the two will segfault on my system. This is
>> because the range-diff code does some expensive calculations and will
>> overflow the "int" type.
> You are holding this thing wrong.
>
> The `main` branch of Git for Windows uses merging rebases, therefore you
> need to use a commit range like
> `git-for-windows/main^{/^Start.the.merging}..git-for-windows/main` and
> compare it to `git-for-windows/main..master`.I'm not sure that a Git repo has an established way of indicating to how it's branching/merging/releasing workflow is set up, especially for projects with non-normative use cases, such as Git for Windows. We don't have a git document for covering the different workflows in common use for easy reference and consistent terminology.
The merging rebase flow, with 'fake' merge does solve a problem that git.git doesn't have but could easily be a common process for 'friendly forks' that follow an upstream with local patches. The choice of '{/^Start.the.merging}' is currently specific to the Git-for-Windows case making it harder to discover this useful maintainer method.
I fully agree that the range-diff should probably have a patch limit at some sensible value.
The 'confusion' between the types size_t, long and int, does ripple through a lot of portable code, as shown in the series. Not an easy problem.
Show 8 quoted lines
> > Failing that, you will receive only bogus results. > > As to the patch series, it likely does the wrong thing. Just like we error > out on insanely large input in libxdiff, `range-diff` should do the same. > > Ciao, > Johannes
-- Philip