Skip to content

Commit d1bce65

Browse files
committed
changes from feedback
1 parent 5f7e7ba commit d1bce65

File tree

4 files changed

+49
-26
lines changed

4 files changed

+49
-26
lines changed

ext/pcntl/pcntl.c

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,14 +1487,13 @@ PHP_FUNCTION(pcntl_getcpuaffinity)
14871487
zend_long pid;
14881488
bool pid_is_null = 1;
14891489
cpu_set_t mask;
1490-
zend_ulong i, maxcpus;
14911490

14921491
ZEND_PARSE_PARAMETERS_START(0, 1)
14931492
Z_PARAM_OPTIONAL
14941493
Z_PARAM_LONG_OR_NULL(pid, pid_is_null)
14951494
ZEND_PARSE_PARAMETERS_END();
14961495

1497-
// 0 == getpid in this context, we re just saving a syscall
1496+
// 0 == getpid in this context, we're just saving a syscall
14981497
pid = pid_is_null ? 0 : pid;
14991498

15001499
CPU_ZERO(&mask);
@@ -1512,13 +1511,13 @@ PHP_FUNCTION(pcntl_getcpuaffinity)
15121511
php_error_docref(NULL, E_WARNING, "Error %d", errno);
15131512
}
15141513

1515-
RETURN_EMPTY_ARRAY();
1514+
RETURN_FALSE;
15161515
}
15171516

1518-
maxcpus = (zend_ulong)sysconf(_SC_NPROCESSORS_ONLN);
1517+
zend_ulong maxcpus = (zend_ulong)sysconf(_SC_NPROCESSORS_ONLN);
15191518
array_init(return_value);
15201519

