Skip to content

SysV minor refactoring #19177

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
+64 −80
Draft

SysV minor refactoring #19177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8272575
ext/sysvshm: shm_detach() always returns true
Girgias Jul 18, 2025
9cddbe9
ext/sysvshm: voidify php_remove_shm_data()
Girgias Jul 18, 2025
2e8b081
ext/sysvshm: refactor php_put_shm_data()
Girgias Jul 18, 2025
4f0d996
ext/sysvshm: use size_t and ssize_t instead of zend_long
Girgias Jul 18, 2025
11d4d6c
ext/sysvsem: use bool type instead of int
Girgias Jul 18, 2025
71cf825
ext/sysvmsg: various minor refactorings
Girgias Jul 18, 2025
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
3 changes: 3 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,9 @@ PHP 8.5 UPGRADE NOTES
. socket_set_option with multicast context throws a ValueError
when the created socket is not of AF_INET/AF_INET6 family.

- SysVSHM:
. shm_detach() now has a return type of true

- Tidy:
. tidy::__construct/parseFile/parseString now throws a ValueError
if the configuration contains an invalid or set a read-only
Expand Down
65 changes: 27 additions & 38 deletions ext/sysvmsg/sysvmsg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,32 +127,32 @@ PHP_MINFO_FUNCTION(sysvmsg)
/* {{{ Set information for a message queue */
PHP_FUNCTION(msg_set_queue)
{
zval *queue, *data;
zval *queue;
HashTable *data;
sysvmsg_queue_t *mq = NULL;
struct msqid_ds stat;

RETVAL_FALSE;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oa", &queue, sysvmsg_queue_ce, &data) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oh", &queue, sysvmsg_queue_ce, &data) == FAILURE) {
RETURN_THROWS();
}

mq = Z_SYSVMSG_QUEUE_P(queue);

