Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
On Mon, Sep 12, 2016 at 4:53 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> diff --git a/diff.c b/diff.c
>> index 156c2aa..9d2e704 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const
>> char *set, const char *res
>>
>> if (len == 0) {
>> has_trailing_newline = (first == '\n');
>> - has_trailing_carriage_return = (!has_trailing_newline &&
>> - (first == '\r'));
>> + has_trailing_carriage_return = (first == '\r');
>> nofirst = has_trailing_newline || has_trailing_carriage_return;
>> } else {
>> has_trailing_newline = (len > 0 && line[len-1] == '\n');
>
> Interesting.
>
> This may be a mis-conversion at 250f7993 ("diff.c: split emit_line()
> from the first char and the rest of the line", 2009-09-14), I
> suspect. The original took line[] with length and peeked for '\n',
> and when it saw one, it decremented length before checking
> line[len-1] for '\r'.
>
> But of course if there is only one byte on the line (i.e. len == 0
> after first is stripped off), it cannot be both '\n' or '\r' at the
> same time.
>
> Thanks for spotting.
Oh, right, it used to be possible to remove \r\n completely and that information
was then kept as has_trailing_newline = has_trailing_carriage_return = 1;
and the resulting line is kept completely without ending line.
After some thought I don't think I can use this mis-conversion
to trigger a bug though, because the len=0 can only ever happen
if first is '\n' alone essentially.
Another thing I noticed when playing around with diffs:
$ printf "\r\n" >crlf
$ git commit crlf -m "add file crlf, empty line"
$ printf "non zero length\r\n" >crlf
$ diff --git a/crlf b/crlf
$ index d3f5a12..ece7140 100644
--- a/crlf
+++ b/crlf
@@ -1 +1 @@
-
+non zero length^M
$ # The - line is missing a ^M ?
Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
Stefan Beller writes:
> diff --git a/diff.c b/diff.c
> index 156c2aa..9d2e704 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const
> char *set, const char *res
>
> if (len == 0) {
> has_trailing_newline = (first == '\n');
> - has_trailing_carriage_return = (!has_trailing_newline &&
> - (first == '\r'));
> + has_trailing_carriage_return = (first == '\r');
> nofirst = has_trailing_newline || has_trailing_carriage_return;
> } else {
> has_trailing_newline = (len > 0 && line[len-1] == '\n');
Interesting.
This may be a mis-conversion at 250f7993 ("diff.c: split emit_line()
from the first char and the rest of the line", 2009-09-14), I
suspect. The original took line[] with length and peeked for '\n',
and when it saw one, it decremented length before checking
line[len-1] for '\r'.
But of course if there is only one byte on the line (i.e. len == 0
after first is stripped off), it cannot be both '\n' or '\r' at the
same time.
Thanks for spotting.
[PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
From: Stefan Beller
When `first` equals '\r', then it cannot equal '\n', which makes
`!has_trailing_newline` always true if `first` is '\r'.
Drop that check.
Signed-off-by: Stefan Beller
---
diff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 156c2aa..9d2e704 100644
--- a/diff.c
+++ b/diff.c
@@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char
*set, const char *res
if (len == 0) {
has_trailing_newline = (first == '\n');
- has_trailing_carriage_return = (!has_trailing_newline &&
- (first == '\r'));
+ has_trailing_carriage_return = (first == '\r');
nofirst = has_trailing_newline || has_trailing_carriage_return;
} else {
has_trailing_newline = (len > 0 && line[len-1] == '\n');
--
2.7.4
