Skip to content

streams: Various minor refactorings #19106

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

Merged
merged 19 commits into from
Aug 4, 2025
Merged

streams: Various minor refactorings #19106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8a232b4
streams: add const specifier
Girgias Jul 12, 2025
837ab97
streams: reduce scope of variables
Girgias Jul 12, 2025
1847239
streams: use type bool instead of type int
Girgias Jul 12, 2025
b322edd
streams: use type zend_result instead of type int
Girgias Jul 12, 2025
95c50e4
streams: use type size_t instead of type unsigned int
Girgias Jul 12, 2025
663c67e
streams: use int type instead of long
Girgias Jul 12, 2025
96299cc
streams: use precomputed persistant variable
Girgias Jul 12, 2025
14ed8cf
streams: remove confusing step variable
Girgias Jul 12, 2025
ff8fa7a
streams: use -1 directly instead of FAILURE macro
Girgias Jul 12, 2025
2772f73
streams: use call in if statement directly
Girgias Jul 12, 2025
a65efe3
streams: remove useless casts
Girgias Jul 12, 2025
f0f2a17
streams: refactor implementation of stream_select()
Girgias Jul 12, 2025
e0b4e2f
streams: use RETURN_BOOL() when possible
Girgias Jul 12, 2025
59a9975
streams: voidify php_stream_filter_prepend_ex()
Girgias Jul 12, 2025
c0cacef
streams: drop unused includes
Girgias Jul 12, 2025
37bfd3d
streams: use an enum for return type of _php_stream_make_seekable()
Girgias Jul 12, 2025
f16cf77
streams: use %zu printf specifier for size_t
Girgias Jul 12, 2025
7af46fd
streams: use type php_socket_t instead of type int
Girgias Jul 12, 2025
0db4f60
streams: refactor statbuf_from_array()
Girgias Jul 12, 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
2 changes: 1 addition & 1 deletion ext/pgsql/pgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6389,7 +6389,7 @@ PHP_FUNCTION(pg_socket_poll)
Z_PARAM_LONG(ts)
ZEND_PARSE_PARAMETERS_END();