RETVAL_FALSE;
if (msgctl(mq->id, IPC_STAT, &stat) == 0) {
zval *item;

/* now pull out members of data and set them in the stat buffer */
if ((item = zend_hash_str_find(Z_ARRVAL_P(data), "msg_perm.uid", sizeof("msg_perm.uid") - 1)) != NULL) {
if ((item = zend_hash_str_find(data, ZEND_STRL("msg_perm.uid"))) != NULL) {
stat.msg_perm.uid = zval_get_long(item);
}
if ((item = zend_hash_str_find(Z_ARRVAL_P(data), "msg_perm.gid", sizeof("msg_perm.gid") - 1)) != NULL) {
if ((item = zend_hash_str_find(data, ZEND_STRL("msg_perm.gid"))) != NULL) {
stat.msg_perm.gid = zval_get_long(item);
}
if ((item = zend_hash_str_find(Z_ARRVAL_P(data), "msg_perm.mode", sizeof("msg_perm.mode") - 1)) != NULL) {
if ((item = zend_hash_str_find(data, ZEND_STRL("msg_perm.mode"))) != NULL) {
stat.msg_perm.mode = zval_get_long(item);
}
if ((item = zend_hash_str_find(Z_ARRVAL_P(data), "msg_qbytes", sizeof("msg_qbytes") - 1)) != NULL) {
if ((item = zend_hash_str_find(data, ZEND_STRL("msg_qbytes"))) != NULL) {
stat.msg_qbytes = zval_get_long(item);
}
if (msgctl(mq->id, IPC_SET, &stat) == 0) {
Expand All @@ -169,28 +169,27 @@ PHP_FUNCTION(msg_stat_queue)
sysvmsg_queue_t *mq = NULL;
struct msqid_ds stat;

RETVAL_FALSE;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &queue, sysvmsg_queue_ce) == FAILURE) {
RETURN_THROWS();
}

mq = Z_SYSVMSG_QUEUE_P(queue);

if (msgctl(mq->id, IPC_STAT, &stat) == 0) {
array_init(return_value);

add_assoc_long(return_value, "msg_perm.uid", stat.msg_perm.uid);
add_assoc_long(return_value, "msg_perm.gid", stat.msg_perm.gid);
add_assoc_long(return_value, "msg_perm.mode", stat.msg_perm.mode);
add_assoc_long(return_value, "msg_stime", stat.msg_stime);
add_assoc_long(return_value, "msg_rtime", stat.msg_rtime);
add_assoc_long(return_value, "msg_ctime", stat.msg_ctime);
add_assoc_long(return_value, "msg_qnum", stat.msg_qnum);
add_assoc_long(return_value, "msg_qbytes", stat.msg_qbytes);
add_assoc_long(return_value, "msg_lspid", stat.msg_lspid);
add_assoc_long(return_value, "msg_lrpid", stat.msg_lrpid);
if (msgctl(mq->id, IPC_STAT, &stat) != 0) {
RETURN_FALSE;
}

array_init(return_value);
Copy link
Member

Choose a reason for hiding this comment

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

nit: seems to me you can use array_init_size here wdyt ?

add_assoc_long(return_value, "msg_perm.uid", stat.msg_perm.uid);
add_assoc_long(return_value, "msg_perm.gid", stat.msg_perm.gid);
add_assoc_long(return_value, "msg_perm.mode", stat.msg_perm.mode);
add_assoc_long(return_value, "msg_stime", stat.msg_stime);
add_assoc_long(return_value, "msg_rtime", stat.msg_rtime);
add_assoc_long(return_value, "msg_ctime", stat.msg_ctime);
add_assoc_long(return_value, "msg_qnum", stat.msg_qnum);
add_assoc_long(return_value, "msg_qbytes", stat.msg_qbytes);
add_assoc_long(return_value, "msg_lspid", stat.msg_lspid);
add_assoc_long(return_value, "msg_lrpid", stat.msg_lrpid);
}
/* }}} */

Expand All @@ -203,11 +202,7 @@ PHP_FUNCTION(msg_queue_exists)
RETURN_THROWS();
}

if (msgget(key, 0) < 0) {
RETURN_FALSE;
}

RETURN_TRUE;
RETURN_BOOL(msgget(key, 0) >= 0);
}
/* }}} */

Expand Down Expand Up @@ -251,11 +246,7 @@ PHP_FUNCTION(msg_remove_queue)

mq = Z_SYSVMSG_QUEUE_P(queue);

if (msgctl(mq->id, IPC_RMID, NULL) == 0) {
RETVAL_TRUE;
} else {
RETVAL_FALSE;
}
RETURN_BOOL(msgctl(mq->id, IPC_RMID, NULL) == 0);
}
/* }}} */

Expand All @@ -270,8 +261,6 @@ PHP_FUNCTION(msg_receive)
struct php_msgbuf *messagebuffer = NULL; /* buffer to transmit */
int result;

RETVAL_FALSE;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "Olzlz|blz",
&queue, sysvmsg_queue_ce, &desiredmsgtype, &out_msgtype, &maxsize,
&out_message, &do_unserialize, &flags, &zerrcode) == FAILURE) {
Expand Down Expand Up @@ -337,6 +326,7 @@ PHP_FUNCTION(msg_receive)
if (zerrcode) {
ZEND_TRY_ASSIGN_REF_LONG(zerrcode, errno);
}
RETVAL_FALSE;
}
efree(messagebuffer);
}
Expand All @@ -353,8 +343,6 @@ PHP_FUNCTION(msg_send)
int result;
size_t message_len = 0;

RETVAL_FALSE;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "Olz|bbz",
&queue, sysvmsg_queue_ce, &msgtype, &message, &do_serialize, &blocking, &zerror) == FAILURE) {
RETURN_THROWS();
Expand Down Expand Up @@ -429,8 +417,9 @@ PHP_FUNCTION(msg_send)
if (zerror) {
ZEND_TRY_ASSIGN_REF_LONG(zerror, errno);
}
RETURN_FALSE;
} else {
RETVAL_TRUE;
RETURN_TRUE;
}
}
/* }}} */
6 changes: 3 additions & 3 deletions ext/sysvsem/sysvsem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ PHP_FUNCTION(sem_get)
/* }}} */

