main: change some PHP globals to zend_string* and uint32_t - #22814
Conversation
github-actions
Bot
added
SAPI: fpm
Extension: standard
SAPI: cgi
SAPI: cli
SAPI: phpdbg
ABI break
labels
d0b5cc4 to
9cc5fe0
Compare
9cc5fe0 to
41e2f2a
Compare
| incomplete_class = 1; | ||
| ce = PHP_IC_ENTRY; | ||
| break; | ||
| } | ||
|
|
||
| /* Call unserialize callback */ | ||
| ZVAL_STRING(&user_func, PG(unserialize_callback_func)); | ||
| ZVAL_STR(&user_func, PG(unserialize_callback_func)); |
There was a problem hiding this comment.
A test along these lines
function unserialize_cb_original($name)
{
ini_set('unserialize_callback_func', 'unserialize_cb_changed');
}
function unserialize_cb_changed($name)
{
}
ini_set('unserialize_callback_func', 'unserialize_cb_original');
unserialize('O:3:"FOO":0:{}');
var_dump(ini_get('unserialize_callback_func'));
ini_restore('unserialize_callback_func');
var_dump(ini_get('unserialize_callback_func'));
along side with USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1 should highlight the underlying refcount issue here.
There was a problem hiding this comment.
I added the test to master and it still seems to work just fine?
There was a problem hiding this comment.
I see the issue as we are zval_dtor the zval... and yet it doesn't fail when not copying... I'm confused.
There was a problem hiding this comment.
ah right ... I m afraid this is going to bite us down the road, I would prefer it copies wdyt ?
There was a problem hiding this comment.
Agreed, let me amend the commit.
This prevents a reallocation
As this is the expected type for lineno everywhere else in the engine
This remove a strlen() computation.
While at it clarify path concatenation code by using the zend_string_concat{2|3} APIs rather than a memcpy and strncpy calls
Allows us to convert a strcmp() call to zend_string_equals_literal() which is less confusing
This prevents a reallocation
41e2f2a to
67e3c4b
Compare
| case 0: | ||
| filename = zend_string_concat3( | ||
| ZSTR_VAL(PG(doc_root)), ZSTR_LEN(PG(doc_root)), | ||
| ZEND_STRL("/"), |
There was a problem hiding this comment.
nit: I just realise I wonder if PHP_DIR_SEPARATOR is not more appropriate
There was a problem hiding this comment.
The issue is that PHP_DIR_SEPARATOR is a char and not a char*, I did thought about using it, but considering Windows doesn't really care I didn't think it was necessary.
2 participants
No description provided.