if (php_stream_cast(stream, PHP_STREAM_AS_SOCKETD, (void **)&socket, 0)) {
if (UNEXPECTED(php_stream_cast(stream, PHP_STREAM_AS_SOCKETD, (void **)&socket, 0) == FAILURE)) {
zend_argument_type_error(1, "invalid resource socket");
RETURN_THROWS();
}
Expand Down
2 changes: 1 addition & 1 deletion ext/sockets/sockets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2571,7 +2571,7 @@ PHP_FUNCTION(socket_import_stream)
ZEND_PARSE_PARAMETERS_END();
php_stream_from_zval(stream, zstream);

if (php_stream_cast(stream, PHP_STREAM_AS_SOCKETD, (void**)&socket, 1)) {
if (php_stream_cast(stream, PHP_STREAM_AS_SOCKETD, (void**)&socket, 1) == FAILURE) {
/* error supposedly already shown */
RETURN_FALSE;
}
Expand Down
138 changes: 46 additions & 92 deletions ext/standard/streamsfuncs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ PHP_FUNCTION(stream_socket_client)
zval *zerrno = NULL, *zerrstr = NULL, *zcontext = NULL;
double timeout;
bool timeout_is_null = 1;
php_timeout_ull conv;
struct timeval tv;
char *hashkey = NULL;
php_stream *stream = NULL;
Expand Down Expand Up @@ -138,7 +137,7 @@ PHP_FUNCTION(stream_socket_client)
if (timeout < 0.0 || timeout >= (double) PHP_TIMEOUT_ULL_MAX / 1000000.0) {
tv_pointer = NULL;
} else {
conv = (php_timeout_ull) (timeout * 1000000.0);
php_timeout_ull conv = (php_timeout_ull) (timeout * 1000000.0);
#ifdef PHP_WIN32
tv.tv_sec = (long)(conv / 1000000);
tv.tv_usec = (long)(conv % 1000000);
Expand Down Expand Up @@ -262,7 +261,6 @@ PHP_FUNCTION(stream_socket_accept)
bool timeout_is_null = 1;
zval *zpeername = NULL;
zend_string *peername = NULL;
php_timeout_ull conv;
struct timeval tv;
php_stream *stream = NULL, *clistream = NULL;
zend_string *errstr = NULL;
Expand All @@ -286,7 +284,7 @@ PHP_FUNCTION(stream_socket_accept)
if (timeout < 0.0 || timeout >= (double) PHP_TIMEOUT_ULL_MAX / 1000000.0) {
tv_pointer = NULL;
} else {
conv = (php_timeout_ull) (timeout * 1000000.0);
php_timeout_ull conv = (php_timeout_ull) (timeout * 1000000.0);
#ifdef PHP_WIN32
tv.tv_sec = (long)(conv / 1000000);
tv.tv_usec = (long)(conv % 1000000);
Expand Down Expand Up @@ -484,7 +482,6 @@ PHP_FUNCTION(stream_copy_to_stream)
zend_long maxlen, pos = 0;
bool maxlen_is_null = 1;
size_t len;
int ret;

ZEND_PARSE_PARAMETERS_START(2, 4)
PHP_Z_PARAM_STREAM(src)
Expand All @@ -503,9 +500,7 @@ PHP_FUNCTION(stream_copy_to_stream)
RETURN_FALSE;
}

ret = php_stream_copy_to_stream_ex(src, dest, maxlen, &len);

if (ret != SUCCESS) {
if (php_stream_copy_to_stream_ex(src, dest, maxlen, &len) != SUCCESS) {
RETURN_FALSE;
}
RETURN_LONG(len);
Expand Down Expand Up @@ -534,9 +529,9 @@ PHP_FUNCTION(stream_get_meta_data)
add_assoc_zval(return_value, "wrapper_data", &stream->wrapperdata);
}
if (stream->wrapper) {
add_assoc_string(return_value, "wrapper_type", (char *)stream->wrapper->wops->label);
add_assoc_string(return_value, "wrapper_type", stream->wrapper->wops->label);
}
add_assoc_string(return_value, "stream_type", (char *)stream->ops->label);
add_assoc_string(return_value, "stream_type", stream->ops->label);

add_assoc_string(return_value, "mode", stream->mode);

Expand All @@ -548,7 +543,7 @@ PHP_FUNCTION(stream_get_meta_data)
array_init(newval);

for (filter = stream->filterhead; filter != NULL; filter = filter->next) {
add_next_index_string(newval, (char *)filter->fops->label);
add_next_index_string(newval, filter->fops->label);
}

add_assoc_zval(return_value, "filters", newval);
Expand All @@ -568,13 +563,13 @@ PHP_FUNCTION(stream_get_meta_data)
/* {{{ Retrieves list of registered socket transports */
PHP_FUNCTION(stream_get_transports)
{
HashTable *stream_xport_hash;
zend_string *stream_xport;

ZEND_PARSE_PARAMETERS_NONE();

stream_xport_hash = php_stream_xport_get_hash();
array_init(return_value);

const HashTable *stream_xport_hash = php_stream_xport_get_hash();
zend_string *stream_xport;
ZEND_HASH_MAP_FOREACH_STR_KEY(stream_xport_hash, stream_xport) {
add_next_index_str(return_value, zend_string_copy(stream_xport));
} ZEND_HASH_FOREACH_END();
Expand All @@ -584,13 +579,12 @@ PHP_FUNCTION(stream_get_transports)
/* {{{ Retrieves list of registered stream wrappers */
PHP_FUNCTION(stream_get_wrappers)
{
HashTable *url_stream_wrappers_hash;
zend_string *stream_protocol;

ZEND_PARSE_PARAMETERS_NONE();

url_stream_wrappers_hash = php_stream_get_url_stream_wrappers_hash();
array_init(return_value);

const HashTable *url_stream_wrappers_hash = php_stream_get_url_stream_wrappers_hash();
zend_string *stream_protocol;
ZEND_HASH_MAP_FOREACH_STR_KEY(url_stream_wrappers_hash, stream_protocol) {
if (stream_protocol) {
add_next_index_str(return_value, zend_string_copy(stream_protocol));
Expand All @@ -601,17 +595,13 @@ PHP_FUNCTION(stream_get_wrappers)
/* }}} */

/* {{{ stream_select related functions */
static int stream_array_to_fd_set(zval *stream_array, fd_set *fds, php_socket_t *max_fd)
static int stream_array_to_fd_set(const HashTable *stream_array, fd_set *fds, php_socket_t *max_fd)
{
zval *elem;
php_stream *stream;
int cnt = 0;

if (Z_TYPE_P(stream_array) != IS_ARRAY) {
return 0;
}

ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(stream_array), elem) {
ZEND_HASH_FOREACH_VAL(stream_array, elem) {
/* Temporary int fd is needed for the STREAM data type on windows, passing this_fd directly to php_stream_cast()
would eventually bring a wrong result on x64. php_stream_cast() casts to int internally, and this will leave
the higher bits of a SOCKET variable uninitialized on systems with little endian. */
Expand Down Expand Up @@ -640,7 +630,7 @@ static int stream_array_to_fd_set(zval *stream_array, fd_set *fds, php_socket_t
return cnt ? 1 : 0;
}

static int stream_array_from_fd_set(zval *stream_array, fd_set *fds)
static int stream_array_from_fd_set(zval *stream_array, const fd_set *fds)
{
zval *elem, *dest_elem;
HashTable *ht;
Expand All @@ -649,9 +639,7 @@ static int stream_array_from_fd_set(zval *stream_array, fd_set *fds)
zend_string *key;
zend_ulong num_ind;

if (Z_TYPE_P(stream_array) != IS_ARRAY) {
return 0;
}
ZEND_ASSERT(Z_TYPE_P(stream_array) == IS_ARRAY);
ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));

ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), num_ind, key, elem) {
Expand All @@ -677,7 +665,6 @@ static int stream_array_from_fd_set(zval *stream_array, fd_set *fds)

zval_add_ref(dest_elem);
ret++;
continue;
}
}
} ZEND_HASH_FOREACH_END();
Expand All @@ -698,9 +685,7 @@ static int stream_array_emulate_read_fd_set(zval *stream_array)
zend_ulong num_ind;
zend_string *key;

if (Z_TYPE_P(stream_array) != IS_ARRAY) {
return 0;
}
ZEND_ASSERT(Z_TYPE_P(stream_array) == IS_ARRAY);
ht = zend_new_array(zend_hash_num_elements(Z_ARRVAL_P(stream_array)));

ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(stream_array), num_ind, key, elem) {
Expand All @@ -723,7 +708,6 @@ static int stream_array_emulate_read_fd_set(zval *stream_array)
}
zval_add_ref(dest_elem);
ret++;
continue;
}
} ZEND_HASH_FOREACH_END();

Expand Down Expand Up @@ -766,21 +750,21 @@ PHP_FUNCTION(stream_select)
FD_ZERO(&efds);

if (r_array != NULL) {
set_count = stream_array_to_fd_set(r_array, &rfds, &max_fd);
Copy link
Member

Choose a reason for hiding this comment

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

One could wonder why the ZPP doesn't use Z_PARAM_ARRAY_HT_EX2 in the first place

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 think because the parameter is a reference that is also written to, and so you need it to be a zval?

Copy link
Member

Choose a reason for hiding this comment

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

It is a ref that is written to, but that doesn't mean it needs to be a zval?

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 frankly never understand how PHP references are meant to work, and how to deal with them in functions.

Will try and see if I can just use Z_PARAM_ARRAY_HT_EX2 directly :)

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 really don't see how to handle the zval* being destroyed and reassigned to when using the HT directly, so I'll leave this for someone else to figure out.

set_count = stream_array_to_fd_set(Z_ARR_P(r_array), &rfds, &max_fd);
if (set_count > max_set_count)
max_set_count = set_count;
sets += set_count;
}

