diff options
author | Jim Meyering <meyering@redhat.com> | 2010-04-15 23:26:22 +0200 |
---|---|---|
committer | Jim Meyering <meyering@redhat.com> | 2010-04-16 11:28:04 +0200 |
commit | f444711a2639b4974ff3e720c455a1f96f1109e8 (patch) | |
tree | aa8c5e45b3808828c9e82f739ec6e4b14ded8ff9 | |
parent | 648802169a29ea096d6e9813b043b91af9342f8d (diff) | |
download | diffutils-f444711a2639b4974ff3e720c455a1f96f1109e8.tar.gz |
diff: fix a regression when at least one input lacks a newline-at-EOF,
and the final hunk plus context-length aligns exactly with the end
of the newline-lacking file. Diff would fail to output the required
"\ No newline at end of file" line, thus rendering the output invalid.
This bug appears to have been introduced by 2006-05-07
commit 58d0483b, "(find_identical_ends): Fix huge performance bug...",
at least to the extent that reverting that change fixes the bug.
Considering the stated effect of that change and lack of metrics,
reverting it is not an option, so here we take a more direct approach.
Given these inputs,
printf '\n1'>a; printf '\n0\n\n1'>b
and running diff like this:
./diff -U1 a b
for input file "b", the pointer, files[1].linbuf[4][-1], to
the last byte on the final line was mistakenly pointing at the
sentinel newline at EOF, rather than at the preceding byte.
(gdb) p files[1].linbuf[4][-1]
$3 = 10 '\n'
Thus, this test in the final print_1_line call:
if ((!line_flag || line_flag[0]) && limit[-1] != '\n')
fprintf (out, "\n\\ %s\n", _("No newline at end of file"));
would fail, because limit[-1] (which is files[1].linbuf[4][-1])
was mistakenly '\n', rather than the desired '1'.
My first thought was simply to adjust the final linbuf[line] setting,
at the end of io.c's find_and_hash_each_line function function:
if (p == bufend)
- break;
+ {
+ if (current->missing_newline)
+ --linbuf[line];
+ break;
+ }
But that would make diff misbehave with this input
(same as above, but with a newline appended to "a"),
printf '\n1\n'>a; printf '\n0\n\n1'>b
./diff -U1 a b
due to the block (100 lines above) that is triggered in that case
(but not in the both-files-missing-newline case):
if (p == bufend
&& current->missing_newline
&& ROBUST_OUTPUT_STYLE (output_style))
{
/* This line is incomplete. If this is significant,
put the line into buckets[-1]. */
if (ignore_white_space < IGNORE_SPACE_CHANGE)
bucket = &buckets[-1];
/* Omit the inserted newline when computing linbuf later. */
p--;
bufend = suffix_begin = p;
}
Note how "p" is decremented and "bufend" adjusted.
When that happens, we certainly don't want to decrement
"bufend" yet again.
Since there is no other way to determine at the end whether "bufend"
was already decremented, add a new variable to serve as witness.
* NEWS (Bug fixes): Mention it.
Reported by Timo Juhani Lindfors in http://bugs.debian.org/577832.
Forwarded by Santiago Vila.
-rw-r--r-- | NEWS | 7 | ||||
-rw-r--r-- | src/io.c | 11 |
2 files changed, 17 insertions, 1 deletions
@@ -2,6 +2,13 @@ GNU diffutils NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + diff once again prints the required "\ No newline at end of file" line + when at least one input lacks a newline-at-EOF and the final hunk plus + context-length aligns exactly with the end of the newline-lacking file. + [bug introduced between 2.8.7 and 2.9] + ** Changes in behavior In context-style diffs, diff prints a portion of a preceding "function" @@ -220,6 +220,7 @@ find_and_hash_each_line (struct file_data *current) bool same_length_diff_contents_compare_anyway = diff_length_compare_anyway | ignore_case; + bool missing_newline_fixup = false; while (p < suffix_begin) { char const *ip = p; @@ -376,6 +377,7 @@ find_and_hash_each_line (struct file_data *current) && current->missing_newline && ROBUST_OUTPUT_STYLE (output_style)) { + missing_newline_fixup = true; /* This line is incomplete. If this is significant, put the line into buckets[-1]. */ if (ignore_white_space < IGNORE_SPACE_CHANGE) @@ -471,7 +473,14 @@ find_and_hash_each_line (struct file_data *current) linbuf[line] = p; if (p == bufend) - break; + { + /* If we've added a newline sentinel and did not adjust "bufend" + above, then linbuf[line] is now pointing at the sentinel, yet + should instead be pointing to the preceding byte. */ + if (!missing_newline_fixup && current->missing_newline) + --linbuf[line]; + break; + } if (context <= i && no_diff_means_no_output) break; |