/* {{{ php_sysvsem_semop */
static void php_sysvsem_semop(INTERNAL_FUNCTION_PARAMETERS, int acquire)
static void php_sysvsem_semop(INTERNAL_FUNCTION_PARAMETERS, const bool acquire)
{
zval *arg_id;
bool nowait = 0;
Expand Down Expand Up @@ -311,14 +311,14 @@ static void php_sysvsem_semop(INTERNAL_FUNCTION_PARAMETERS, int acquire)
/* {{{ Acquires the semaphore with the given id, blocking if necessary */
PHP_FUNCTION(sem_acquire)
{
php_sysvsem_semop(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
php_sysvsem_semop(INTERNAL_FUNCTION_PARAM_PASSTHRU, true);
}
/* }}} */

/* {{{ Releases the semaphore with the given id */
PHP_FUNCTION(sem_release)
{
php_sysvsem_semop(INTERNAL_FUNCTION_PARAM_PASSTHRU, 0);
php_sysvsem_semop(INTERNAL_FUNCTION_PARAM_PASSTHRU, false);
}
/* }}} */

Expand Down
12 changes: 6 additions & 6 deletions ext/sysvshm/php_sysvshm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,17 @@ typedef struct {

typedef struct {
zend_long key;
zend_long length;
zend_long next;
size_t length;
size_t next;
char mem;
} sysvshm_chunk;

typedef struct {
char magic[8];
zend_long start;
zend_long end;
zend_long free;
zend_long total;
size_t start;
size_t end;
size_t free;
size_t total;
} sysvshm_chunk_head;

typedef struct {
Expand Down
56 changes: 24 additions & 32 deletions ext/sysvshm/sysvshm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ ZEND_GET_MODULE(sysvshm)
/* TODO: Make this thread-safe. */
sysvshm_module php_sysvshm;

static int php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const char *data, zend_long len);
static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key);
static int php_remove_shm_data(sysvshm_chunk_head *ptr, zend_long shm_varpos);
static bool php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const zend_string *data);
static ssize_t php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key);
static void php_remove_shm_data(sysvshm_chunk_head *ptr, size_t shm_varpos);

/* {{{ PHP_MINIT_FUNCTION */
PHP_MINIT_FUNCTION(sysvshm)
Expand Down Expand Up @@ -235,7 +235,6 @@ PHP_FUNCTION(shm_remove)
PHP_FUNCTION(shm_put_var)
{
zval *shm_id, *arg_var;
int ret;
zend_long shm_key;
sysvshm_shm *shm_list_ptr;
smart_str shm_var = {0};
Expand All @@ -262,13 +261,15 @@ PHP_FUNCTION(shm_put_var)
RETURN_THROWS();
}

ZEND_ASSERT(shm_var.s != NULL);

/* insert serialized variable into shared memory */
ret = php_put_shm_data(shm_list_ptr->ptr, shm_key, shm_var.s? ZSTR_VAL(shm_var.s) : NULL, shm_var.s? ZSTR_LEN(shm_var.s) : 0);
bool ret = php_put_shm_data(shm_list_ptr->ptr, shm_key, shm_var.s);

/* free string */
smart_str_free(&shm_var);

if (ret == -1) {
if (!ret) {
php_error_docref(NULL, E_WARNING, "Not enough shared memory left");
RETURN_FALSE;
}
Expand All @@ -283,7 +284,6 @@ PHP_FUNCTION(shm_get_var)
zend_long shm_key;
sysvshm_shm *shm_list_ptr;
char *shm_data;
zend_long shm_varpos;
sysvshm_chunk *shm_var;
php_unserialize_data_t var_hash;

Expand All @@ -299,9 +299,9 @@ PHP_FUNCTION(shm_get_var)

/* setup string-variable and serialize */
/* get serialized variable from shared memory */
shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);
ssize_t shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);

