Jim Meyering wrote:
There are still numerous unguarded [-1] references, so this updated
patch is doubtless still incomplete:
The real bug was elsewhere, I think. I installed the attached patch. This patch
lacks your test case, which didn't work for me because there is no
require_valgrind_ in diffutils. Is require_valgrind_ from coreutils or from some
other location?
>From c0097514df398ab70c58fe75af17ba0d7ec58c73 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 28 Dec 2018 19:00:50 -0800
Subject: [PATCH] diff: fix UMR with --strip-trailing-cr
Problem reported by Hongxu Chen (Bug#31935).
* src/io.c (prepare_text): Strip trailing CR before
doing the rest of the analysis.
* NEWS: Mention the fix.
Co-authored-by: Jim Meyering <j...@meyering.net>
---
NEWS | 7 +++++++
src/io.c | 39 +++++++++++++++------------------------
2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/NEWS b/NEWS
index 56e0445..7d115a3 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU diffutils NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** Bug fixes
+
+ diff --strip-trailing-cr with a single CR byte in one input file
+ would provoke an uninitialized memory read, e.g.,
+ diff -a --strip-trailing-cr <(printf '\r') <(echo a)
+ [bug introduced in 2.8 with addition of the --strip-trailing-cr option]
+
* Noteworthy changes in release 3.6 (2017-05-21) [stable]
diff --git a/src/io.c b/src/io.c
index fb86392..6c03c70 100644
--- a/src/io.c
+++ b/src/io.c
@@ -481,42 +481,33 @@ prepare_text (struct file_data *current)
{
size_t buffered = current->buffered;
char *p = FILE_BUFFER (current);
-
- if (buffered == 0 || p[buffered - 1] == '\n')
- current->missing_newline = false;
- else
- {
- p[buffered++] = '\n';
- current->missing_newline = true;
- }
-
if (!p)
return;
- /* Don't use uninitialized storage when planting or using sentinels. */
- memset (p + buffered, 0, sizeof (word));
-
if (strip_trailing_cr)
{
- char *dst;
char *srclim = p + buffered;
*srclim = '\r';
- dst = rawmemchr (p, '\r');
+ char *dst = rawmemchr (p, '\r');
- if (dst != srclim)
+ for (char const *src = dst; src != srclim; src++)
{
- char const *src = dst;
- do
- {
- *dst = *src++;
- dst += ! (*dst == '\r' && *src == '\n');
- }
- while (src < srclim);
-
- buffered -= src - dst;
+ src += *src == '\r' && src[1] == '\n';
+ *dst++ = *src;
}
+
+ buffered -= srclim - dst;
}
+ if (buffered != 0 && p[buffered - 1] != '\n')
+ {
+ p[buffered++] = '\n';
+ current->missing_newline = true;
+ }
+
+ /* Don't use uninitialized storage when planting or using sentinels. */
+ memset (p + buffered, 0, sizeof (word));
+
current->buffered = buffered;
}
--
2.17.1
