string-search robustness and documentation improvement (bug#43598)

* src/fns.c (Fstring_search): Check START-POS argument range.
Simplify logic.  Improve doc string.
* test/src/fns-tests.el (string-search): Add test cases.
* doc/lispref/strings.texi (Text Comparison): Elaborate.
* lisp/emacs-lisp/byte-opt.el (pure-fns): Mark string-search as pure.

doc/lispref/strings.texi

@@ -660,8 +660,10 @@ ignores case differences.
 Return the position of the first instance of @var{needle} in
 @var{haystack}, both of which are strings.  If @var{start-pos} is
 non-@code{nil}, start searching from that position in @var{needle}.
+Return @code{nil} if no match was found.
 This function only considers the characters in the strings when doing
-the comparison; text properties are ignored.
+the comparison; text properties are ignored.  Matching is always
+case-sensitive.
 @end defun
 
 @defun compare-strings string1 start1 end1 string2 start2 end2 &optional ignore-case

lisp/emacs-lisp/byte-opt.el

@@ -1264,6 +1264,7 @@
          floor ceiling round truncate
          ffloor fceiling fround ftruncate
          string= string-equal string< string-lessp
+         string-search
          consp atom listp nlistp propert-list-p
          sequencep arrayp vectorp stringp bool-vector-p hash-table-p
          null not

src/fns.c

@@ -5456,15 +5456,18 @@ It should not be used for anything security-related.  See
 
 DEFUN ("string-search", Fstring_search, Sstring_search, 2, 3, 0,
        doc: /* Search for the string NEEDLE in the string HAYSTACK.
-The return value is the position of the first instance of NEEDLE in
-HAYSTACK.
+The return value is the position of the first occurrence of NEEDLE in
+HAYSTACK, or nil if no match was found.
 
 The optional START-POS argument says where to start searching in
-HAYSTACK.  If not given, start at the beginning. */)
+HAYSTACK and defaults to zero (start at the beginning).
+It must be between zero and the length of HAYSTACK, inclusive.
+
+Case is always significant and text properties are ignored. */)
   (register Lisp_Object needle, Lisp_Object haystack, Lisp_Object start_pos)
 {
   ptrdiff_t start_byte = 0, haybytes;
-  char *res = NULL, *haystart;
+  char *res, *haystart;
 
   CHECK_STRING (needle);
   CHECK_STRING (haystack);
@@ -5472,7 +5475,10 @@ HAYSTACK.  If not given, start at the beginning. */)
   if (!NILP (start_pos))
     {
       CHECK_FIXNUM (start_pos);
-      start_byte = string_char_to_byte (haystack, XFIXNUM (start_pos));
+      EMACS_INT start = XFIXNUM (start_pos);
+      if (start < 0 || start > SCHARS (haystack))
+        xsignal1 (Qargs_out_of_range, start_pos);
+      start_byte = string_char_to_byte (haystack, start);
     }
 
   haystart = SSDATA (haystack) + start_byte;
@@ -5481,13 +5487,13 @@ HAYSTACK.  If not given, start at the beginning. */)
   if (STRING_MULTIBYTE (haystack) == STRING_MULTIBYTE (needle))
     res = memmem (haystart, haybytes,
 		  SSDATA (needle), SBYTES (needle));
-  else if (STRING_MULTIBYTE (haystack) && !STRING_MULTIBYTE (needle))
+  else if (STRING_MULTIBYTE (haystack))  /* unibyte needle */
     {
       Lisp_Object multi_needle = string_to_multibyte (needle);
       res = memmem (haystart, haybytes,
 		    SSDATA (multi_needle), SBYTES (multi_needle));
     }
-  else if (!STRING_MULTIBYTE (haystack) && STRING_MULTIBYTE (needle))
+  else                        /* unibyte haystack, multibyte needle */
     {
       Lisp_Object uni_needle = Fstring_as_unibyte (needle);
       res = memmem (haystart, haybytes,

test/src/fns-tests.el

@@ -907,6 +907,12 @@
   (should (equal (string-search "foo" "foobarzot") 0))
   (should (not (string-search "fooz" "foobarzot")))
   (should (not (string-search "zot" "foobarzo")))
+  (should (equal (string-search "ab" "ab") 0))
+  (should (equal (string-search "ab\0" "ab") nil))
+  (should (equal (string-search "ab" "abababab" 3) 4))
+  (should (equal (string-search "ab" "ababac" 3) nil))
+  (let ((case-fold-search t))
+    (should (equal (string-search "ab" "AB") nil)))
 
   (should (equal
            (string-search (make-string 2 130)
@@ -923,4 +929,26 @@
   (should (not (string-search (make-string 1 255) "a\377ø")))
   (should (not (string-search (make-string 1 255) "a\377a")))
 
-  (should (equal (string-search "fóo" "zotfóo") 3)))
+  (should (equal (string-search "fóo" "zotfóo") 3))
+
+  (should (equal (string-search (string-to-multibyte "\377") "ab\377c") 2))
+  (should (equal (string-search "\303" "aøb") nil))
+  (should (equal (string-search "\270" "aøb") nil))
+  ;; This test currently fails, but it shouldn't!
+  ;;(should (equal (string-search "ø" "\303\270") nil))
+
+  (should-error (string-search "a" "abc" -1))
+  (should-error (string-search "a" "abc" 4))
+  (should-error (string-search "a" "abc" 100000000000))
+
+  (should (equal (string-search "a" "aaa" 3) nil))
+  (should (equal (string-search "\0" "") nil))
+
+  (should (equal (string-search "" "") 0))
+  (should-error (string-search "" "" 1))
+  (should (equal (string-search "" "abc") 0))
+  (should (equal (string-search "" "abc" 2) 2))
+  (should (equal (string-search "" "abc" 3) 3))
+  (should-error (string-search "" "abc" 4))
+  (should-error (string-search "" "abc" -1))
+  )