Skip to content

Convert shmop resources to opaque objects #5537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Convert shmop resources to opaque objects #5537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5d3c9e
Convert shmop resources to opaque objects
cmb69 May 7, 2020
d7c886d
Don't expose php_shmop type
cmb69 Jun 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 0 additions & 20 deletions ext/shmop/php_shmop.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
138 changes: 81 additions & 57 deletions ext/shmop/shmop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include "php.h"
#include "php_ini.h"
#include "Zend/zend_interfaces.h"
#include "php_shmop.h"
#include "shmop_arginfo.h"

Expand All @@ -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 = {
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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?)");
Expand Down
21 changes: 9 additions & 12 deletions ext/shmop/shmop.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@

/** @generate-function-entries */

/** @return resource|false */
function shmop_open(int $key, string $flags, int $mode, int $size) {}
final class Shmop {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using a class name like SharedMemoryBlock? For now, the chosen class name is mostly opaque, but if we end up adding an OO API, naming would play a more important role.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against a more descriptive name, but Shmop has the advantage that it's short and also unlikely to collide.


/** @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 {}
19 changes: 12 additions & 7 deletions ext/shmop/shmop_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
/* This is a generated file, edit the .stub.php file instead. */

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)
ZEND_ARG_TYPE_INFO(0, size, IS_LONG, 0)
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()


Expand All @@ -43,9 +43,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
};
Loading