Refactor mb_substr_count()

Promote empty needle warning to ValueError
Convert if branch into an assertion as if mbfl_substr_count fails this now implies a bug
Thus mb_substr_count() can only return int now, fix stubs accordingly

diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c
index e8fb63e6754cf0465716f73e4e884b86f84e7ffc..16452b0a5b6b26deff84abc687ced31aac8ad869 100644 (file)
--- a/ext/mbstring/mbstring.c
+++ b/ext/mbstring/mbstring.c
@@ -2269,23 +2269,24 @@ PHP_FUNCTION(mb_substr_count)
                RETURN_THROWS();
        }
 
+       if (needle.len == 0) {
+               zend_argument_value_error(2, "must not be empty");
+               RETURN_THROWS();
+       }
+
        haystack.no_language = needle.no_language = MBSTRG(language);
        haystack.encoding = needle.encoding = php_mb_get_encoding(enc_name, 3);
        if (!haystack.encoding) {
                RETURN_THROWS();
        }
 
-       if (needle.len == 0) {
-               php_error_docref(NULL, E_WARNING, "Empty substring");
-               RETURN_FALSE;
-       }
-
        n = mbfl_substr_count(&haystack, &needle);
-       if (!mbfl_is_error(n)) {
-               RETVAL_LONG(n);
-       } else {
-               RETVAL_FALSE;
-       }
+       /* An error can only occur if needle is empty,
+        * an encoding error happens (which should not happen at this stage and is a bug)
+        * or the haystack is more than sizeof(size_t) bytes
+        * If one of these things occur this is a bug and should be flagged as such */
+       ZEND_ASSERT(!mbfl_is_error(n));
+       RETVAL_LONG(n);
 }
 /* }}} */
 

diff --git a/ext/mbstring/mbstring.stub.php b/ext/mbstring/mbstring.stub.php
index aa2c7dc0e4b6ca53bb5b7428c9054c5e68cf959d..dfc920eb9643569e045a477e58a08688ac17c35f 100644 (file)
--- a/ext/mbstring/mbstring.stub.php
+++ b/ext/mbstring/mbstring.stub.php
@@ -39,7 +39,7 @@ function mb_stristr(string $haystack, string $needle, bool $part = false, string
 
 function mb_strrichr(string $haystack, string $needle, bool $part = false, string $encoding = UNKNOWN): string|false {}
 
-function mb_substr_count(string $haystack, string $needle, string $encoding = UNKNOWN): int|false {}
+function mb_substr_count(string $haystack, string $needle, string $encoding = UNKNOWN): int {}
 
 function mb_substr(string $str, int $start, ?int $length = null, string $encoding = UNKNOWN): string|false {}
 

diff --git a/ext/mbstring/mbstring_arginfo.h b/ext/mbstring/mbstring_arginfo.h
index 09e277d37b3a60b9be1f245b39cbfbdd488aba38..ea6af386e3426033e197afff07efe147ed7c28b1 100644 (file)
--- a/ext/mbstring/mbstring_arginfo.h
+++ b/ext/mbstring/mbstring_arginfo.h
@@ -78,7 +78,7 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_mb_strrichr arginfo_mb_strstr
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_substr_count, 0, 2, MAY_BE_LONG|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_substr_count, 0, 2, IS_LONG, 0)
        ZEND_ARG_TYPE_INFO(0, haystack, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, needle, IS_STRING, 0)
        ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 0)

diff --git a/ext/mbstring/tests/mb_substr_count.phpt b/ext/mbstring/tests/mb_substr_count.phpt
index 84571515de9b30597ef2d50823f7faad6107400a..0d2962866fe2b5f02e583d67fb6dabc657c3322d 100644 (file)
--- a/ext/mbstring/tests/mb_substr_count.phpt
+++ b/ext/mbstring/tests/mb_substr_count.phpt
@@ -7,11 +7,20 @@ output_handler=
 --FILE--
 <?php
     mb_internal_encoding("EUC-JP");
-    var_dump(@mb_substr_count("", ""));
-    var_dump(@mb_substr_count("¤¢", ""));
-    var_dump(@mb_substr_count("", "¤¢"));
-    var_dump(@mb_substr_count("", "¤¢"));
-    var_dump(@mb_substr_count("", chr(0)));
+    try {
+        var_dump(mb_substr_count("", ""));
+    } catch (\ValueError $e) {
+        echo $e->getMessage() . \PHP_EOL;
+    }
+    try {
+        var_dump(mb_substr_count("��", ""));
+    } catch (\ValueError $e) {
+        echo $e->getMessage() . \PHP_EOL;
+    }
+
+    var_dump(mb_substr_count("", "¤¢"));
+    var_dump(mb_substr_count("", "¤¢"));
+    var_dump(mb_substr_count("", chr(0)));
 
     $a = str_repeat("abcacba", 100);
     var_dump(@mb_substr_count($a, "bca"));
@@ -32,8 +41,8 @@ output_handler=
     var_dump(@mb_substr_count($a, "bca"));
 ?>
 --EXPECT--
-bool(false)
-bool(false)
+mb_substr_count(): Argument #2 ($needle) must not be empty
+mb_substr_count(): Argument #2 ($needle) must not be empty
 int(0)
 int(0)
 int(0)