strstr: fix a bug whereby strstr would mistakenly return NULL

* lib/str-two-way.h (two_way_short_needle): Correct off-by-one error
in period calculation.
(two_way_long_needle): Likewise.
Reported by Ralf Wildenhues, with the short needle and haystack.
* tests/test-strstr.c: Add Ralf's test case to trigger the bug.
Add a more involved test to trigger the bug in two_way_long_needle.

diff --git a/ChangeLog b/ChangeLog
index 139b38f746950417cf7801e312307544e077f5b2..7795cce5e22d9809718ae41aae480dfed831c5c5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-02-24  Jim Meyering  <meyering@redhat.com>
+
+       strstr: fix a bug whereby strstr would mistakenly return NULL
+       * lib/str-two-way.h (two_way_short_needle): Correct off-by-one error
+       in period calculation.
+       (two_way_long_needle): Likewise.
+       Reported by Ralf Wildenhues, with the short needle and haystack.
+       * tests/test-strstr.c: Add Ralf's test case to trigger the bug.
+       Add a more involved test to trigger the bug in two_way_long_needle.
+
 2011-02-24  Stefano Lattarini  <stefano.lattarini@gmail.com>  (tiny change)
 
        gnulib-tool: remove use of bold display in help screen

diff --git a/lib/str-two-way.h b/lib/str-two-way.h
index dd8097667d93d6b57ae755d8d7486a209c655713..317612c7b28845d28188378f1dd30e09f4c4ccbd 100644 (file)
--- 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 f63cb33c9ed8eaffb1ada90b00e7b1d9cacb1132..718ead7f0c0ef1ca1aa581cf4877895a977315ce 100644 (file)
--- a/tests/test-strstr.c
+++ b/tests/test-strstr.c
@@ -184,5 +184,84 @@ 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);
+  }
+
+  {
+    /* Like the above, but trigger the flaw in two_way_long_needle
+       by using a needle of length LONG_NEEDLE_THRESHOLD (32) or greater.
+       Rather than trying to find the right alignment manually, I've
+       arbitrarily chosen the following needle and template for the
+       haystack, and ensure that for each placement of the needle in
+       that haystack, strstr finds it.  */
+    const char *needle = "\nwith_gnu_ld-extend-to-len-32-b\n";
+    const char *h =
+      "\n"
+      "with_build_libsubdir\n"
+      "with_local_prefix\n"
+      "with_gxx_include_dir\n"
+      "with_cpp_install_dir\n"
+      "with_e_\n"
+      "..............................\n"
+      "with_FGHIJKLMNOPQRSTUVWXYZ\n"
+      "with_567890123456789\n" "with_multilib_list\n";
+    size_t h_len = strlen (h);
+    char *haystack = malloc (h_len + 1);
+    ASSERT (haystack);
+    size_t i;
+    for (i = 0; i < h_len - strlen (needle); i++)
+      {
+        memcpy (haystack, h, h_len + 1);
+        memcpy (haystack + i, needle, strlen (needle) + 1);
+        const char *p = strstr (haystack, needle);
+        ASSERT (p);
+        ASSERT (p - haystack == i);
+      }
+  }
+
   return 0;
 }