Skip to content

Commit 3df306d

Browse files
kocsismatekocsismate
committed
Promote warnings to exceptions in ext/pcntl
Closes GH-6004
1 parent 4a98b64 commit 3df306d

9 files changed

+113
-34
lines changed

‎ext/pcntl/pcntl.c‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -892,9 +892,14 @@ PHP_FUNCTION(pcntl_signal)
892892
RETURN_THROWS();
893893
}
894894

895-
if (signo < 1 || signo >= NSIG) {
896-
php_error_docref(NULL, E_WARNING, "Invalid signal");
897-
RETURN_FALSE;
895+
if (signo < 1) {
896+
zend_argument_value_error(1, "must be greater than or equal to 1");
897+
RETURN_THROWS();
898+
}
899+
900+
if (signo >= NSIG) {
901+
zend_argument_value_error(1, "must be less than %d", NSIG);
902+
RETURN_THROWS();
898903
}
899904

900905
if (!PCNTL_G(spares)) {
@@ -920,8 +925,8 @@ PHP_FUNCTION(pcntl_signal)
920925
/* Special long value case for SIG_DFL and SIG_IGN */
921926
if (Z_TYPE_P(handle) == IS_LONG) {
922927
if (Z_LVAL_P(handle) != (zend_long) SIG_DFL && Z_LVAL_P(handle) != (zend_long) SIG_IGN) {
923-
php_error_docref(NULL, E_WARNING, "Invalid value for handle argument specified");
924-
RETURN_FALSE;
928+
zend_argument_value_error(2, "must be either SIG_DFL or SIG_IGN when an integer value is given");
929+
RETURN_THROWS();
925930
}
926931
if (php_signal(signo, (Sigfunc *) Z_LVAL_P(handle), (int) restart_syscalls) == (void *)SIG_ERR) {
927932
PCNTL_G(last_error) = errno;
@@ -935,10 +940,11 @@ PHP_FUNCTION(pcntl_signal)
935940
if (!zend_is_callable_ex(handle, NULL, 0, NULL, NULL, &error)) {
936941
zend_string *func_name = zend_get_callable_name(handle);
937942
PCNTL_G(last_error) = EINVAL;
938-
php_error_docref(NULL, E_WARNING, "Specified handler \"%s\" is not callable (%s)", ZSTR_VAL(func_name), error);
943+
944+
zend_argument_type_error(2, "must be of type callable|int, %s given", zend_zval_type_name(handle));
939945
zend_string_release_ex(func_name, 0);
940946
efree(error);
941-
RETURN_FALSE;
947+
RETURN_THROWS();
942948
}
943949
ZEND_ASSERT(!error);
944950

@@ -966,8 +972,8 @@ PHP_FUNCTION(pcntl_signal_get_handler)
966972
}
967973

968974
if (signo < 1 || signo > 32) {
969-
php_error_docref(NULL, E_WARNING, "Invalid signal");
970-
RETURN_FALSE;
975+
zend_argument_value_error(1, "must be between 1 and 32");
976+
RETURN_THROWS();
971977
}
972978

973979
if ((prev_handle = zend_hash_index_find(&PCNTL_G(php_signal_table), signo)) != NULL) {
@@ -1197,8 +1203,8 @@ PHP_FUNCTION(pcntl_getpriority)
11971203
php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno);
11981204
break;
11991205
case EINVAL:
1200-
php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno);
1201-
break;
1206+
zend_argument_value_error(2, "must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS");
1207+
RETURN_THROWS();
12021208
default:
12031209
php_error_docref(NULL, E_WARNING, "Unknown error %d has occurred", errno);
12041210
break;
@@ -1231,8 +1237,8 @@ PHP_FUNCTION(pcntl_setpriority)
12311237
php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno);
12321238
break;
12331239
case EINVAL:
1234-
php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno);
1235-
break;
1240+
zend_argument_value_error(3, "must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS");
1241+
RETURN_THROWS();
12361242
case EPERM:
12371243
php_error_docref(NULL, E_WARNING, "Error %d: A process was located, but neither its effective nor real user ID matched the effective user ID of the caller", errno);
12381244
break;
@@ -1400,19 +1406,18 @@ PHP_FUNCTION(pcntl_async_signals)
14001406
PHP_FUNCTION(pcntl_unshare)
14011407
{
14021408
zend_long flags;
1403-
int ret;
14041409

14051410
ZEND_PARSE_PARAMETERS_START(1, 1)
14061411
Z_PARAM_LONG(flags)
14071412
ZEND_PARSE_PARAMETERS_END();
14081413

1409-
ret = unshare(flags);
1410-
if (ret == -1) {
1414+
if (unshare(flags) == -1) {
14111415
PCNTL_G(last_error) = errno;
14121416
switch (errno) {
14131417
#ifdef EINVAL
14141418
case EINVAL:
1415-
php_error_docref(NULL, E_WARNING, "Error %d: Invalid flag specified", errno);
1419+
zend_argument_value_error(1, "must be a combination of CLONE_* flags");
1420+
RETURN_THROWS();
14161421
break;
14171422
#endif
14181423
#ifdef ENOMEM

‎ext/pcntl/pcntl.stub.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ function pcntl_wait(&$status, int $options = 0, &$rusage = []): int {}
1919
/** @param callable|int $handler */
2020
function pcntl_signal(int $signo, $handler, bool $restart_syscalls = true): bool {}
2121

22-
/** @return mixed */
22+
/** @return callable|int */
2323
function pcntl_signal_get_handler(int $signo) {}
2424

2525
function pcntl_signal_dispatch(): bool {}

‎ext/pcntl/pcntl_arginfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: df744f88533ce9b84864fa2aa4dd7a5b7373231d */
2+
* Stub hash: 306208d94ba3bf6f8112f868a332e99717bc07fa */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_fork, 0, 0, IS_LONG, 0)
55
ZEND_END_ARG_INFO()

‎ext/pcntl/tests/pcntl_getpriority_error.phpt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
pcntl_getpriority() - Wrong process identifier
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('pcntl')) {
6+
die('skip ext/pcntl not loaded');
7+
}
8+
if (!function_exists('pcntl_getpriority')) {
9+
die('skip pcntl_getpriority doesn\'t exist');
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
15+
try {
16+
pcntl_getpriority(null, 42);
17+
} catch (ValueError $exception) {
18+
echo $exception->getMessage() . "\n";
19+
}
20+
21+
?>
22+
--EXPECT--
23+
pcntl_getpriority(): Argument #2 ($process_identifier) must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS

‎ext/pcntl/tests/pcntl_setpriority_error.phpt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
pcntl_setpriority() - Wrong process identifier
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('pcntl')) {
6+
die('skip ext/pcntl not loaded');
7+
}
8+
if (!function_exists('pcntl_setpriority')) {
9+
die('skip pcntl_setpriority doesn\'t exist');
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
15+
try {
16+
pcntl_setpriority(0, null, 42);
17+
} catch (ValueError $exception) {
18+
echo $exception->getMessage() . "\n";
19+
}
20+
21+
?>
22+
--EXPECT--
23+
pcntl_setpriority(): Argument #3 ($process_identifier) must be one of PRIO_PGRP, PRIO_USER, or PRIO_PROCESS

‎ext/pcntl/tests/pcntl_signal.phpt‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,24 @@ posix_kill(posix_getpid(), SIGUSR1);
1818
pcntl_signal_dispatch();
1919

2020
var_dump(pcntl_signal(SIGALRM, SIG_IGN));
21-
var_dump(pcntl_signal(-1, -1));
22-
var_dump(pcntl_signal(-1, function(){}));
23-
var_dump(pcntl_signal(SIGALRM, "not callable"));
2421

22+
try {
23+
pcntl_signal(-1, -1);
24+
} catch (ValueError $exception) {
25+
echo $exception->getMessage() . "\n";
26+
}
27+
28+
try {
29+
pcntl_signal(-1, function(){});
30+
} catch (ValueError $exception) {
31+
echo $exception->getMessage() . "\n";
32+
}
33+
34+
try {
35+
pcntl_signal(SIGALRM, "not callable");
36+
} catch (TypeError $exception) {
37+
echo $exception->getMessage() . "\n";
38+
}
2539

2640
/* test freeing queue in RSHUTDOWN */
2741
posix_kill(posix_getpid(), SIGTERM);
@@ -31,13 +45,7 @@ echo "ok\n";
3145
signal dispatched
3246
got signal from %r\d+|nobody%r
3347
bool(true)
34-
35-
Warning: pcntl_signal(): Invalid signal %s
36-
bool(false)
37-
38-
Warning: pcntl_signal(): Invalid signal %s
39-
bool(false)
40-
41-
Warning: pcntl_signal(): Specified handler "not callable" is not callable (%s) in %s
42-
bool(false)
48+
pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1
49+
pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1
50+
pcntl_signal(): Argument #2 ($handler) must be of type callable|int, string given
4351
ok

‎ext/pcntl/tests/pcntl_unshare_01.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ if (!extension_loaded("posix")) die("skip posix extension not available");
77
if (!function_exists("pcntl_unshare")) die("skip pcntl_unshare is not available");
88
if (!defined("CLONE_NEWUSER")) die("skip flag unavailable");
99
if (@pcntl_unshare(CLONE_NEWUSER) == false && pcntl_get_last_error() == PCNTL_EPERM) {
10-
die("skip Insufficient previleges to use CLONE_NEWUSER");
10+
die("skip Insufficient privileges to use CLONE_NEWUSER");
1111
}
12-
12+
?>
1313
--FILE--
1414
<?php
1515

‎ext/pcntl/tests/pcntl_unshare_02.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ if (posix_getuid() !== 0 &&
1515
if (@pcntl_unshare(CLONE_NEWPID) == false && pcntl_get_last_error() == PCNTL_EPERM) {
1616
die("skip Insufficient privileges for CLONE_NEWPID");
1717
}
18-
18+
?>
1919
--FILE--
2020
<?php
2121

‎ext/pcntl/tests/pcntl_unshare_04.phpt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
pcntl_unshare() with wrong flag
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded("pcntl")) die("skip");
6+
if (!extension_loaded("posix")) die("skip posix extension not available");
7+
if (!function_exists("pcntl_unshare")) die("skip pcntl_unshare is not available");
8+
?>
9+
--FILE--
10+
<?php
11+
12+
try {
13+
pcntl_unshare(42);
14+
} catch (ValueError $exception) {
15+
echo $exception->getMessage() . "\n";
16+
}
17+
18+
?>
19+
--EXPECT--
20+
pcntl_unshare(): Argument #1 ($flags) must be a combination of CLONE_* flags

0 commit comments

Comments
 (0)