if (shm_varpos < 0) {
if (shm_varpos == -1) {
php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key);
RETURN_FALSE;
}
Expand Down Expand Up @@ -342,7 +342,7 @@ PHP_FUNCTION(shm_has_var)
PHP_FUNCTION(shm_remove_var)
{
zval *shm_id;
zend_long shm_key, shm_varpos;
zend_long shm_key;
sysvshm_shm *shm_list_ptr;

if (SUCCESS != zend_parse_parameters(ZEND_NUM_ARGS(), "Ol", &shm_id, sysvshm_ce, &shm_key)) {
Expand All @@ -355,9 +355,9 @@ PHP_FUNCTION(shm_remove_var)
RETURN_THROWS();
}

shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);
ssize_t shm_varpos = php_check_shm_data(shm_list_ptr->ptr, shm_key);

if (shm_varpos < 0) {
if (shm_varpos == -1) {
php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key);
RETURN_FALSE;
}
Expand All @@ -366,44 +366,41 @@ PHP_FUNCTION(shm_remove_var)
}
/* }}} */

/* {{{ php_put_shm_data
/* {{{
* inserts an ascii-string into shared memory */
static int php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const char *data, zend_long len)
static bool php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const zend_string *data)
{
sysvshm_chunk *shm_var;
zend_long total_size;
zend_long shm_varpos;
ssize_t shm_varpos;

total_size = ((zend_long) (len + sizeof(sysvshm_chunk) - 1) / sizeof(zend_long)) * sizeof(zend_long) + sizeof(zend_long); /* zend_long alligment */
size_t total_size = ((zend_long) (ZSTR_LEN(data) + sizeof(sysvshm_chunk) - 1) / sizeof(zend_long)) * sizeof(zend_long) + sizeof(zend_long); /* zend_long alligment */

if ((shm_varpos = php_check_shm_data(ptr, key)) > 0) {
php_remove_shm_data(ptr, shm_varpos);
}

if (ptr->free < total_size) {
return -1; /* not enough memory */
return false; /* not enough memory */
}

shm_var = (sysvshm_chunk *) ((char *) ptr + ptr->end);
shm_var->key = key;
shm_var->length = len;
shm_var->length = ZSTR_LEN(data);
shm_var->next = total_size;
memcpy(&(shm_var->mem), data, len);
memcpy(&(shm_var->mem), ZSTR_VAL(data), ZSTR_LEN(data));
ptr->end += total_size;
ptr->free -= total_size;
return 0;
return true;
}
/* }}} */

/* {{{ php_check_shm_data */
static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key)
static ssize_t php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key)
{
zend_long pos;
sysvshm_chunk *shm_var;

ZEND_ASSERT(ptr);

pos = ptr->start;
size_t pos = ptr->start;

for (;;) {
if (pos >= ptr->end) {
Expand All @@ -421,27 +418,22 @@ static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key)
}
return -1;
}
/* }}} */

/* {{{ php_remove_shm_data */
static int php_remove_shm_data(sysvshm_chunk_head *ptr, zend_long shm_varpos)
static void php_remove_shm_data(sysvshm_chunk_head *ptr, size_t shm_varpos)
{
sysvshm_chunk *chunk_ptr, *next_chunk_ptr;
zend_long memcpy_len;

ZEND_ASSERT(ptr);

chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos);
next_chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos + chunk_ptr->next);

memcpy_len = ptr->end-shm_varpos - chunk_ptr->next;
size_t memcpy_len = ptr->end-shm_varpos - chunk_ptr->next;
ptr->free += chunk_ptr->next;
ptr->end -= chunk_ptr->next;
if (memcpy_len > 0) {
memmove(chunk_ptr, next_chunk_ptr, memcpy_len);
}
return 0;
}
/* }}} */

#endif /* HAVE_SYSVSHM */
2 changes: 1 addition & 1 deletion ext/sysvshm/sysvshm.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ final class SysvSharedMemory

function shm_attach(int $key, ?int $size = null, int $permissions = 0666): SysvSharedMemory|false {}

function shm_detach(SysvSharedMemory $shm): bool {}
function shm_detach(SysvSharedMemory $shm): true {}

function shm_has_var(SysvSharedMemory $shm, int $key): bool {}

Expand Down
Loading