Improved impelmentation(and cleanup behaviro with errors conditions)

5 files changed, 93 insertions, 110 deletions

diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c
index 3f1d2529e2..57b0eb921e 100644
--- a/ext/pcre/php_pcre.c
+++ b/ext/pcre/php_pcre.c
@@ -1353,38 +1353,18 @@ static zend_string *php_replace_in_subject(zval *regex, zval *replace, zval *sub
 
 /* {{{ preg_replace_impl
  */
-static void preg_replace_impl(zval *return_value, int argc, zval *regex, zval *replace, zval *subject, zend_long limit, zval *zcount, int is_callable_replace, int is_filter)
+static int preg_replace_impl(zval *return_value, zval *regex, zval *replace, zval *subject, zend_long limit_val, int is_callable_replace, int is_filter)
 {
-	zval			*subject_entry;
-	int				 limit_val = -1;
-	zend_string		*result;
-	zend_string		*string_key;
-	zend_ulong		 num_key;
-	zend_string		*callback_name;
-	int				 replace_count=0, old_replace_count;
-
-	if (!is_callable_replace && Z_TYPE_P(replace) == IS_ARRAY && Z_TYPE_P(regex) != IS_ARRAY) {
-		php_error_docref(NULL, E_WARNING, "Parameter mismatch, pattern is a string while replacement is an array");
-		RETURN_FALSE;
-	}
+	zval		*subject_entry;
+	zend_string	*result;
+	zend_string	*string_key;
+	zend_ulong	 num_key;
+	int			 replace_count = 0, old_replace_count;
 
 	if (Z_TYPE_P(replace) != IS_ARRAY && (Z_TYPE_P(replace) != IS_OBJECT || !is_callable_replace)) {
 		SEPARATE_ZVAL(replace);
 		convert_to_string_ex(replace);
 	}
-	if (is_callable_replace) {
-		if (!zend_is_callable(replace, 0, &callback_name)) {
-			php_error_docref(NULL, E_WARNING, "Requires argument 2, '%s', to be a valid callback", callback_name->val);
-			zend_string_release(callback_name);
-			ZVAL_DUP(return_value, subject);
-			return;
-		}
-		zend_string_release(callback_name);
-	}
-
-	if (argc > 3) {
-		limit_val = (int)limit;
-	}
 
 	if (Z_TYPE_P(regex) != IS_ARRAY) {
 		SEPARATE_ZVAL(regex);
@@ -1423,10 +1403,8 @@ static void preg_replace_impl(zval *return_value, int argc, zval *regex, zval *r
 			}
 		}
 	}
-	if (argc > 4) {
-		zval_dtor(zcount);
-		ZVAL_LONG(zcount, replace_count);
-	}
+	
+	return replace_count;
 }
 /* }}} */
 
@@ -1436,6 +1414,7 @@ static PHP_FUNCTION(preg_replace)
 {
 	zval *regex, *replace, *subject, *zcount = NULL;
 	zend_long limit = -1;
+	int replace_count;
 
 #ifndef FAST_ZPP
 	/* Get function parameters and do error-checking. */
@@ -1453,7 +1432,16 @@ static PHP_FUNCTION(preg_replace)
 	ZEND_PARSE_PARAMETERS_END();
 #endif
 
-	preg_replace_impl(return_value, ZEND_NUM_ARGS(), regex, replace, subject, limit, zcount, 0, 0);
+	if (Z_TYPE_P(replace) == IS_ARRAY && Z_TYPE_P(regex) != IS_ARRAY) {
+		php_error_docref(NULL, E_WARNING, "Parameter mismatch, pattern is a string while replacement is an array");
+		RETURN_FALSE;
+	}
+
+	replace_count = preg_replace_impl(return_value, regex, replace, subject, limit, 0, 0);
+	if (zcount) {
+		zval_dtor(zcount);
+		ZVAL_LONG(zcount, replace_count);
+	}
 }
 /* }}} */
 