if (w_array != NULL) {
set_count = stream_array_to_fd_set(w_array, &wfds, &max_fd);
set_count = stream_array_to_fd_set(Z_ARR_P(w_array), &wfds, &max_fd);
if (set_count > max_set_count)
max_set_count = set_count;
sets += set_count;
}

if (e_array != NULL) {
set_count = stream_array_to_fd_set(e_array, &efds, &max_fd);
set_count = stream_array_to_fd_set(Z_ARR_P(e_array), &efds, &max_fd);
if (set_count > max_set_count)
max_set_count = set_count;
sets += set_count;
Expand Down Expand Up @@ -884,7 +868,7 @@ static void user_space_stream_notifier_dtor(php_stream_notifier *notifier)
notifier->fcc = NULL;
}

static zend_result parse_context_options(php_stream_context *context, HashTable *options)
static zend_result parse_context_options(php_stream_context *context, const HashTable *options)
{
zval *wval, *oval;
zend_string *wkey, *okey;
Expand All @@ -908,7 +892,7 @@ static zend_result parse_context_options(php_stream_context *context, HashTable
return SUCCESS;
}

static zend_result parse_context_params(php_stream_context *context, HashTable *params)
static zend_result parse_context_params(php_stream_context *context, const HashTable *params)
{
zval *tmp;

Expand Down Expand Up @@ -1220,15 +1204,14 @@ PHP_FUNCTION(stream_context_create)
/* }}} */

/* {{{ streams filter functions */
static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
static void apply_filter_to_stream(bool append, INTERNAL_FUNCTION_PARAMETERS)
{
php_stream *stream;
char *filtername;
size_t filternamelen;
zend_long read_write = 0;
zval *filterparams = NULL;
php_stream_filter *filter = NULL;
int ret;

ZEND_PARSE_PARAMETERS_START(2, 4)
PHP_Z_PARAM_STREAM(stream)
Expand Down Expand Up @@ -1259,13 +1242,13 @@ static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
}

if (append) {
ret = php_stream_filter_append_ex(&stream->readfilters, filter);
zend_result ret = php_stream_filter_append_ex(&stream->readfilters, filter);
if (ret != SUCCESS) {
php_stream_filter_remove(filter, 1);
RETURN_FALSE;
}
} else {
ret = php_stream_filter_prepend_ex(&stream->readfilters, filter);
}
if (ret != SUCCESS) {
php_stream_filter_remove(filter, 1);
RETURN_FALSE;
php_stream_filter_prepend_ex(&stream->readfilters, filter);
}
}

Expand All @@ -1276,13 +1259,13 @@ static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
}

