Skip to content

Commit 025054e

Browse files
GirgiasGirgias
committed
Refactor pcntl_sigwaitinfo()/pcntl_sigtimedwait()
1 parent 65a02f4 commit 025054e

File tree

4 files changed

+201
-52
lines changed

4 files changed

+201
-52
lines changed

‎ext/pcntl/pcntl.c

Lines changed: 105 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -767,81 +767,135 @@ PHP_FUNCTION(pcntl_sigprocmask)
767767

768768
#ifdef HAVE_STRUCT_SIGINFO_T
769769
# if defined(HAVE_SIGWAITINFO) && defined(HAVE_SIGTIMEDWAIT)
770-
static void pcntl_sigwaitinfo(INTERNAL_FUNCTION_PARAMETERS, int timedwait) /* {{{ */
771-
{
772-
zval *user_set, *user_signo, *user_siginfo = NULL;
773-
zend_long tv_sec = 0, tv_nsec = 0;
774-
sigset_t set;
775-
int signo;
776-
siginfo_t siginfo;
777-
struct timespec timeout;
778-
779-
if (timedwait) {
780-
ZEND_PARSE_PARAMETERS_START(1, 4)
781-
Z_PARAM_ARRAY(user_set)
782-
Z_PARAM_OPTIONAL
783-
Z_PARAM_ZVAL(user_siginfo)
784-
Z_PARAM_LONG(tv_sec)
785-
Z_PARAM_LONG(tv_nsec)
786-
ZEND_PARSE_PARAMETERS_END();
787-
} else {
788-
ZEND_PARSE_PARAMETERS_START(1, 2)
789-
Z_PARAM_ARRAY(user_set)
790-
Z_PARAM_OPTIONAL
791-
Z_PARAM_ZVAL(user_siginfo)
792-
ZEND_PARSE_PARAMETERS_END();
770+
771+
static bool php_pcntl_process_user_signal_infos(
772+
/* const */ HashTable *const user_signals,
773+
sigset_t *const set
774+
) {
775+
if (zend_hash_num_elements(user_signals) == 0) {
776+
zend_argument_value_error(1, "cannot be empty");
777+
return false;
793778
}
794779

795-
if (sigemptyset(&set) != 0) {
780+
errno = 0;
781+
if (sigemptyset(set) != 0) {
796782
PCNTL_G(last_error) = errno;
797783
php_error_docref(NULL, E_WARNING, "%s", strerror(errno));
798-
RETURN_FALSE;
784+
return false;
799785
}
800786

801-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(user_set), user_signo) {
802-
signo = zval_get_long(user_signo);
803-
if (sigaddset(&set, signo) != 0) {
787+
zval *user_signal_no;
788+
ZEND_HASH_FOREACH_VAL(user_signals, user_signal_no) {
789+
bool failed = true;
790+
zend_long tmp = zval_try_get_long(user_signal_no, &failed);
791+
792+
if (failed) {
793+
zend_argument_type_error(1, "signals must be of type int, %s given", zend_zval_value_name(user_signal_no));
794+
return false;
795+
}
796+
/* Signals are positive integers */
797+
if (tmp < 0 || tmp > INT_MAX) {
798+
zend_argument_value_error(1, "signals must be between 0 and %d", INT_MAX);
799+
return false;
800+
}
801+
802+
int signal_no = (int) tmp;
803+
errno = 0;
804+
if (sigaddset(set, signal_no) != 0) {
804805
PCNTL_G(last_error) = errno;
805806
php_error_docref(NULL, E_WARNING, "%s", strerror(errno));
806-
RETURN_FALSE;
807+
return false;
807808
}
808809
} ZEND_HASH_FOREACH_END();
810+
return true;
811+
}
809812

810-
if (timedwait) {
811-
timeout.tv_sec = (time_t) tv_sec;
812-
timeout.tv_nsec = tv_nsec;
813-
signo = sigtimedwait(&set, &siginfo, &timeout);
814-
} else {
815-
signo = sigwaitinfo(&set, &siginfo);
813+
/* {{{ Synchronously wait for queued signals */
814+
PHP_FUNCTION(pcntl_sigwaitinfo)
815+
{
816+
HashTable *user_set;
817+
/* Optional by-ref array of ints */
818+
zval *user_siginfo = NULL;
819+
820+
ZEND_PARSE_PARAMETERS_START(1, 2)
821+
Z_PARAM_ARRAY_HT(user_set)
822+
Z_PARAM_OPTIONAL
823+
Z_PARAM_ZVAL(user_siginfo)
824+
ZEND_PARSE_PARAMETERS_END();
825+
826+
sigset_t set;
827+
bool status = php_pcntl_process_user_signal_infos(user_set, &set);
828+
/* Some error occurred */
829+
if (!status) {
830+
RETURN_FALSE;
816831
}
817-
if (signo == -1 && errno != EAGAIN) {
832+
833+
errno = 0;
834+
siginfo_t siginfo;
835+
int signal_no = sigwaitinfo(&set, &siginfo);
836+
if (signal_no == -1 && errno != EAGAIN) {
818837
PCNTL_G(last_error) = errno;
819838
php_error_docref(NULL, E_WARNING, "%s", strerror(errno));
839+
// TODO BC Break, as -1 used to be returned?
840+
// RETURN_FALSE;
820841
}
821842

822-
/*
823-
* sigtimedwait and sigwaitinfo can return 0 on success on some
824-
* platforms, e.g. NetBSD
825-
*/
826-
if (!signo && siginfo.si_signo) {
827-
signo = siginfo.si_signo;
843+
/* sigwaitinfo can return 0 on success on some platforms, e.g. NetBSD */
844+
if (!signal_no && siginfo.si_signo) {
845+
signal_no = siginfo.si_signo;
828846
}
829-
pcntl_siginfo_to_zval(signo, &siginfo, user_siginfo);
830-
RETURN_LONG(signo);
831-
}
832-
/* }}} */
833847

834-
/* {{{ Synchronously wait for queued signals */
835-
PHP_FUNCTION(pcntl_sigwaitinfo)
836-
{
837-
pcntl_sigwaitinfo(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
848+
pcntl_siginfo_to_zval(signal_no, &siginfo, user_siginfo);
849+
850+
RETURN_LONG(signal_no);
838851
}
839852
/* }}} */
840853

841854
/* {{{ Wait for queued signals */
842855
PHP_FUNCTION(pcntl_sigtimedwait)
843856
{
844-
pcntl_sigwaitinfo(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
857+
HashTable *user_set;
858+
/* Optional by-ref array of ints */
859+
zval *user_siginfo = NULL;
860+
zend_long tv_sec = 0;
861+
zend_long tv_nsec = 0;
862+
struct timespec timeout;
863+
864+
ZEND_PARSE_PARAMETERS_START(1, 4)
865+
Z_PARAM_ARRAY_HT(user_set)
866+
Z_PARAM_OPTIONAL
867+
Z_PARAM_ZVAL(user_siginfo)
868+
Z_PARAM_LONG(tv_sec)
869+
Z_PARAM_LONG(tv_nsec)
870+
ZEND_PARSE_PARAMETERS_END();
871+
872+
sigset_t set;
873+
bool status = php_pcntl_process_user_signal_infos(user_set, &set);
874+
/* Some error occurred */
875+
if (!status) {
876+
RETURN_FALSE;
877+
}
878+
879+
errno = 0;
880+
siginfo_t siginfo;
881+
timeout.tv_sec = (time_t) tv_sec;
882+
timeout.tv_nsec = tv_nsec;
883+
int signal_no = sigtimedwait(&set, &siginfo, &timeout);
884+
if (signal_no == -1 && errno != EAGAIN) {
885+
PCNTL_G(last_error) = errno;
886+
php_error_docref(NULL, E_WARNING, "%s", strerror(errno));
887+
// TODO BC Break, as -1 used to be returned?
888+
// RETURN_FALSE;
889+
}
890+
891+
/* sigtimedwait can return 0 on success on some platforms, e.g. NetBSD */
892+
if (!signal_no && siginfo.si_signo) {
893+
signal_no = siginfo.si_signo;
894+
}
895+
896+
pcntl_siginfo_to_zval(signal_no, &siginfo, user_siginfo);
897+
898+
RETURN_LONG(signal_no);
845899
}
846900
/* }}} */
847901
# endif

‎ext/pcntl/tests/pcntl_alarm.phpt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ pcntl_alarm(0);
1414
var_dump(pcntl_alarm(60));
1515
var_dump(pcntl_alarm(1) > 0);
1616
$siginfo = array();
17-
var_dump(pcntl_sigtimedwait(array(SIGALRM),$siginfo,2) === SIGALRM);
17+
$signo = pcntl_sigtimedwait(array(SIGALRM),$siginfo,2);
18+
var_dump($signo === SIGALRM);
1819
?>
1920
--EXPECT--
2021
int(0)

‎ext/pcntl/tests/pcntl_sigtimedwait_errors.phpt

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
pcntl_sigtimedwait() errors
3+
--EXTENSIONS--
4+
pcntl
5+
--SKIPIF--
6+
<?php if (!function_exists("pcntl_sigtimedwait")) die("skip pcntl_sigtimedwait() not available"); ?>
7+
--INI--
8+
max_execution_time=0
9+
--FILE--
10+
<?php
11+
12+
try {
13+
/* This used to return -1 prior to PHP 8.3.0 */
14+
$signals = [];
15+
$signal_no = pcntl_sigtimedwait($signals, $signal_infos, 2);
16+
} catch (\Throwable $e) {
17+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
18+
}
19+
20+
try {
21+
$signals = [-1];
22+
$signal_no = pcntl_sigtimedwait($signals, $signal_infos, 2);
23+
} catch (\Throwable $e) {
24+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
25+
}
26+
27+
try {
28+
$signals = ["not a signal"];
29+
$signal_no = pcntl_sigtimedwait($signals, $signal_infos, 2);
30+
} catch (\Throwable $e) {
31+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
32+
}
33+
34+
/* Unlikely valid signal */
35+
$signals = [2**10];
36+
$signal_no = pcntl_sigtimedwait($signals, $signal_infos, 2);
37+
var_dump($signal_no);
38+
var_dump($signal_infos);
39+
?>
40+
--EXPECTF--
41+
ValueError: pcntl_sigtimedwait(): Argument #1 ($signals) cannot be empty
42+
ValueError: pcntl_sigtimedwait(): Argument #1 ($signals) signals must be between 0 and %d
43+
TypeError: pcntl_sigtimedwait(): Argument #1 ($signals) signals must be of type int, string given
44+
45+
Warning: pcntl_sigtimedwait(): Invalid argument in %s on line %d
46+
bool(false)
47+
NULL

‎ext/pcntl/tests/pcntl_sigwaitinfo_errors.phpt

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
pcntl_sigwaitinfo() errors
3+
--EXTENSIONS--
4+
pcntl
5+
--SKIPIF--
6+
<?php if (!function_exists("pcntl_sigwaitinfo")) die("skip pcntl_sigtimedwait() not available"); ?>
7+
--INI--
8+
max_execution_time=0
9+
--FILE--
10+
<?php
11+
12+
try {
13+
/* This used to return -1 prior to PHP 8.3.0 */
14+
$signals = [];
15+
$signal_no = pcntl_sigwaitinfo($signals, $signal_infos);
16+
} catch (\Throwable $e) {
17+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
18+
}
19+
20+
try {
21+
$signals = [-1];
22+
$signal_no = pcntl_sigwaitinfo($signals, $signal_infos);
23+
} catch (\Throwable $e) {
24+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
25+
}
26+
27+
try {
28+
$signals = ["not a signal"];
29+
$signal_no = pcntl_sigwaitinfo($signals, $signal_infos);
30+
} catch (\Throwable $e) {
31+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
32+
}
33+
34+
/* Unlikely valid signal */
35+
$signals = [2**10];
36+
$signal_no = pcntl_sigwaitinfo($signals, $signal_infos);
37+
var_dump($signal_no);
38+
var_dump($signal_infos);
39+
?>
40+
--EXPECTF--
41+
ValueError: pcntl_sigwaitinfo(): Argument #1 ($signals) cannot be empty
42+
ValueError: pcntl_sigwaitinfo(): Argument #1 ($signals) signals must be between 0 and %d
43+
TypeError: pcntl_sigwaitinfo(): Argument #1 ($signals) signals must be of type int, string given
44+
45+
Warning: pcntl_sigwaitinfo(): Invalid argument in %s on line %d
46+
bool(false)
47+
NULL

0 commit comments

Comments
 (0)