bug-grep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] fix hang in grep -F for empty string search


From: Paolo Bonzini
Subject: [PATCH] fix hang in grep -F for empty string search
Date: Wed, 31 Mar 2010 10:10:19 +0200

Here is the patch to improve the testsuite coverage for empty string
search, and fix the bug.

The bug is due to is_mb_middle returning true for match_len = 0 (Jim's
change) but it was latent until my patch to fix SJIS, which does
"beg += mb_len - 1" even when mb_len = 0.

Paolo Bonzini (5):
  tests: convert empty.sh to new style
  tests: rename empty.sh to empty
  grep: fix grep -F against empty string
  tests: improve empty test with respect to locales
  tests: improve empty test

 NEWS              |    3 ++
 src/searchutils.c |    7 ++++-
 tests/Makefile.am |    2 +-
 tests/empty       |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/empty.sh    |   39 ------------------------
 5 files changed, 94 insertions(+), 41 deletions(-)
 create mode 100755 tests/empty
 delete mode 100755 tests/empty.sh

>From 3330f81932ddd28fde8c2e1bd1fb50a64207c680 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <address@hidden>
Date: Wed, 31 Mar 2010 08:32:55 +0200
Subject: [PATCH 1/5] tests: convert empty.sh to new style

* tests/empty.sh: Convert to init.sh, add 10 seconds timeout.
---
 tests/empty.sh |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/empty.sh b/tests/empty.sh
index b677aa3..9bb78b3 100755
--- a/tests/empty.sh
+++ b/tests/empty.sh
@@ -9,31 +9,34 @@
 # notice and this notice are preserved.
 
 : ${srcdir=.}
+. "$srcdir/init.sh"; path_prepend_ ../src
+
+require_timeout_
 
 failures=0
 
 for options in '-E' '-E -w' '-F -x' '-G -w -x'; do
 
        # should return 0 found a match
-       echo "" | ${GREP} $options -e '' > /dev/null 2>&1
+       echo "" | timeout 10s grep $options -e ''
        if test $? -ne 0 ; then
                echo "Status: Wrong status code, test \#1 failed ($options)"
                failures=1
        fi
 
        # should return 1 found no match
-       echo "abcd" | ${GREP} $options -f /dev/null  > /dev/null 2>&1
+       echo "abcd" | timeout 10s grep $options -f /dev/null
        if test $? -ne 1 ; then
                echo "Status: Wrong status code, test \#2 failed ($options)"
                failures=1
        fi
 
        # should return 0 found a match
-       echo "abcd" | ${GREP} $options -f /dev/null -e "abcd" > /dev/null 2>&1
+       echo "abcd" | timeout 10s grep $options -f /dev/null -e "abcd"
        if test $? -ne 0 ; then
                echo "Status: Wrong status code, test \#3 failed ($options)"
                failures=1
        fi
 done
 
-exit $failures
+Exit $failures
-- 
1.6.6.1


>From 4e03d908a468ebcc2b71b0b157a3c94976a81552 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <address@hidden>
Date: Wed, 31 Mar 2010 08:33:52 +0200
Subject: [PATCH 2/5] tests: rename empty.sh to empty

* tests/empty.sh: Rename to...
* tests/empty: ... this.
* tests/Makefile.am (TESTS): Adjust.
---
 tests/Makefile.am         |    2 +-
 tests/{empty.sh => empty} |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename tests/{empty.sh => empty} (100%)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 71f47c3..242dd6b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -33,7 +33,7 @@ TESTS =                                               \
   case-fold-char-type                          \
   char-class-multibyte                         \
   dfaexec-multibyte                            \
-  empty.sh                                     \
+  empty                                                \
   ere.sh                                       \
   euc-mb                                       \
   fedora                                       \
diff --git a/tests/empty.sh b/tests/empty
similarity index 100%
rename from tests/empty.sh
rename to tests/empty
-- 
1.6.6.1


>From 981c70b64e10b75c0533bb88919dfede6f22a20a Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <address@hidden>
Date: Wed, 31 Mar 2010 09:09:28 +0200
Subject: [PATCH 3/5] grep: fix grep -F against empty string

* src/searchutils.c (is_mb_middle): Do not return true for empty matches
when p == buf.
---
 NEWS              |    3 +++
 src/searchutils.c |    7 ++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index e822ea1..eb94184 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU grep NEWS                                    -*- outline -*-
 
 ** Bug fixes
 
+  Searching with grep -F for an empty string in a multibyte locale
+  would hang grep. [bug introduced in 2.6.2]
+
   PCRE support is once again detected on systems with <pcre/pcre.h>
   [bug introduced in 2.6.2]
 
diff --git a/src/searchutils.c b/src/searchutils.c
index 8c34e31..b04f36f 100644
--- a/src/searchutils.c
+++ b/src/searchutils.c
@@ -142,6 +142,11 @@ is_mb_middle (const char **good, const char *buf, const 
char *end,
     }
 
   *good = prev;
