From 18f58080dcb07295c09ae2f757691c96a5a312de Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 8 Jun 2020 17:58:08 +0200 Subject: [PATCH] Convert shmop resources to opaque objects We make `shmop_close()` a NOP, and deprecate the function right away; detaching from SHM now happens when the wrapper object is freed. --- NEWS | 3 + UPGRADING | 7 ++ ext/shmop/php_shmop.h | 20 ---- ext/shmop/shmop.c | 138 ++++++++++++++---------- ext/shmop/shmop.stub.php | 21 ++-- ext/shmop/shmop_arginfo.h | 19 ++-- ext/shmop/tests/001.phpt | 6 -- ext/shmop/tests/shmop_open_private.phpt | 3 - 8 files changed, 112 insertions(+), 105 deletions(-) diff --git a/NEWS b/NEWS index 3732e9946d..be5aae762a 100644 --- a/NEWS +++ b/NEWS @@ -151,6 +151,9 @@ PHP NEWS handlers). (bshaffer) . Fixed bug #73529 (session_decode() silently fails on wrong input). (cmb) +- Shmop: + . Converted shmop resources to objects. (cmb) + - SimpleXML: . Fixed bug #75245 (Don't set content of elements with only whitespaces). (eriklundin) diff --git a/UPGRADING b/UPGRADING index 165c133b70..f2210ee055 100644 --- a/UPGRADING +++ b/UPGRADING @@ -793,6 +793,13 @@ PHP 8.0 UPGRADE NOTES - PGSQL / PDO PGSQL: . The PGSQL and PDO PGSQL extensions now require at least libpq 9.1. +- Shmop: + . shmop_open() will now return a Shmop object rather than a resource. Return + value checks using is_resource() should be replaced with checks for `false`. + The shmop_close() function no longer has an effect, and is deprecated; + instead the Shmop instance is automatically destroyed if it is no longer + referenced. + ======================================== 10. New Global Constants ======================================== diff --git a/ext/shmop/php_shmop.h b/ext/shmop/php_shmop.h index 659941aa3e..375c53eba8 100644 --- a/ext/shmop/php_shmop.h +++ b/ext/shmop/php_shmop.h @@ -32,26 +32,6 @@ PHP_MINFO_FUNCTION(shmop); # include "win32/ipc.h" #endif -struct php_shmop -{ - int shmid; - key_t key; - int shmflg; - int shmatflg; - char *addr; - zend_long size; -}; - -typedef struct { - int le_shmop; -} php_shmop_globals; - -#ifdef ZTS -#define SHMOPG(v) TSRMG(shmop_globals_id, php_shmop_globals *, v) -#else -#define SHMOPG(v) (shmop_globals.v) -#endif - #else #define phpext_shmop_ptr NULL diff --git a/ext/shmop/shmop.c b/ext/shmop/shmop.c index 6f464e8745..c1e815eeb2 100644 --- a/ext/shmop/shmop.c +++ b/ext/shmop/shmop.c @@ -23,6 +23,7 @@ #include "php.h" #include "php_ini.h" +#include "Zend/zend_interfaces.h" #include "php_shmop.h" #include "shmop_arginfo.h" @@ -38,14 +39,6 @@ #include "ext/standard/info.h" -#ifdef ZTS -int shmop_globals_id; -#else -php_shmop_globals shmop_globals; -#endif - -int shm_type; - /* {{{ shmop_module_entry */ zend_module_entry shmop_module_entry = { @@ -66,22 +59,70 @@ zend_module_entry shmop_module_entry = { ZEND_GET_MODULE(shmop) #endif -/* {{{ rsclean - */ -static void rsclean(zend_resource *rsrc) +typedef struct php_shmop +{ + int shmid; + key_t key; + int shmflg; + int shmatflg; + char *addr; + zend_long size; + zend_object std; +} php_shmop; + +zend_class_entry *shmop_ce; +static zend_object_handlers shmop_object_handlers; + +static inline php_shmop *shmop_from_obj(zend_object *obj) +{ + return (php_shmop *)((char *)(obj) - XtOffsetOf(php_shmop, std)); +} + +#define Z_SHMOP_P(zv) shmop_from_obj(Z_OBJ_P(zv)) + +static zend_object *shmop_create_object(zend_class_entry *class_type) +{ + php_shmop *intern = zend_object_alloc(sizeof(php_shmop), class_type); + + zend_object_std_init(&intern->std, class_type); + object_properties_init(&intern->std, class_type); + intern->std.handlers = &shmop_object_handlers; + + return &intern->std; +} + +static zend_function *shmop_get_constructor(zend_object *object) +{ + zend_throw_error(NULL, "Cannot directly construct Shmop, use shmop_open() instead"); + return NULL; +} + +static void shmop_free_obj(zend_object *object) { - struct php_shmop *shmop = (struct php_shmop *)rsrc->ptr; + php_shmop *shmop = shmop_from_obj(object); shmdt(shmop->addr); - efree(shmop); + + zend_object_std_dtor(&shmop->std); } -/* }}} */ /* {{{ PHP_MINIT_FUNCTION */ PHP_MINIT_FUNCTION(shmop) { - shm_type = zend_register_list_destructors_ex(rsclean, NULL, "shmop", module_number); + zend_class_entry ce; + INIT_CLASS_ENTRY(ce, "Shmop", class_Shmop_methods); + shmop_ce = zend_register_internal_class(&ce); + shmop_ce->ce_flags |= ZEND_ACC_FINAL; + shmop_ce->create_object = shmop_create_object; + shmop_ce->serialize = zend_class_serialize_deny; + shmop_ce->unserialize = zend_class_unserialize_deny; + + memcpy(&shmop_object_handlers, &std_object_handlers, sizeof(zend_object_handlers)); + shmop_object_handlers.offset = XtOffsetOf(php_shmop, std); + shmop_object_handlers.free_obj = shmop_free_obj; + shmop_object_handlers.get_constructor = shmop_get_constructor; + shmop_object_handlers.clone_obj = NULL; return SUCCESS; } @@ -97,12 +138,12 @@ PHP_MINFO_FUNCTION(shmop) } /* }}} */ -/* {{{ proto resource shmop_open(int key, string flags, int mode, int size) +/* {{{ proto Shmop shmop_open(int key, string flags, int mode, int size) gets and attaches a shared memory segment */ PHP_FUNCTION(shmop_open) { zend_long key, mode, size; - struct php_shmop *shmop; + php_shmop *shmop; struct shmid_ds shm; char *flags; size_t flags_len; @@ -116,9 +157,8 @@ PHP_FUNCTION(shmop_open) RETURN_FALSE; } - shmop = emalloc(sizeof(struct php_shmop)); - memset(shmop, 0, sizeof(struct php_shmop)); - + object_init_ex(return_value, shmop_ce); + shmop = Z_SHMOP_P(return_value); shmop->key = key; shmop->shmflg |= mode; @@ -175,32 +215,30 @@ PHP_FUNCTION(shmop_open) } shmop->size = shm.shm_segsz; + return; - RETURN_RES(zend_register_resource(shmop, shm_type)); err: - efree(shmop); + zend_object_release(Z_OBJ_P(return_value)); RETURN_FALSE; } /* }}} */ -/* {{{ proto string shmop_read(resource shmid, int start, int count) +/* {{{ proto string shmop_read(Shmop shmid, int start, int count) reads from a shm segment */ PHP_FUNCTION(shmop_read) { zval *shmid; zend_long start, count; - struct php_shmop *shmop; + php_shmop *shmop; char *startaddr; int bytes; zend_string *return_string; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "rll", &shmid, &start, &count) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oll", &shmid, shmop_ce, &start, &count) == FAILURE) { RETURN_THROWS(); } - if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) { - RETURN_THROWS(); - } + shmop = Z_SHMOP_P(shmid); if (start < 0 || start > shmop->size) { php_error_docref(NULL, E_WARNING, "Start is out of range"); @@ -221,62 +259,50 @@ PHP_FUNCTION(shmop_read) } /* }}} */ -/* {{{ proto void shmop_close(resource shmid) - closes a shared memory segment */ +/* {{{ proto void shmop_close(Shmop shmid) + used to close a shared memory segment; now a NOP */ PHP_FUNCTION(shmop_close) { zval *shmid; - struct php_shmop *shmop; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) { RETURN_THROWS(); } - - - if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) { - RETURN_THROWS(); - } - - zend_list_close(Z_RES_P(shmid)); } /* }}} */ -/* {{{ proto int shmop_size(resource shmid) +/* {{{ proto int shmop_size(Shmop shmid) returns the shm size */ PHP_FUNCTION(shmop_size) { zval *shmid; - struct php_shmop *shmop; + php_shmop *shmop; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) { RETURN_THROWS(); } - if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) { - RETURN_THROWS(); - } + shmop = Z_SHMOP_P(shmid); RETURN_LONG(shmop->size); } /* }}} */ -/* {{{ proto int shmop_write(resource shmid, string data, int offset) +/* {{{ proto int shmop_write(Shmop shmid, string data, int offset) writes to a shared memory segment */ PHP_FUNCTION(shmop_write) { - struct php_shmop *shmop; + php_shmop *shmop; zend_long writesize; zend_long offset; zend_string *data; zval *shmid; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "rSl", &shmid, &data, &offset) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "OSl", &shmid, shmop_ce, &data, &offset) == FAILURE) { RETURN_THROWS(); } - if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) { - RETURN_THROWS(); - } + shmop = Z_SHMOP_P(shmid); if ((shmop->shmatflg & SHM_RDONLY) == SHM_RDONLY) { php_error_docref(NULL, E_WARNING, "Trying to write to a read only segment"); @@ -295,20 +321,18 @@ PHP_FUNCTION(shmop_write) } /* }}} */ -/* {{{ proto bool shmop_delete(resource shmid) +/* {{{ proto bool shmop_delete(Shmop shmid) mark segment for deletion */ PHP_FUNCTION(shmop_delete) { zval *shmid; - struct php_shmop *shmop; + php_shmop *shmop; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &shmid) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &shmid, shmop_ce) == FAILURE) { RETURN_THROWS(); } - if ((shmop = (struct php_shmop *)zend_fetch_resource(Z_RES_P(shmid), "shmop", shm_type)) == NULL) { - RETURN_THROWS(); - } + shmop = Z_SHMOP_P(shmid); if (shmctl(shmop->shmid, IPC_RMID, NULL)) { php_error_docref(NULL, E_WARNING, "Can't mark segment for deletion (are you the owner?)"); diff --git a/ext/shmop/shmop.stub.php b/ext/shmop/shmop.stub.php index 92f5c212b8..c19cbc26f0 100644 --- a/ext/shmop/shmop.stub.php +++ b/ext/shmop/shmop.stub.php @@ -2,20 +2,17 @@ /** @generate-function-entries */ -/** @return resource|false */ -function shmop_open(int $key, string $flags, int $mode, int $size) {} +final class Shmop {} -/** @param resource $shmid */ -function shmop_read($shmid, int $start, int $count): string|false {} +function shmop_open(int $key, string $flags, int $mode, int $size): Shmop|false {} -/** @param resource $shmid */ -function shmop_close($shmid): void {} +function shmop_read(Shmop $shmid, int $start, int $count): string|false {} -/** @param resource $shmid */ -function shmop_size($shmid): int {} +/** @deprecated */ +function shmop_close(Shmop $shmid): void {} -/** @param resource $shmid */ -function shmop_write($shmid, string $data, int $offset): int|false {} +function shmop_size(Shmop $shmid): int {} -/** @param resource $shmid */ -function shmop_delete($shmid): bool {} +function shmop_write(Shmop $shmid, string $data, int $offset): int|false {} + +function shmop_delete(Shmop $shmid): bool {} diff --git a/ext/shmop/shmop_arginfo.h b/ext/shmop/shmop_arginfo.h index 39f8615783..55914efb8c 100644 --- a/ext/shmop/shmop_arginfo.h +++ b/ext/shmop/shmop_arginfo.h @@ -1,7 +1,7 @@ /* This is a generated file, edit the .stub.php file instead. * Stub hash: a2e7d50e79d253e7136a54222346341003cc3b04 */ -ZEND_BEGIN_ARG_INFO_EX(arginfo_shmop_open, 0, 0, 4) +ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_shmop_open, 0, 4, Shmop, MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, key, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, flags, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0) @@ -9,27 +9,27 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_shmop_open, 0, 0, 4) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_shmop_read, 0, 3, MAY_BE_STRING|MAY_BE_FALSE) - ZEND_ARG_INFO(0, shmid) + ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0) ZEND_ARG_TYPE_INFO(0, start, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, count, IS_LONG, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_close, 0, 1, IS_VOID, 0) - ZEND_ARG_INFO(0, shmid) + ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_size, 0, 1, IS_LONG, 0) - ZEND_ARG_INFO(0, shmid) + ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_shmop_write, 0, 3, MAY_BE_LONG|MAY_BE_FALSE) - ZEND_ARG_INFO(0, shmid) + ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0) ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, offset, IS_LONG, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_shmop_delete, 0, 1, _IS_BOOL, 0) - ZEND_ARG_INFO(0, shmid) + ZEND_ARG_OBJ_INFO(0, shmid, Shmop, 0) ZEND_END_ARG_INFO() @@ -44,9 +44,14 @@ ZEND_FUNCTION(shmop_delete); static const zend_function_entry ext_functions[] = { ZEND_FE(shmop_open, arginfo_shmop_open) ZEND_FE(shmop_read, arginfo_shmop_read) - ZEND_FE(shmop_close, arginfo_shmop_close) + ZEND_DEP_FE(shmop_close, arginfo_shmop_close) ZEND_FE(shmop_size, arginfo_shmop_size) ZEND_FE(shmop_write, arginfo_shmop_write) ZEND_FE(shmop_delete, arginfo_shmop_delete) ZEND_FE_END }; + + +static const zend_function_entry class_Shmop_methods[] = { + ZEND_FE_END +}; diff --git a/ext/shmop/tests/001.phpt b/ext/shmop/tests/001.phpt index 48e722d2f0..55e8476b84 100644 --- a/ext/shmop/tests/001.phpt +++ b/ext/shmop/tests/001.phpt @@ -32,8 +32,6 @@ shmop extension test echo "data in memory is: " . shmop_read($shm_id, 0, $written) . "\n"; - shmop_close($shm_id); - echo "shm open for read only: "; $shm_id = shmop_open($hex_shm_id, "a", 0644, 1024); if (!$shm_id) { @@ -47,8 +45,6 @@ shmop extension test /* try to append data to the shared memory segment, this should fail */ shmop_write($shm_id, $write_d1, $written); - shmop_close($shm_id); - echo "shm open for read only: "; $shm_id = shmop_open($hex_shm_id, "w", 0644, 1024); if (!$shm_id) { @@ -73,8 +69,6 @@ shmop extension test } else { echo "ok\n"; } - - shmop_close($shm_id); ?> --EXPECTF-- shm open for create: ok diff --git a/ext/shmop/tests/shmop_open_private.phpt b/ext/shmop/tests/shmop_open_private.phpt index df969885c9..f7e3a4666a 100644 --- a/ext/shmop/tests/shmop_open_private.phpt +++ b/ext/shmop/tests/shmop_open_private.phpt @@ -15,9 +15,6 @@ $shm2 = shmop_open(0, 'c', 0777, 1024); $read = shmop_read($shm2, 0, 4); var_dump(is_string($read) && $read !== $write); - -shmop_close($shm1); -shmop_close($shm2); ?> --EXPECT-- bool(true) -- 2.40.0