Re: [PATCH 2/2] xdiff: implement empty line chunk heuristic

From: Stefan Beller <sbeller@google.com>
Date: Apr 16, 2016, 00:59 UTC
Message-ID: <CAGZ79kY2u_X2xmm=+wvA13bnD57+o1br7vhAL0-Rf6K_1PeXTA@mail.gmail.com>
In-Reply-To: <xmqqd1pq74ys.fsf@gitster.mtv.corp.google.com>

On Fri, Apr 15, 2016 at 5:49 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +static int line_length(const char *recs)
>> +{
>> +     char *s = strchr(recs, '\n');
>> +     return s ? s - recs : strlen(recs);
>> +}
>
> It seems that you guys are discarding this "number of bytes on a
> line, no matter what these bytes are" idea, so this may be moot, but
> is there a guarantee that reading through recs until you happen to
> see a NUL is safe?

We discarded this idea as it produces to many errors. (We'd be back at the 50:50 case, "is it really worth it?")

We will go back to the "empty line" heuristic, which will be solved via xdl_blankline(rec[i]->ptr, rec[i]->size, flags); which could be inlined. That will solve the CRLF issue as a CR is covered as a whitespace (with CRLF you'd have to specify diff to ignore white spaces).

For the safety I assumed
* there is always a \n even on the last line by convention.
* in case it is not, the string is null terminated, hence
  strchr and strlen for the rescue.

>
> Shouldn't the code that accesses a "line" be using the same "from
> here to there", i.e. recs[]->ptr, recs[]->size, interface to avoid
> having to scan the underlying string in an unbounded way?

xdl_blankline will use ->size, so we'll be holding it right.

Thanks, Stefan

>
>