Re: bug in autoconf-2.64

[Jim Meyering]

Jim Meyering wrote:

> Ralf Wildenhues wrote:
>
>> [ this is http://thread.gmane.org/gmane.comp.sysutils.autoconf.bugs/7834
>>   from http://gcc.gnu.org/ml/gcc-patches/2011-02/msg01480.html
>>   adding bug-gnulib; followups can elide bug-autoconf ]
>>
>> * Ralf Wildenhues wrote on Thu, Feb 24, 2011 at 07:24:35AM CET:
>>> IOW, it looks like the replacement code in strstr.c and str-two-way.h
>>> has a bug (or glibc strchr, which seems rather unlikely).  Besides
>>> copyright year bumps, these two files have not been updated since in
>>> gnulib.
>>
>> Here's a reproducer.  Link with gnulib's strstr.c and it will fail.
>
> Wow.  Thanks for that, guys.
>
> Here's a fix that passes this test, which includes Ralf's test case:
>
>     ./gnulib-tool --create-testdir --with-tests --test strstr
>
> I haven't finished testing, and will add a comment attributing
> the test case.  Oh, and I'll make the test assertion stronger
> (to include the precise offset).

FYI, this patch also corrects the identical period = ...
assignment in two_way_long_needle, but I haven't yet added a test
to exercise that case.  That's next.

>From a3a2ee84e682f6be311bf5b23a6b3e8c80996b45 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 24 Feb 2011 10:57:22 +0100
Subject: [PATCH] strstr: fix a bug whereby strstr would mistakenly return NULL

* tests/test-strstr.c: Add a test case that triggers the bug.
Haystack and needle provided by Ralf Wildenhues.
* lib/str-two-way.h (two_way_short_needle): Correct off-by-one error
in period calculation.
(two_way_long_needle): Likewise.
---
 ChangeLog           |    9 +++++++++
 lib/str-two-way.h   |    4 ++--
 tests/test-strstr.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7d670a5..d1f9950 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2011-02-24  Jim Meyering  <address@hidden>
+
+       strstr: fix a bug whereby strstr would mistakenly return NULL
+       * tests/test-strstr.c: Add a test case that triggers the bug.
+       Haystack and needle provided by Ralf Wildenhues.
+       * lib/str-two-way.h (two_way_short_needle): Correct off-by-one error
+       in period calculation.
+       (two_way_long_needle): Likewise.
+
 2011-02-23  Bruno Haible  <address@hidden>

        Fix misindentation of preprocessor directives.
diff --git a/lib/str-two-way.h b/lib/str-two-way.h
index dd80976..317612c 100644
--- a/lib/str-two-way.h
+++ b/lib/str-two-way.h
@@ -284,7 +284,7 @@ two_way_short_needle (const unsigned char *haystack, size_t 
haystack_len,
     {
       /* The two halves of needle are distinct; no extra memory is
          required, and any mismatch results in a maximal shift.  */
-      period = MAX (suffix, needle_len - suffix) + 1;
+      period = MAX (suffix, needle_len - suffix);
       j = 0;
       while (AVAILABLE (haystack, haystack_len, j, needle_len))
         {
@@ -407,7 +407,7 @@ two_way_long_needle (const unsigned char *haystack, size_t 
haystack_len,
       /* The two halves of needle are distinct; no extra memory is
          required, and any mismatch results in a maximal shift.  */
       size_t shift;
-      period = MAX (suffix, needle_len - suffix) + 1;
+      period = MAX (suffix, needle_len - suffix);
       j = 0;
       while (AVAILABLE (haystack, haystack_len, j, needle_len))
         {
diff --git a/tests/test-strstr.c b/tests/test-strstr.c
index f63cb33..3bea406 100644
--- a/tests/test-strstr.c
+++ b/tests/test-strstr.c
@@ -184,5 +184,52 @@ main (int argc, char *argv[])
   /* Sublinear speed is only possible in memmem; strstr must examine
      every character of haystack to find its length.  */

+
+  {
+    /* Ensure that with a barely periodic "short" needle, strstr's
+       search does not mistakenly skip just past the match point.
+       This use of strstr would mistakenly return NULL before
+       gnulib v0.0-4927.  */
+    const char *haystack =
+      "\n"
+      "with_build_libsubdir\n"
+      "with_local_prefix\n"
+      "with_gxx_include_dir\n"
+      "with_cpp_install_dir\n"
+      "enable_generated_files_in_srcdir\n"
+      "with_gnu_ld\n"
+      "with_ld\n"
+      "with_demangler_in_ld\n"
+      "with_gnu_as\n"
+      "with_as\n"
+      "enable_largefile\n"
+      "enable_werror_always\n"
+      "enable_checking\n"
+      "enable_coverage\n"
+      "enable_gather_detailed_mem_stats\n"
+      "enable_build_with_cxx\n"
+      "with_stabs\n"
+      "enable_multilib\n"
+      "enable___cxa_atexit\n"
+      "enable_decimal_float\n"
+      "enable_fixed_point\n"
+      "enable_threads\n"
+      "enable_tls\n"
+      "enable_objc_gc\n"
+      "with_dwarf2\n"
+      "enable_shared\n"
+      "with_build_sysroot\n"
+      "with_sysroot\n"
+      "with_specs\n"
+      "with_pkgversion\n"
+      "with_bugurl\n"
+      "enable_languages\n"
+      "with_multilib_list\n";
+    const char *needle = "\n"
+      "with_gnu_ld\n";
+    const char* p = strstr (haystack, needle);
+    ASSERT (p - haystack == 114);
+  }
+
   return 0;
 }
--
1.7.4.1.16.g759e8