@@ -1463,6 +1451,8 @@ static PHP_FUNCTION(preg_replace_callback)
 {
 	zval *regex, *replace, *subject, *zcount = NULL;
 	zend_long limit = -1;
+	zend_string	*callback_name;
+	int replace_count;
 
 #ifndef FAST_ZPP
 	/* Get function parameters and do error-checking. */
@@ -1480,7 +1470,19 @@ static PHP_FUNCTION(preg_replace_callback)
 	ZEND_PARSE_PARAMETERS_END();
 #endif
 
-	preg_replace_impl(return_value, ZEND_NUM_ARGS(), regex, replace, subject, limit, zcount, 1, 0);
+	if (!zend_is_callable(replace, 0, &callback_name)) {
+		php_error_docref(NULL, E_WARNING, "Requires argument 2, '%s', to be a valid callback", callback_name->val);
+		zend_string_release(callback_name);
+		ZVAL_COPY(return_value, subject);
+		return;
+	}
+	zend_string_release(callback_name);
+
+	replace_count = preg_replace_impl(return_value, regex, replace, subject, limit, 1, 0);
+	if (zcount) {
+		zval_dtor(zcount);
+		ZVAL_LONG(zcount, replace_count);
+	}
 }
 /* }}} */
 
@@ -1488,11 +1490,12 @@ static PHP_FUNCTION(preg_replace_callback)
    Perform Perl-style regular expression replacement using replacement callback. */
 static PHP_FUNCTION(preg_replace_callback_array)
 {
-	zval regex, tmp_ret, *replace, *subject, *pattern, *zcount = NULL;
+	zval regex, zv, *replace, *subject, *pattern, *zcount = NULL;
 	zend_long limit = -1;
-	zend_ulong num_idx, count = 0;
+	zend_ulong num_idx;
 	zend_string *str_idx;
-	int argc;
+	zend_string *callback_name;
+	int replace_count = 0;
 
 #ifndef FAST_ZPP
 	/* Get function parameters and do error-checking. */
@@ -1508,7 +1511,6 @@ static PHP_FUNCTION(preg_replace_callback_array)
 		Z_PARAM_ZVAL_EX(zcount, 0, 1)
 	ZEND_PARSE_PARAMETERS_END();
 #endif
-	argc = ZEND_NUM_ARGS();
 	
 	ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(pattern), num_idx, str_idx, replace) {
 		if (str_idx) {
@@ -1516,28 +1518,32 @@ static PHP_FUNCTION(preg_replace_callback_array)
 		} else {
 			php_error_docref(NULL, E_WARNING, "Delimiter must not be alphanumeric or backslash");
 			zval_dtor(return_value);
-			ZVAL_COPY(return_value, subject);
-			break;
+			RETURN_NULL();
 		}		
 
+		if (!zend_is_callable(replace, 0, &callback_name)) {
+			php_error_docref(NULL, E_WARNING, "'%s' is not a valid callback", callback_name->val);
+			zend_string_release(callback_name);
+			zval_dtor(return_value);
+			ZVAL_COPY(return_value, subject);
+			return;
+		}
+		zend_string_release(callback_name);
+
 		if (Z_ISNULL_P(return_value)) {
-			preg_replace_impl(&tmp_ret, argc + 1, &regex, replace, subject, limit, zcount, 1, 0);
+			replace_count += preg_replace_impl(&zv, &regex, replace, subject, limit, 1, 0);
 		} else {
-			preg_replace_impl(&tmp_ret, argc + 1, &regex, replace, return_value, limit, zcount, 1, 0);
+			replace_count += preg_replace_impl(&zv, &regex, replace, return_value, limit, 1, 0);
 			zval_ptr_dtor(return_value);
 		}
 
-		if (zcount && Z_TYPE_P(zcount) == IS_LONG) {
-			count += Z_LVAL_P(zcount);
-		}
-
-		ZVAL_COPY_VALUE(return_value, &tmp_ret);
+		ZVAL_COPY_VALUE(return_value, &zv);
 		zval_ptr_dtor(&regex);
 	} ZEND_HASH_FOREACH_END();
 
-	if (zcount && Z_TYPE_P(zcount) == IS_LONG) {
+	if (zcount) {
 		zval_dtor(zcount);
-		ZVAL_LONG(zcount, count);
+		ZVAL_LONG(zcount, replace_count);
 	}
 }
 /* }}} */
