Subject: bug#15773: grep-2.15 bug report

From: Jim Meyering <[email-masked]>
Date: Sat, 2 Nov 2013 11:32:39 -0700

On Sat, Nov 2, 2013 at 9:56 AM, Stefano Lattarini
<[email-masked]> wrote:
> On 11/02/2013 03:20 PM, Jim Meyering wrote:> On Fri, Nov 1, 2013 at 8:55 AM, Stefano Lattarini
>> <[email-masked]> wrote:
>>> This probably calls for a two patch series: the first introducing the test
>>> as
>>> an XFAIL, the second fixing the bug without touching the tests, and
>>> verifying
>>> that the test succeeds.
>>
>> That seems like overkill, and unnecessary churn in git.  Usually, once I
>> have a complete(including test case) and committed-but-not-pushed patch ,
>> I either arrange to run the test against the previous binary by replacing
>> src/grep with the grep from my path, or (probably better) temporarily
>> backing out the fix, e.g., with "git log -1 -p src/dfa.c|patch -R -p1"
>> and ensuring that "make check" fails.
>>
> This nit I pointed out was admittedly minor, and in large part a matter
> of personal preferences, so I have no problem with you disagreeing and
> ignoring it.
>
>>> Maybe you could even amend the test to run with all of the default locale,
>>> the
>>> en_US.UTF-8 locale, and the C locale.  Possibly overly paranoid, but the
>>> enhancement would be trivial, so why not get the extra coverage anyway?
>>
>> That seems worthwhile.
>> The default locale is set via tests/Makefile.am to LC_ALL=C, so I have
>> done this:
...
> I think you attached the wrong patch ;-)

Sigh, you're right.
Here's the intended one (along with a NEWS update):
From ecb7452cd420b7a2e0cca186e0d6666261c16c46 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Thu, 31 Oct 2013 21:10:02 -0700
Subject: [PATCH 1/2] maint: NEWS: document a release-related bug fix

* NEWS (Bug fixes): Add an entry for a fix pulled from gnulib.
---
 NEWS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/NEWS b/NEWS
index db9e365..161be50 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU grep NEWS                                    -*- outline -*-

 * Noteworthy changes in release ?.? (????-??-??) [?]

+** Bug fixes
+
+  Fix gnulib-provided maint.mk so that the release procedure described
+  in README-release actually does what we want.  Before that fix, that
+  procedure resulted in a grep-2.15 tarball that would lead to a grep
+  binary whose --version-reported version number was 2.14.51...
+

 * Noteworthy changes in release 2.15 (2013-10-26) [stable]

-- 
1.8.4.2.564.g0d6cf24

From c4bacd524ca1e7fec97543542d6179c25a3c1506 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Thu, 31 Oct 2013 20:20:30 -0700
Subject: [PATCH 2/2] grep: fix regression involving \s and \S

Commit v2.14-40-g01ec90b made \s and \S work with multi-byte
characters, but it made it so any use like \s*, \s+, \s?, \s{3}
would malfunction in a multi-byte locale.
* src/dfa.c (lex): Also reset laststart.
* tests/backslash-s-and-repetition-operators: New file.
* tests/Makefile.am (TESTS): Add it.
* NEWS (Bug fixes): Mention it.
* THANKS: Update.
Reported by Mirraz Mirraz in http://bugs.gnu.org/15773.
---
 NEWS                                       |  5 +++++
 THANKS                                     |  1 +
 src/dfa.c                                  |  1 +
 tests/Makefile.am                          |  1 +
 tests/backslash-s-and-repetition-operators | 36 ++++++++++++++++++++++++++++++
 5 files changed, 44 insertions(+)
 create mode 100755 tests/backslash-s-and-repetition-operators

diff --git a/NEWS b/NEWS
index 161be50..262b74b 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,11 @@ GNU grep NEWS                                    -*- outline -*-
   procedure resulted in a grep-2.15 tarball that would lead to a grep
   binary whose --version-reported version number was 2.14.51...

+  The fix to make \s and \S work with multi-byte white space broke
+  the use of each shortcut whenever followed by a repetition operator.
+  For example, \s*, \s+, \s? and \s{3} would all malfunction in a
+  multi-byte locale.  [bug introduced in grep-2.14]
+

 * Noteworthy changes in release 2.15 (2013-10-26) [stable]

diff --git a/THANKS b/THANKS
index 1a1901c..475c51e 100644
--- a/THANKS
+++ b/THANKS
@@ -66,6 +66,7 @@ Martin Rex                 <[email-masked]>
 Michael Aichlmayr          <[email-masked]>
 Mike Frysinger             <[email-masked]>
 Miles Bader                <[email-masked]>
+Mirraz Mirraz              <[email-masked]>
 Nelson H. F. Beebe         <[email-masked]>
 Olaf Kirch                 <[email-masked]>
 Paolo Bonzini              <[email-masked]>
diff --git a/src/dfa.c b/src/dfa.c
index de6c671..92c410e 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -1473,6 +1473,7 @@ lex (void)

           POP_LEX_STATE ();

+          laststart = 0;
           return lasttok;

         case 'w':
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a64a2d2..970a9de 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -36,6 +36,7 @@ TESTS =						\
   backref					\
   backref-multibyte-slow			\
   backref-word					\
+  backslash-s-and-repetition-operators		\
   backslash-s-vs-invalid-multitype		\
   big-hole					\
   big-match					\
diff --git a/tests/backslash-s-and-repetition-operators b/tests/backslash-s-and-repetition-operators
new file mode 100755
index 0000000..3b250d4
--- /dev/null
+++ b/tests/backslash-s-and-repetition-operators
@@ -0,0 +1,36 @@
+#! /bin/sh
+# Ensure that \s and \S work with repetition operators.
+#
+# Copyright (C) 2013 Free Software Foundation, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+require_en_utf8_locale_
+
+fail=0
+
+for loc in en_US.UTF-8 C; do
+  echo locale=$loc
+  LC_ALL=$loc
+  export LC_ALL
+
+  printf ' \n' > in || framework_failure_
+
+  for re in '\s\+' '\s*' '\s\?' '\s\{1\}'; do
+    grep "^$re\$" in > out || fail=1
+    compare in out || fail=1
+  done
+
+  printf 'X\n' > in || framework_failure_
+
+  for re in '\S\+' '\S*' '\S\?' '\S\{1\}'; do
+    grep "^$re\$" in > out || fail=1
+    compare in out || fail=1
+  done
+done
+
+Exit $fail
-- 
1.8.4.2.564.g0d6cf24