-  return p > buf || match_len < mbrlen (p, end - p, &cur_state);
+
+  if (p > buf)
+    return true;
+
+  /* P == BUF here.  */
+  return match_len > 0 && match_len < mbrlen (p, end - p, &cur_state);
 }
 #endif /* MBS_SUPPORT */
-- 
1.6.6.1


>From fdb5bc850033d31c7d3ca7c88fc957afdfd6f47f Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <address@hidden>
Date: Wed, 31 Mar 2010 08:56:55 +0200
Subject: [PATCH 4/5] tests: improve empty test with respect to locales

* tests/empty: Add tests for multiple locales.
---
 tests/empty |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/empty b/tests/empty
index 9bb78b3..cd0d3fa 100755
--- a/tests/empty
+++ b/tests/empty
@@ -15,28 +15,30 @@ require_timeout_
 
 failures=0
 
-for options in '-E' '-E -w' '-F -x' '-G -w -x'; do
+for locale in C en_US.UTF-8; do
+    for options in '-E' '-E -w' '-F -x' '-G -w -x'; do
 
        # should return 0 found a match
-       echo "" | timeout 10s grep $options -e ''
+       echo "" | LC_ALL=$locale timeout 10s grep $options -e ''
        if test $? -ne 0 ; then
-               echo "Status: Wrong status code, test \#1 failed ($options)"
+               echo "Status: Wrong status code, test \#1 failed ($options 
$locale)"
                failures=1
        fi
 
        # should return 1 found no match
-       echo "abcd" | timeout 10s grep $options -f /dev/null
+       echo "abcd" | LC_ALL=$locale timeout 10s grep $options -f /dev/null
        if test $? -ne 1 ; then
-               echo "Status: Wrong status code, test \#2 failed ($options)"
+               echo "Status: Wrong status code, test \#2 failed ($options 
$locale)"
                failures=1
        fi
 
        # should return 0 found a match
-       echo "abcd" | timeout 10s grep $options -f /dev/null -e "abcd"
+       echo "abcd" | LC_ALL=$locale timeout 10s grep $options -f /dev/null -e 
"abcd"
        if test $? -ne 0 ; then
-               echo "Status: Wrong status code, test \#3 failed ($options)"
+               echo "Status: Wrong status code, test \#3 failed ($options 
$locale)"
                failures=1
        fi
+    done
 done
 
 Exit $failures
-- 
1.6.6.1


>From 9e161077827db22c1f84167aec67bdedced08c23 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <address@hidden>
Date: Wed, 31 Mar 2010 08:42:43 +0200
Subject: [PATCH 5/5] tests: improve empty test

* tests/empty: Add more tests, note expected failure.
---
 tests/empty |   42 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/tests/empty b/tests/empty
index cd0d3fa..9e8f335 100755
--- a/tests/empty
+++ b/tests/empty
@@ -16,7 +16,7 @@ require_timeout_
 failures=0
 
 for locale in C en_US.UTF-8; do
-    for options in '-E' '-E -w' '-F -x' '-G -w -x'; do
+    for options in '-E' '-F'; do
 
        # should return 0 found a match
        echo "" | LC_ALL=$locale timeout 10s grep $options -e ''
@@ -38,6 +38,46 @@ for locale in C en_US.UTF-8; do
                echo "Status: Wrong status code, test \#3 failed ($options 
$locale)"
                failures=1
        fi
+
+       # should return 0 found a match
+       echo "" | LC_ALL=$locale timeout 10s grep $options -e ''
+       if test $? -ne 0 ; then
+               echo "Status: Wrong status code, test \#4 failed ($options 
$locale)"
+               failures=1
+       fi
+
+       # should return 0 found a match
+       echo "abcd" | LC_ALL=$locale timeout 10s grep $options -e ''
+       if test $? -ne 0 ; then
+               echo "Status: Wrong status code, test \#5 failed ($options 
$locale)"
+               failures=1
+       fi
+    done
+
+    # -F -w omitted because it fails test #6, will be revisited after 2.6
+    # stabilizes.
+    for options in '-E -w' '-E -x' '-E -w -x' '-F -x' '-F -w -x'; do
+
+       # should return 0 found a match
+       echo "" | LC_ALL=$locale timeout 10s grep $options -e ''
+       if test $? -ne 0 ; then
+               echo "Status: Wrong status code, test \#6 failed ($options 
$locale)"
+               failures=1
+       fi
+
+       # should return 1 found no match
+       echo "abcd" | LC_ALL=$locale timeout 10s grep $options -f /dev/null
+       if test $? -ne 1 ; then
+               echo "Status: Wrong status code, test \#7 failed ($options 
$locale)"
+               failures=1
+       fi
+
+       # should return 1 found no match
+       echo "abcd" | LC_ALL=$locale timeout 10s grep $options -f /dev/null -e 
""
+       if test $? -ne 1 ; then
+               echo "Status: Wrong status code, test \#8 failed ($options 
$locale)"
+               failures=1
+       fi
     done
 done
 
-- 
1.6.6.1





reply via email to

[Prev in Thread] Current Thread [Next in Thread]