Subject: Re: [bug-diffutils] bug#16864: overlapping chunks with the -I option
From: Paul Eggert <[email-masked]>
To: Vincent Lefevre <[email-masked]>, [email-masked]
Date: Mon, 24 Feb 2014 22:01:05 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

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-masked]>
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-masked]>
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