@@ -1548,6 +1554,7 @@ static PHP_FUNCTION(preg_filter)
 {
 	zval *regex, *replace, *subject, *zcount = NULL;
 	zend_long limit = -1;
+	int replace_count;
 
 #ifndef FAST_ZPP
 	/* Get function parameters and do error-checking. */
@@ -1565,7 +1572,16 @@ static PHP_FUNCTION(preg_filter)
 	ZEND_PARSE_PARAMETERS_END();
 #endif
 
-	preg_replace_impl(return_value, ZEND_NUM_ARGS(), regex, replace, subject, limit, zcount, 0, 1);
+	if (Z_TYPE_P(replace) == IS_ARRAY && Z_TYPE_P(regex) != IS_ARRAY) {
+		php_error_docref(NULL, E_WARNING, "Parameter mismatch, pattern is a string while replacement is an array");
+		RETURN_FALSE;
+	}
+
+	replace_count = preg_replace_impl(return_value, regex, replace, subject, limit, 0, 1);
+	if (zcount) {
+		zval_dtor(zcount);
+		ZVAL_LONG(zcount, replace_count);
+	}
 }
 /* }}} */
 
@@ -2077,16 +2093,16 @@ ZEND_END_ARG_INFO()
 /* }}} */
 
 static const zend_function_entry pcre_functions[] = {
-	PHP_FE(preg_match,				arginfo_preg_match)
-	PHP_FE(preg_match_all,			arginfo_preg_match_all)
-	PHP_FE(preg_replace,			arginfo_preg_replace)
-	PHP_FE(preg_replace_callback,	arginfo_preg_replace_callback)
+	PHP_FE(preg_match,					arginfo_preg_match)
+	PHP_FE(preg_match_all,				arginfo_preg_match_all)
+	PHP_FE(preg_replace,				arginfo_preg_replace)
+	PHP_FE(preg_replace_callback,		arginfo_preg_replace_callback)
 	PHP_FE(preg_replace_callback_array,	arginfo_preg_replace_callback_array)
-	PHP_FE(preg_filter,				arginfo_preg_replace)
-	PHP_FE(preg_split,				arginfo_preg_split)
-	PHP_FE(preg_quote,				arginfo_preg_quote)
-	PHP_FE(preg_grep,				arginfo_preg_grep)
-	PHP_FE(preg_last_error,			arginfo_preg_last_error)
+	PHP_FE(preg_filter,					arginfo_preg_replace)
+	PHP_FE(preg_split,					arginfo_preg_split)
+	PHP_FE(preg_quote,					arginfo_preg_quote)
+	PHP_FE(preg_grep,					arginfo_preg_grep)
+	PHP_FE(preg_last_error,				arginfo_preg_last_error)
 	PHP_FE_END
 };
 
diff --git a/ext/pcre/tests/preg_replace_callback_array.phpt b/ext/pcre/tests/preg_replace_callback_array.phpt
index 04ee8465ea..529f704db6 100644
--- a/ext/pcre/tests/preg_replace_callback_array.phpt
+++ b/ext/pcre/tests/preg_replace_callback_array.phpt
@@ -1,5 +1,5 @@
 --TEST--
-preg_replace_callback_array()
+preg_replace_callback_array() basic functions
 --FILE--
 <?php
 
diff --git a/ext/pcre/tests/preg_replace_callback_array2.phpt b/ext/pcre/tests/preg_replace_callback_array2.phpt
index dc6cbde85c..c3d3acf0a8 100644
--- a/ext/pcre/tests/preg_replace_callback_array2.phpt
+++ b/ext/pcre/tests/preg_replace_callback_array2.phpt
@@ -1,5 +1,5 @@
 --TEST--
-preg_replace_callback_array() 2
+preg_replace_callback_array() errors
 --FILE--
 <?php
 
@@ -12,7 +12,12 @@ var_dump(preg_replace_callback_array(1,2,3,$a));
 $a = "";
 var_dump(preg_replace_callback_array(array("" => ""),"","",$a));
 $a = array();
-var_dump(preg_replace_callback($a,$a,$a,$a));
+$b = "";
+var_dump(preg_replace_callback($a, $a, $a, $a, $b));
+var_dump($b);
+$b = "";
+var_dump(preg_replace_callback_array(array("xx" => "s"), $a, -1, $b));
+var_dump($b);
 
 echo "Done\n";
 ?>
@@ -37,4 +42,10 @@ NULL
 
 Warning: preg_replace_callback() expects parameter 4 to be integer, array given in %s on line %d
 NULL
