Re: [RFC PATCH, WAS: "weird diff output?" 0/2] implement better chunk heuristics

From: Stefan Beller <sbeller@google.com>
Date: Apr 15, 2016, 17:10 UTC
Message-ID: <CAGZ79kZs33sJj+DPSS4FPoJTLqbCLpvSe_h9UUQM-dBe=8ExKw@mail.gmail.com>
In-Reply-To: <CAGZ79kbCHA3L6mUfYn6OfVXLDEyhv70PwxXo-YHP_QZXXAB8ig@mail.gmail.com>

On Fri, Apr 15, 2016 at 10:02 AM, Stefan Beller <sbeller@google.com> wrote:

Show 29 quoted lines

>> @@ -458,11 +458,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
>>                          * the group.
>>                          */
>>                         while (ixs > 0 && xdl_hash_and_recmatch(recs, ixs - 1, ix - 1, flags)) {
>> +                               emptylines += is_emptyline(recs[ix - 1]->ptr);
>> +
>>                                 rchg[--ixs] = 1;
>>                                 rchg[--ix] = 0;
>>
>> -                               has_emptyline |=
>> -                                       starts_with_emptyline(recs[ix]->ptr);
>
> How is this fixing the segfault bug?
> From my understanding ix should have the same value here as ix-1 above.
>
>>                                 /*
>>                                  * This change might have joined two change groups,
>>                                  * so we try to take this scenario in account by moving
>> @@ -492,9 +492,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
>>                                 rchg[ixs++] = 0;
>>                                 rchg[ix++] = 1;
>>
>> -                               has_emptyline |=
>> -                                       starts_with_emptyline(recs[ix]->ptr);
>> -
>
> Or would this fix the segfault bug?
> I think we would need to have the check for empty lines
> in both loops to be sure to cover the whole movable range.

Actually we would only need to have the empty line count in the second loop as the first loop shifted it as much up(backwards) as possible, such that the hunk sits on one end of the movable range. The second loop would iterate over the whole range, so that would be the only place needed to count.