if (append) {
ret = php_stream_filter_append_ex(&stream->writefilters, filter);
zend_result ret = php_stream_filter_append_ex(&stream->writefilters, filter);
if (ret != SUCCESS) {
php_stream_filter_remove(filter, 1);
RETURN_FALSE;
}
} else {
ret = php_stream_filter_prepend_ex(&stream->writefilters, filter);
}
if (ret != SUCCESS) {
php_stream_filter_remove(filter, 1);
RETURN_FALSE;
php_stream_filter_prepend_ex(&stream->writefilters, filter);
}
}

Expand Down Expand Up @@ -1380,11 +1363,7 @@ PHP_FUNCTION(stream_set_blocking)
Z_PARAM_BOOL(block)
ZEND_PARSE_PARAMETERS_END();

if (php_stream_set_option(stream, PHP_STREAM_OPTION_BLOCKING, block, NULL) == -1) {
RETURN_FALSE;
}

RETURN_TRUE;
RETURN_BOOL(-1 != php_stream_set_option(stream, PHP_STREAM_OPTION_BLOCKING, block, NULL));
}

/* }}} */
Expand Down Expand Up @@ -1425,11 +1404,7 @@ PHP_FUNCTION(stream_set_timeout)
}
#endif

if (PHP_STREAM_OPTION_RETURN_OK == php_stream_set_option(stream, PHP_STREAM_OPTION_READ_TIMEOUT, 0, &t)) {
RETURN_TRUE;
}

RETURN_FALSE;
RETURN_BOOL(PHP_STREAM_OPTION_RETURN_OK == php_stream_set_option(stream, PHP_STREAM_OPTION_READ_TIMEOUT, 0, &t));
}
#endif /* HAVE_SYS_TIME_H || defined(PHP_WIN32) */
/* }}} */
Expand Down Expand Up @@ -1605,11 +1580,7 @@ PHP_FUNCTION(stream_is_local)
wrapper = php_stream_locate_url_wrapper(Z_STRVAL_P(zstream), NULL, 0);
}

if (!wrapper) {
RETURN_FALSE;
}

RETURN_BOOL(wrapper->is_url==0);
RETURN_BOOL(wrapper && wrapper->is_url == 0);
}
/* }}} */

Expand All @@ -1622,11 +1593,7 @@ PHP_FUNCTION(stream_supports_lock)
PHP_Z_PARAM_STREAM(stream)
ZEND_PARSE_PARAMETERS_END();

if (!php_stream_supports_lock(stream)) {
RETURN_FALSE;
}

RETURN_TRUE;
RETURN_BOOL(php_stream_supports_lock(stream));
}

/* {{{ Check if a stream is a TTY. */
Expand All @@ -1653,15 +1620,13 @@ PHP_FUNCTION(stream_isatty)

#ifdef PHP_WIN32
/* Check if the Windows standard handle is redirected to file */
RETVAL_BOOL(php_win32_console_fileno_is_console(fileno));
RETURN_BOOL(php_win32_console_fileno_is_console(fileno));
#elif defined(HAVE_UNISTD_H)
/* Check if the file descriptor identifier is a terminal */
RETVAL_BOOL(isatty(fileno));
RETURN_BOOL(isatty(fileno));
#else
{
zend_stat_t stat = {0};
RETVAL_BOOL(zend_fstat(fileno, &stat) == 0 && (stat.st_mode & /*S_IFMT*/0170000) == /*S_IFCHR*/0020000);
}
zend_stat_t stat = {0};
RETURN_BOOL(zend_fstat(fileno, &stat) == 0 && (stat.st_mode & /*S_IFMT*/0170000) == /*S_IFCHR*/0020000);
#endif
}

Expand Down Expand Up @@ -1707,21 +1672,10 @@ PHP_FUNCTION(sapi_windows_vt100_support)

if (enable_is_null) {
/* Check if the Windows standard handle has VT100 control codes enabled */
if (php_win32_console_fileno_has_vt100(fileno)) {
RETURN_TRUE;
}
else {
RETURN_FALSE;
}
}
else {
RETURN_BOOL(php_win32_console_fileno_has_vt100(fileno));
} else {
/* Enable/disable VT100 control codes support for the specified Windows standard handle */
if (php_win32_console_fileno_set_vt100(fileno, enable ? TRUE : FALSE)) {
RETURN_TRUE;
}
else {
RETURN_FALSE;
}
RETURN_BOOL(php_win32_console_fileno_set_vt100(fileno, enable ? TRUE : FALSE));
}
}
#endif
Expand Down
Loading
Loading