Skip to content

Commit 9ac2790

Browse files
committed
Fix code review
1 parent d18d8fe commit 9ac2790

File tree

6 files changed

+42
-19
lines changed

6 files changed

+42
-19
lines changed

UPGRADING

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,7 @@ PHP 8.0 UPGRADE NOTES
451451
- Sysvshm:
452452
. shm_attach() will now return an SysvSharedMemory object rather than a resource.
453453
Return value checks using is_resource() should be replaced with checks
454-
for `false`. The shm_detach() function no longer has an effect, instead
455-
the SysvSharedMemory instance is automatically destroyed if it is no longer referenced.
454+
for `false`.
456455

457456
- tidy:
458457
. The $use_include_path parameter, which was not used internally, has been

ext/sysvshm/sysvshm.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ PHP_FUNCTION(shm_detach)
203203

204204
shm_list_ptr = Z_SYSVSHM_P(shm_id);
205205
if (!shm_list_ptr->ptr) {
206-
RETURN_TRUE;
206+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
207+
RETURN_THROWS();
207208
}
208209

209210
shmdt((void *) shm_list_ptr->ptr);
@@ -226,7 +227,8 @@ PHP_FUNCTION(shm_remove)
226227

227228
shm_list_ptr = Z_SYSVSHM_P(shm_id);
228229
if (!shm_list_ptr->ptr) {
229-
RETURN_TRUE;
230+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
231+
RETURN_THROWS();
230232
}
231233

232234
if (shmctl(shm_list_ptr->id, IPC_RMID, NULL) < 0) {
@@ -255,7 +257,8 @@ PHP_FUNCTION(shm_put_var)
255257

256258
shm_list_ptr = Z_SYSVSHM_P(shm_id);
257259
if (!shm_list_ptr->ptr) {
258-
RETURN_FALSE;
260+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
261+
RETURN_THROWS();
259262
}
260263

261264
/* setup string-variable and serialize */
@@ -294,10 +297,14 @@ PHP_FUNCTION(shm_get_var)
294297
}
295298

296299
shm_list_ptr = Z_SYSVSHM_P(shm_id);
300+
if (!shm_list_ptr->ptr) {
301+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
302+
RETURN_THROWS();
303+
}
297304

298305
/* setup string-variable and serialize */
299306
/* get serialized variable from shared memory */
300-
shm_varpos = php_check_shm_data((shm_list_ptr->ptr), shm_key);
307+
shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);
301308

302309
if (shm_varpos < 0) {
303310
php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key);
@@ -328,9 +335,9 @@ PHP_FUNCTION(shm_has_var)
328335
}
329336

330337
shm_list_ptr = Z_SYSVSHM_P(shm_id);
331-
332338
if (!shm_list_ptr->ptr) {
333-
RETURN_FALSE;
339+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
340+
RETURN_THROWS();
334341
}
335342

336343
RETURN_BOOL(php_check_shm_data(shm_list_ptr->ptr, shm_key) >= 0);
@@ -350,8 +357,12 @@ PHP_FUNCTION(shm_remove_var)
350357
}
351358

352359
shm_list_ptr = Z_SYSVSHM_P(shm_id);
360+
if (!shm_list_ptr->ptr) {
361+
zend_throw_error(NULL, "Shared memory block has already been destroyed.");
362+
RETURN_THROWS();
363+
}
353364

354-
shm_varpos = php_check_shm_data((shm_list_ptr->ptr), shm_key);
365+
shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);
355366

356367
if (shm_varpos < 0) {
357368
php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key);

ext/sysvshm/sysvshm.stub.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ function shm_has_var(SysvSharedMemory $shm, int $variable_key): bool {}
1414

1515
function shm_remove(SysvSharedMemory $shm): bool {}
1616

17-
function shm_put_var(SysvSharedMemory $shm, int $variable_key, $variable): bool {}
17+
function shm_put_var(SysvSharedMemory $shm, int $variable_key, mixed $variable): bool {}
1818

19-
/** @return mixed */
20-
function shm_get_var(SysvSharedMemory $shm, int $variable_key) {}
19+
function shm_get_var(SysvSharedMemory $shm, int $variable_key): mixed {}
2120

2221
function shm_remove_var(SysvSharedMemory $shm, int $variable_key): bool {}

ext/sysvshm/sysvshm_arginfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ ZEND_END_ARG_INFO()
2020
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shm_put_var, 0, 3, _IS_BOOL, 0)
2121
ZEND_ARG_OBJ_INFO(0, shm, SysvSharedMemory, 0)
2222
ZEND_ARG_TYPE_INFO(0, variable_key, IS_LONG, 0)
23-
ZEND_ARG_INFO(0, variable)
23+
ZEND_ARG_TYPE_INFO(0, variable, IS_MIXED, 0)
2424
ZEND_END_ARG_INFO()
2525

26-
ZEND_BEGIN_ARG_INFO_EX(arginfo_shm_get_var, 0, 0, 2)
26+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shm_get_var, 0, 2, IS_MIXED, 0)
2727
ZEND_ARG_OBJ_INFO(0, shm, SysvSharedMemory, 0)
2828
ZEND_ARG_TYPE_INFO(0, variable_key, IS_LONG, 0)
2929
ZEND_END_ARG_INFO()

ext/sysvshm/tests/003.phpt

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,17 @@ $key = ftok(__DIR__."/003.phpt", 'q');
1313
$s = shm_attach($key);
1414

1515
var_dump(shm_detach($s));
16-
var_dump(shm_detach($s));
17-
shm_remove($s);
16+
try {
17+
shm_detach($s);
18+
} catch (Error $exception) {
19+
echo $exception->getMessage() . "\n";
20+
}
21+
22+
try {
23+
shm_remove($s);
24+
} catch (Error $exception) {
25+
echo $exception->getMessage() . "\n";
26+
}
1827

1928
echo "Done\n";
2029
?>
@@ -28,5 +37,6 @@ shm_remove($s);
2837
?>
2938
--EXPECT--
3039
bool(true)
31-
bool(true)
40+
Shared memory block has already been destroyed.
41+
Shared memory block has already been destroyed.
3242
Done

ext/sysvshm/tests/007.phpt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@ $s = shm_attach($key, 1024);
1414
var_dump(shm_remove($s));
1515

1616
shm_detach($s);
17-
var_dump(shm_remove($s));
17+
try {
18+
shm_remove($s);
19+
} catch (Error $exception) {
20+
echo $exception->getMessage() . "\n";
21+
}
1822

1923
echo "Done\n";
2024
?>
2125
--EXPECTF--
2226
bool(true)
23-
bool(true)
27+
Shared memory block has already been destroyed.
2428
Done

0 commit comments

Comments
 (0)