1521-
for (i = 0; i < maxcpus; i ++) {
1520+
for (zend_ulong i = 0; i < maxcpus; i ++) {
15221521
if (CPU_ISSET(i, &mask)) {
15231522
add_next_index_long(return_value, i);
15241523
}
@@ -1530,36 +1529,48 @@ PHP_FUNCTION(pcntl_setcpuaffinity)
15301529
zend_long pid;
15311530
bool pid_is_null = 1;
15321531
cpu_set_t mask;
1533-
zval *hmask = NULL, *cpu;
1534-
zend_ulong maxcpus;
1532+
zval *hmask = NULL, *ncpu;
15351533

15361534
ZEND_PARSE_PARAMETERS_START(0, 2)
15371535
Z_PARAM_OPTIONAL
15381536
Z_PARAM_LONG_OR_NULL(pid, pid_is_null)
15391537
Z_PARAM_ARRAY(hmask)
15401538
ZEND_PARSE_PARAMETERS_END();
15411539

1542-
if (!hmask || Z_ARRVAL_P(hmask)->nNumOfElements == 0) {
1540+
if (!hmask || zend_hash_num_elements(Z_ARRVAL_P(hmask)) == 0) {
15431541
zend_argument_value_error(2, "must not be empty");
15441542
RETURN_THROWS();
15451543
}
15461544

1547-
// 0 == getpid in this context, we re just saving a syscall
1545+
// 0 == getpid in this context, we're just saving a syscall
15481546
pid = pid_is_null ? 0 : pid;
1549-
maxcpus = sysconf(_SC_NPROCESSORS_ONLN);
1547+
zend_ulong maxcpus = (zend_ulong)sysconf(_SC_NPROCESSORS_ONLN);
15501548
CPU_ZERO(&mask);
15511549

1552-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(hmask), cpu) {
1553-
if (Z_TYPE_P(cpu) == IS_LONG && Z_LVAL_P(cpu) >= 0 &&
1554-
Z_LVAL_P(cpu) < maxcpus && !CPU_ISSET(Z_LVAL_P(cpu), &mask)) {
1555-
CPU_SET(Z_LVAL_P(cpu), &mask);
1550+
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(hmask), ncpu) {
1551+
ZVAL_DEREF(ncpu);
1552+
zend_long cpu;
1553+
if (Z_TYPE_P(ncpu) != IS_LONG) {
1554+
if (Z_TYPE_P(ncpu) == IS_STRING) {
1555+
cpu = zval_get_long(ncpu);
1556+
} else {
1557+
zend_string *wcpu = zval_get_string_func(ncpu);
1558+
zend_value_error("cpu id invalid type (%s)", ZSTR_VAL(wcpu));
1559+
RETURN_THROWS();
1560+
}
1561+
} else {
1562+
cpu = Z_LVAL_P(ncpu);
15561563
}
1557-
} ZEND_HASH_FOREACH_END();
15581564

1559-
if (!CPU_COUNT(&mask)) {
1560-
zend_argument_value_error(2, "invalid cpu affinity mapping");
1561-
RETURN_THROWS();
1562-
}
1565+
if (cpu < 0 || cpu >= maxcpus) {
1566+
zend_value_error("cpu id must be between 0 and " ZEND_ULONG_FMT " (" ZEND_LONG_FMT ")", maxcpus, cpu);
1567+
RETURN_THROWS();
1568+
}
1569+
1570+
if (!CPU_ISSET(cpu, &mask)) {
1571+
CPU_SET(cpu, &mask);
1572+
}
1573+
} ZEND_HASH_FOREACH_END();
15631574

15641575
if (sched_setaffinity(pid, sizeof(mask), &mask) != 0) {
15651576
PCNTL_G(last_error) = errno;
@@ -1570,6 +1581,8 @@ PHP_FUNCTION(pcntl_setcpuaffinity)
15701581
case EPERM:
15711582
php_error_docref(NULL, E_WARNING, "Calling process not having the proper privileges");
15721583
break;
1584+
default:
1585+
php_error_docref(NULL, E_WARNING, "Error %d", errno);
15731586
}
15741587
RETURN_FALSE;
15751588
} else {

ext/pcntl/pcntl.stub.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,6 @@ function pcntl_setns(?int $process_id = null, int $nstype = CLONE_NEWNET): bool
996996
#endif
997997

998998
#ifdef HAVE_SCHED_SETAFFINITY
999-
function pcntl_getcpuaffinity(?int $process_id = null): array {}
999+
function pcntl_getcpuaffinity(?int $process_id = null): array|false {}
10001000
function pcntl_setcpuaffinity(?int $process_id = null, array $cpu_ids = []): bool {}
10011001
#endif

ext/pcntl/pcntl_arginfo.h

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/pcntl/tests/pcntl_cpuaffinity.phpt

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ $act_mask = pcntl_getcpuaffinity();
1414
var_dump(array_diff($mask, $act_mask));
1515
$n_act_mask = pcntl_getcpuaffinity();
1616
var_dump(array_diff($act_mask, $n_act_mask));
17+
var_dump(pcntl_setcpuaffinity(null, ["0", "1"]));
1718

1819
try {
1920
pcntl_setcpuaffinity(null, []);
@@ -41,18 +42,27 @@ try {
4142

4243
try {
4344
pcntl_getcpuaffinity(-1024);
45+
} catch (\ValueError $e) {
46+
echo $e->getMessage() . PHP_EOL;
47+
}
48+
49+
try {
50+
pcntl_setcpuaffinity(null, [1, array(1)]);
4451
} catch (\ValueError $e) {
4552
echo $e->getMessage();
4653
}
4754
?>
48-
--EXPECT--
55+
--EXPECTF--
4956
bool(true)
5057
array(0) {
5158
}
5259
array(0) {
5360
}
61+
bool(true)
5462
pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) must not be empty
55-
pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) invalid cpu affinity mapping
56-
pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) invalid cpu affinity mapping
57-
pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) invalid cpu affinity mapping
63+
cpu id must be between 0 and %d (%d)
64+
cpu id must be between 0 and %d (-1024)
5865
pcntl_getcpuaffinity(): Argument #1 ($process_id) invalid process (-1024)
66+
67+
Warning: Array to string conversion in %s on line %d
68+
cpu id invalid type (Array)

0 commit comments

Comments
 (0)