+string(0) ""
+
+Warning: preg_replace_callback_array(): 's' is not a valid callback in %s on line %d
+array(0) {
+}
+string(0) ""
 Done
diff --git a/ext/pcre/tests/preg_replace_callback_array3.phpt b/ext/pcre/tests/preg_replace_callback_array3.phpt
index b0834327ff..ace1e88999 100644
--- a/ext/pcre/tests/preg_replace_callback_array3.phpt
+++ b/ext/pcre/tests/preg_replace_callback_array3.phpt
@@ -1,5 +1,5 @@
 --TEST--
-preg_replace_callback_array() 3
+preg_replace_callback_array() multiply callbacks
 --FILE--
 <?php
 $code = "test-EXECUTE_DATA-xcvxcv-ZEND_VM_DISPATCH_TO_HELPER";
@@ -19,21 +19,17 @@ $code = array("test-EXECUTE_DATA-ZEND_VM_DISPATCH_TO_HELPER_EX-test",
 
 $code = preg_replace_callback_array(
                 array(
-                    "/EXECUTE_DATA/m" => function($matches) { return "execute_data";},
                     "/ZEND_VM_DISPATCH_TO_HANDLER/m" => function($matches) { return "handler"; },
+                    23234 => function($matches) { return "execute_data";},
                     "/ZEND_VM_DISPATCH_TO_HELPER/m" => function($matches) { return "helper"; },
                 ),
                 $code, -1, $count);
 
 var_dump($code, $count);
-
 ?>
 --EXPECTF--	
 string(31) "test-execute_data-xcvxcv-helper"
-array(2) {
-  [0]=>
-  string(32) "test-execute_data-helper_EX-test"
-  [1]=>
-  string(48) "test-sdf-xcvxcv-helper_EX-test-execute_data-test"
-}
-int(4)
+
+Warning: preg_replace_callback_array(): Delimiter must not be alphanumeric or backslash in %s on line %d
+NULL
+NULL
diff --git a/ext/pcre/tests/preg_replace_callback_array4.phpt b/ext/pcre/tests/preg_replace_callback_array4.phpt
deleted file mode 100644
index 12065ab36b..0000000000
--- a/ext/pcre/tests/preg_replace_callback_array4.phpt
+++ /dev/null
@@ -1,40 +0,0 @@
---TEST--
-preg_replace_callback_array() 4
---FILE--
-<?php
-$code = "test-EXECUTE_DATA-xcvxcv-ZEND_VM_DISPATCH_TO_HELPER";
-
-$code = preg_replace_callback_array(
-                array(
-                    "/EXECUTE_DATA/m" => function($matches) { return "execute_data";},
-                    "/ZEND_VM_DISPATCH_TO_HANDLER/m" => function($matches) { return "handler"; },
-                    "/ZEND_VM_DISPATCH_TO_HELPER/m" => function($matches) { return "helper"; },
-                    "/ZEND_VM_DISPATCH_TO_HELPER_EX/m" => function($matches) { return "helper_ex"; },
-                ),
-                $code);
-
-var_dump($code);
-$code = array("test-EXECUTE_DATA-ZEND_VM_DISPATCH_TO_HELPER_EX-test",
-                "test-sdf-xcvxcv-ZEND_VM_DISPATCH_TO_HELPER_EX-test-EXECUTE_DATA-test");
-
-$code = preg_replace_callback_array(
-                array(
-                    "/ZEND_VM_DISPATCH_TO_HANDLER/m" => function($matches) { return "handler"; },
-                    23234 => function($matches) { return "execute_data";},
-                    "/ZEND_VM_DISPATCH_TO_HELPER/m" => function($matches) { return "helper"; },
-                ),
-                $code, -1, $count);
-
-var_dump($code, $count);
-?>
---EXPECTF--	
-string(31) "test-execute_data-xcvxcv-helper"
-
-Warning: preg_replace_callback_array(): Delimiter must not be alphanumeric or backslash in %s on line %d
-array(2) {
-  [0]=>
-  string(52) "test-EXECUTE_DATA-ZEND_VM_DISPATCH_TO_HELPER_EX-test"
-  [1]=>
-  string(68) "test-sdf-xcvxcv-ZEND_VM_DISPATCH_TO_HELPER_EX-test-EXECUTE_DATA-test"
-}
-int(0)