Address review comments

Author: kocsismate

ext/mbstring/mbstring.c

@@ -3499,7 +3499,7 @@ PHP_FUNCTION(mb_send_mail)
 		str_headers = php_trim(str_headers, NULL, 0, 2);
 	} else if (headers_ht) {
 		str_headers = php_mail_build_headers(headers_ht);
-		if (!str_headers) {
+		if (EG(exception)) {
 			RETURN_THROWS();
 		}
 	}

ext/standard/file.c

@@ -942,20 +942,23 @@ PHP_FUNCTION(popen)
 			mode_len--;
 		}
 	}
+#endif
 
 	/* Musl only partially validates the mode. Manually check it to ensure consistent behavior. */
-	if (mode_len != 1 || (*posix_mode != 'r' && *posix_mode != 'w')) {
-		zend_argument_value_error(2, "must be either \"r\" or \"w\"");
+	if (mode_len > 2 ||
+		(mode_len == 1 && (*posix_mode != 'r' && *posix_mode != 'w')) ||
+		(mode_len == 2 && (memcmp(posix_mode, "rb", 2) || memcmp(posix_mode, "wb", 2)))
+	) {
+		zend_argument_value_error(2, "must be one of \"r\", \"rb\", \"w\", or \"wb\"");
 		efree(posix_mode);
 		RETURN_THROWS();
 	}
-#endif
 
 	fp = VCWD_POPEN(command, posix_mode);
 	if (!fp) {
 		php_error_docref2(NULL, command, posix_mode, E_WARNING, "%s", strerror(errno));
 		efree(posix_mode);
-		RETURN_FALSE;
+		RETURN_THROWS();
 	}
 
 	stream = php_stream_fopen_from_pipe(fp, mode);

ext/standard/mail.c

@@ -134,11 +134,11 @@ static void php_mail_build_headers_elems(smart_str *s, zend_string *key, zval *v
 	ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(val), tmp_key, tmp_val) {
 		if (tmp_key) {
 			zend_type_error("Header \"%s\" must only contain numeric keys, \"%s\" found", ZSTR_VAL(key), ZSTR_VAL(tmp_key));
-			continue;
+			break;
 		}
 		if (Z_TYPE_P(tmp_val) != IS_STRING) {
 			zend_type_error("Header \"%s\" must only contain values of type string, %s found", ZSTR_VAL(key), zend_zval_type_name(tmp_val));
-			continue;
+			break;
 		}
 		php_mail_build_headers_elem(s, key, tmp_val);
 	} ZEND_HASH_FOREACH_END();
@@ -276,7 +276,7 @@ PHP_FUNCTION(mail)
 		headers_str = php_trim(headers_str, NULL, 0, 2);
 	} else if (headers_ht) {
 		headers_str = php_mail_build_headers(headers_ht);
-		if (!headers_str) {
+		if (EG(exception)) {
 			RETURN_THROWS();
 		}
 	}

ext/standard/password.c

@@ -109,7 +109,7 @@ static zend_string* php_password_make_salt(size_t length) /* {{{ */
 
 static zend_string* php_password_get_salt(zval *unused_, size_t required_salt_len, HashTable *options) {
 	if (options && zend_hash_str_exists(options, "salt", sizeof("salt") - 1)) {
-		php_error_docref(NULL, E_WARNING, "The \"salt\" option is ignored, since providing a custom salt is no longer supported");
+		php_error_docref(NULL, E_WARNING, "The \"salt\" option has been ignored, since providing a custom salt is no longer supported");
 	}
 
 	return php_password_make_salt(required_salt_len);

ext/standard/tests/file/popen_pclose_basic2.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Test popen() and pclose function
+--SKIPIF--
+<?php
+if (strtoupper( substr(PHP_OS, 0, 3) ) == 'SUN')
+  die("skip Not valid for Sun Solaris");
+
+if (strtoupper( substr(PHP_OS, 0, 3) ) == 'WIN')
+  die("skip Not valid for Windows");
+?>
+--FILE--
+<?php
+$file_path = __DIR__;
+
+$file_handle = fopen($file_path."/popen.tmp", "w");
+fclose($file_handle);
+
+$file_handle = fopen($file_path."/popen.tmp", "wb");
+fclose($file_handle);
+echo "--- Done ---";
+
+?>
+--CLEAN--
+<?php
+$file_path = __DIR__;
+unlink($file_path."/popen.tmp");
+?>
+--EXPECT--
+--- Done ---

ext/standard/tests/file/popen_pclose_error.phpt

@@ -1,36 +1,28 @@
 --TEST--
 Test popen() and pclose function: error conditions
---SKIPIF--
-<?php
-if (strtoupper( substr(PHP_OS, 0, 3) ) == 'SUN')
-  die("skip Not valid for Sun Solaris");
-
-if (strtoupper( substr(PHP_OS, 0, 3) ) == 'WIN')
-  die("skip Not valid for Windows");
-?>
 --FILE--
 <?php
-$file_path = __DIR__;
-echo "*** Testing for error conditions ***\n";
 
 try {
-    popen("abc.txt", "rw");   // Invalid mode Argument
+    popen("abc.txt", "x");
 } catch (ValueError $exception) {
     echo $exception->getMessage() . "\n";
 }
 
-$file_handle = fopen($file_path."/popen.tmp", "w");
-fclose($file_handle);
-echo "\n--- Done ---";
+try {
+    popen("abc.txt", "rw");
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+try {
+    popen("abc.txt", "rwb");
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 
-?>
---CLEAN--
-<?php
-$file_path = __DIR__;
-unlink($file_path."/popen.tmp");
 ?>
 --EXPECT--
-*** Testing for error conditions ***
-popen(): Argument #2 ($mode) must be either "r" or "w"
-
---- Done ---
+popen(): Argument #2 ($mode) must be one of "r", "rb", "w", or "wb"
+popen(): Argument #2 ($mode) must be one of "r", "rb", "w", or "wb"
+popen(): Argument #2 ($mode) must be one of "r", "rb", "w", or "wb"

ext/standard/tests/general_functions/bug41037.phpt

@@ -4,7 +4,7 @@ Bug #41037 (unregister_tick_function() inside the tick function crash PHP)
 <?php
 
 function a() {
-    echo "hello";
+    echo "hello\n";
     try {
         unregister_tick_function('a');
     } catch (Error $exception) {
@@ -19,6 +19,8 @@ declare (ticks=1) {
 }
 ?>
 --EXPECT--
-helloRegistered tick function cannot be unregistered while it is being executed
+hello
+Registered tick function cannot be unregistered while it is being executed
 Done
-helloRegistered tick function cannot be unregistered while it is being executed
+hello
+Registered tick function cannot be unregistered while it is being executed