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.

diff --git a/NEWS b/NEWS
index 3732e9946db846c579b245191d951c9403967d14..be5aae762aec8a4d0e2d1f64ab43d8952e1bafc0 100644 (file)
--- 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 165c133b708cf3246c90f9d56da7be4c2660002f..f2210ee055abb6bd0413cbf3ce5ac642ffa7e204 100644 (file)
--- 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 659941aa3e8b9e9a775b604953488bc034b65470..375c53eba87e50ee97db61f5d53a1974c9f21094 100644 (file)
--- 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 6f464e8745e900273d9a7f15242ea0565d08835e..c1e815eeb2e68f6c4d0a783e42674f6c14ae1695 100644 (file)
--- 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 92f5c212b8d41a2792e826d7316d9308276ef341..c19cbc26f0e07bb36dde53c5b7ef57e93cc61963 100644 (file)
--- 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 39f8615783cb03885c14420182dcbe6d4e7e8533..55914efb8c95e95d61311c86c1dd346479cae398 100644 (file)
--- 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 48e722d2f0cb099b7bce697dd1fcf79d1c212b89..55e8476b844a2b6a2cd6105a777bbd6b693f38a1 100644 (file)
--- 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 df969885c9e7145fd923a140896048aa72531db4..f7e3a4666aa9a0998c79d2c9b3e7aee184c065c5 100644 (file)
--- 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)