Skip to content

Promote warnings to exceptions in ext/pcntl #6004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Promote warnings to exceptions in ext/pcntl #6004

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99c1b79
Promote warnings to exceptions in ext/pcntl
kocsismate Aug 17, 2020
4506432
Address code review
kocsismate Aug 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions ext/pcntl/pcntl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,9 +892,12 @@ PHP_FUNCTION(pcntl_signal)
RETURN_THROWS();
}

if (signo < 1 || signo >= NSIG) {
php_error_docref(NULL, E_WARNING, "Invalid signal");
RETURN_FALSE;
if (signo < 1) {
zend_argument_value_error(1, "must be greater than or equal to 1");
RETURN_THROWS();
} else if (signo >= NSIG) {
zend_argument_value_error(1, "must be less than %d", NSIG);
RETURN_THROWS();
}

if (!PCNTL_G(spares)) {
Expand All @@ -920,8 +923,8 @@ PHP_FUNCTION(pcntl_signal)
/* Special long value case for SIG_DFL and SIG_IGN */
if (Z_TYPE_P(handle) == IS_LONG) {
if (Z_LVAL_P(handle) != (zend_long) SIG_DFL && Z_LVAL_P(handle) != (zend_long) SIG_IGN) {
php_error_docref(NULL, E_WARNING, "Invalid value for handle argument specified");
RETURN_FALSE;
zend_argument_value_error(2, "must be either SIG_DFL or SIG_IGN when an integer value is given");
RETURN_THROWS();
}
if (php_signal(signo, (Sigfunc *) Z_LVAL_P(handle), (int) restart_syscalls) == (void *)SIG_ERR) {
PCNTL_G(last_error) = errno;
Expand All @@ -935,10 +938,11 @@ PHP_FUNCTION(pcntl_signal)
if (!zend_is_callable_ex(handle, NULL, 0, NULL, NULL, &error)) {
zend_string *func_name = zend_get_callable_name(handle);
PCNTL_G(last_error) = EINVAL;
php_error_docref(NULL, E_WARNING, "Specified handler \"%s\" is not callable (%s)", ZSTR_VAL(func_name), error);

zend_argument_type_error(2, "must be of type callable|int, %s given", zend_zval_type_name(handle));
zend_string_release_ex(func_name, 0);
efree(error);
RETURN_FALSE;
RETURN_THROWS();
}
ZEND_ASSERT(!error);

Expand Down Expand Up @@ -966,8 +970,8 @@ PHP_FUNCTION(pcntl_signal_get_handler)
}

if (signo < 1 || signo > 32) {
php_error_docref(NULL, E_WARNING, "Invalid signal");
RETURN_FALSE;
zend_argument_value_error(1, "must be between 1 and 32");
RETURN_THROWS();
}

if ((prev_handle = zend_hash_index_find(&PCNTL_G(php_signal_table), signo)) != NULL) {
Expand Down Expand Up @@ -1197,8 +1201,8 @@ PHP_FUNCTION(pcntl_getpriority)
php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno);
break;
case EINVAL:
php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno);
break;
zend_argument_value_error(2, "must be a valid identifier flag");
RETURN_THROWS();
default:
php_error_docref(NULL, E_WARNING, "Unknown error %d has occurred", errno);
break;
Expand Down Expand Up @@ -1231,7 +1235,8 @@ PHP_FUNCTION(pcntl_setpriority)
php_error_docref(NULL, E_WARNING, "Error %d: No process was located using the given parameters", errno);
break;
case EINVAL:
php_error_docref(NULL, E_WARNING, "Error %d: Invalid identifier flag", errno);
zend_argument_value_error(3, "must be a valid identifier flag");
RETURN_THROWS();
break;
case EPERM:
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);
Expand Down Expand Up @@ -1400,19 +1405,18 @@ PHP_FUNCTION(pcntl_async_signals)
PHP_FUNCTION(pcntl_unshare)
{
zend_long flags;
int ret;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_LONG(flags)
ZEND_PARSE_PARAMETERS_END();

ret = unshare(flags);
if (ret == -1) {
if (unshare(flags) == -1) {
PCNTL_G(last_error) = errno;
switch (errno) {
#ifdef EINVAL
case EINVAL:
php_error_docref(NULL, E_WARNING, "Error %d: Invalid flag specified", errno);
zend_argument_value_error(1, "must be a valid identifier flag");
RETURN_THROWS();
break;
#endif
#ifdef ENOMEM
Expand Down
3 changes: 1 addition & 2 deletions ext/pcntl/pcntl.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ function pcntl_wait(&$status, int $options = 0, &$rusage = []): int {}
/** @param callable|int $handler */
function pcntl_signal(int $signo, $handler, bool $restart_syscalls = true): bool {}

/** @return mixed */
function pcntl_signal_get_handler(int $signo) {}
function pcntl_signal_get_handler(int $signo): mixed {}

function pcntl_signal_dispatch(): bool {}

Expand Down
4 changes: 2 additions & 2 deletions ext/pcntl/pcntl_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: df744f88533ce9b84864fa2aa4dd7a5b7373231d */
* Stub hash: 79f0005cda5f8bc55a913c8d2c8791699abc2f0f */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_fork, 0, 0, IS_LONG, 0)
ZEND_END_ARG_INFO()
Expand All @@ -23,7 +23,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_signal, 0, 2, _IS_BOOL, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, restart_syscalls, _IS_BOOL, 0, "true")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_pcntl_signal_get_handler, 0, 0, 1)
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_signal_get_handler, 0, 1, IS_MIXED, 0)
ZEND_ARG_TYPE_INFO(0, signo, IS_LONG, 0)
ZEND_END_ARG_INFO()

Expand Down
32 changes: 20 additions & 12 deletions ext/pcntl/tests/pcntl_signal.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,24 @@ posix_kill(posix_getpid(), SIGUSR1);
pcntl_signal_dispatch();

var_dump(pcntl_signal(SIGALRM, SIG_IGN));
var_dump(pcntl_signal(-1, -1));
var_dump(pcntl_signal(-1, function(){}));
var_dump(pcntl_signal(SIGALRM, "not callable"));

try {
pcntl_signal(-1, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

try {
pcntl_signal(-1, function(){});
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

try {
pcntl_signal(SIGALRM, "not callable");
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

/* test freeing queue in RSHUTDOWN */
posix_kill(posix_getpid(), SIGTERM);
Expand All @@ -31,13 +45,7 @@ echo "ok\n";
signal dispatched
got signal from %r\d+|nobody%r
bool(true)

Warning: pcntl_signal(): Invalid signal %s
bool(false)

Warning: pcntl_signal(): Invalid signal %s
bool(false)

Warning: pcntl_signal(): Specified handler "not callable" is not callable (%s) in %s
bool(false)
pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1
pcntl_signal(): Argument #1 ($signo) must be greater than or equal to 1
pcntl_signal(): Argument #2 ($handler) must be of type callable|int, string given
ok