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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
