Thanks for reporting that. I applied the attached patches. The first
fixes the bug and adds a test case, the second fixes some unlikely and
hard-to-test-for integer-overflow bugs I noticed while I was in the
neighborhood.
From 15f78a1970a6d2c5cd41123864c44cb694b46d14 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Mon, 24 Feb 2014 21:38:02 -0800
Subject: [PATCH 1/2] diff: fix bug with -I and overlapping hunks
Problem reported by Vincent Lefevre in <http://bugs.gnu.org/16864>.
* src/context.c (find_hunk): Threshold is CONTEXT only if
the second change is ignorable.
* tests/ignore-matching-lines: New test.
* tests/Makefile.am (TESTS): Add it.
---
src/context.c | 7 +++----
tests/Makefile.am | 1 +
tests/ignore-matching-lines | 47 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+), 4 deletions(-)
create mode 100755 tests/ignore-matching-lines
diff --git a/src/context.c b/src/context.c
index dd79f89..42f1eed 100644
--- a/src/context.c
+++ b/src/context.c
@@ -402,9 +402,8 @@ find_hunk (struct change *start)
lin top0, top1;
lin thresh;
- /* Threshold distance is 2 * CONTEXT + 1 between two non-ignorable
- changes, but only CONTEXT if one is ignorable. Watch out for
- integer overflow, though. */
+ /* Threshold distance is CONTEXT if the second change is ignorable,
+ 2 * CONTEXT + 1 otherwise. Watch out for integer overflow. */
lin non_ignorable_threshold =
(LIN_MAX - 1) / 2 < context ? LIN_MAX : 2 * context + 1;
lin ignorable_threshold = context;
@@ -416,7 +415,7 @@ find_hunk (struct change *start)
top1 = start->line1 + start->inserted;
prev = start;
start = start->link;
- thresh = (prev->ignore || (start && start->ignore)
+ thresh = (start && start->ignore
? ignorable_threshold
: non_ignorable_threshold);
/* It is not supposed to matter which file we check in the end-test.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index dd2d514..005d9f0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -7,6 +7,7 @@ TESTS = \
excess-slash \
help-version \
function-line-vs-leading-space \
+ ignore-matching-lines \
label-vs-func \
new-file \
no-dereference \
diff --git a/tests/ignore-matching-lines b/tests/ignore-matching-lines
new file mode 100755
index 0000000..5db9ba3
--- /dev/null
+++ b/tests/ignore-matching-lines
@@ -0,0 +1,47 @@
+#!/bin/sh
+# --ignore-matching-lines
+
+# Bug reported by Vincent Lefevre in <http://bugs.gnu.org/16864>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+fail=0
+
+cat <<'EOF' >a
+1a
+2
+3a
+4
+5
+6
+EOF
+
+cat <<'EOF' >b
+1b
+2
+3b
+4
+5
+6
+7
+EOF
+
+cat <<'EOF' >exp
+@@ -1,6 +1,7 @@
+-1a
++1b
+ 2
+-3a
++3b
+ 4
+ 5
+ 6
++7
+EOF
+
+diff -u --ignore-matching-lines 3 a b >out 2>err
+test $? = 1 || fail=1
+sed 1,2d out >outtail || framework_failure+
+compare exp outtail || fail=1
+
+Exit $fail
--
1.8.5.3
From 3b7b9766119b8d19eec2d73d40968fd662b60aa6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Mon, 24 Feb 2014 21:56:21 -0800
Subject: [PATCH 2/2] diff, sdiff: minor integer overflow fixes
* src/context.c (find_hunk):
Simplify, now that 2 * context + 1 cannot overflow.
* src/diff.c (main):
* src/sdiff.c (interact):
Don't rely on undefined behavior on signed integer overflow.
* src/diff.c (main): Don't let contexts exceed CONTEXT_MAX.
* src/system.h (CONTEXT_MAX): New macro.
---
src/context.c | 6 +++---
src/diff.c | 15 ++++++++-------
src/sdiff.c | 10 ++++++----
src/system.h | 5 +++++
4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/context.c b/src/context.c
index 42f1eed..32053d1 100644
--- a/src/context.c
+++ b/src/context.c
@@ -403,10 +403,10 @@ find_hunk (struct change *start)
lin thresh;
/* Threshold distance is CONTEXT if the second change is ignorable,
- 2 * CONTEXT + 1 otherwise. Watch out for integer overflow. */
- lin non_ignorable_threshold =
- (LIN_MAX - 1) / 2 < context ? LIN_MAX : 2 * context + 1;
+ 2 * CONTEXT + 1 otherwise. Integer overflow can't happen, due
+ to CONTEXT_LIM. */
lin ignorable_threshold = context;
+ lin non_ignorable_threshold = 2 * context + 1;
do
{
diff --git a/src/diff.c b/src/diff.c
index 50d0365..c6ba5f6 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -304,11 +304,12 @@ main (int argc, char **argv)
case '7':
case '8':
case '9':
- if (! ISDIGIT (prev))
- ocontext = c - '0';
- else if (LIN_MAX / 10 < ocontext
- || ((ocontext = 10 * ocontext + c - '0') < 0))
- ocontext = LIN_MAX;
+ ocontext = (! ISDIGIT (prev)
+ ? c - '0'
+ : (ocontext - (c - '0' <= CONTEXT_MAX % 10)
+ < CONTEXT_MAX / 10)
+ ? 10 * ocontext + (c - '0')
+ : CONTEXT_MAX);
break;
case 'a':
@@ -337,8 +338,8 @@ main (int argc, char **argv)
numval = strtoumax (optarg, &numend, 10);
if (*numend)
try_help ("invalid context length '%s'", optarg);
- if (LIN_MAX < numval)
- numval = LIN_MAX;
+ if (CONTEXT_MAX < numval)
+ numval = CONTEXT_MAX;
}
else
numval = 3;
diff --git a/src/sdiff.c b/src/sdiff.c
index e7bc657..329fa52 100644
--- a/src/sdiff.c
+++ b/src/sdiff.c
@@ -1099,12 +1099,14 @@ interact (struct line_filter *diff,
uintmax_t val;
lin llen, rlen, lenmax;
errno = 0;
- llen = val = strtoumax (diff_help + 1, &numend, 10);
- if (llen < 0 || llen != val || errno || *numend != ',')
+ val = strtoumax (diff_help + 1, &numend, 10);
+ if (LIN_MAX < val || errno || *numend != ',')
fatal (diff_help);
- rlen = val = strtoumax (numend + 1, &numend, 10);
- if (rlen < 0 || rlen != val || errno || *numend)
+ llen = val;
+ val = strtoumax (numend + 1, &numend, 10);
+ if (LIN_MAX < val || errno || *numend)
fatal (diff_help);
+ rlen = val;
lenmax = MAX (llen, rlen);
diff --git a/src/system.h b/src/system.h
index f39fff0..1f81a72 100644
--- a/src/system.h
+++ b/src/system.h
@@ -135,6 +135,11 @@ typedef ptrdiff_t lin;
verify (TYPE_SIGNED (lin));
verify (sizeof (ptrdiff_t) <= sizeof (lin));
verify (sizeof (lin) <= sizeof (long int));
+
+/* Limit so that 2 * CONTEXT + 1 does not overflow. */
+
+#define CONTEXT_MAX ((LIN_MAX - 1) / 2)
+
/* This section contains POSIX-compliant defaults for macros
that are meant to be overridden by hand in config.h as needed. */
--
1.8.5.3
