zend_string: Support NUL bytes in ZSTR_*_LITERAL() and zend_string_*literal*()
#19582
Conversation
TimWolla
requested review from
kocsismate,
dstogov and
nielsdos
as code owners
TimWolla
force-pushed
the
zstr-init-literal-nul
branch
from
5addc69 to
c76cf66
Compare
There was a problem hiding this comment.
I'm around a -0.5 on this generally, and maybe -0.75 on this for PHP 8.5 - @edorian, what do you think?
UPGRADING.INTERNALS
Outdated
| @@ -86,6 +86,8 @@ PHP 8.5 INTERNALS UPGRADE NOTES | |||
| . zend_mm_refresh_key_child() must be called on any zend_mm_heap inherited | |||
| from the parent process after a fork(). | |||
| . HASH_KEY_IS_* constants have been moved in the zend_hash_key_type enum. | |||
| . ZSTR_INIT_LITERAL() now supports strings containing NUL bytes. Passing | |||
| non-literal char* is no longer supported. | |||
There was a problem hiding this comment.
The dropping of support for non-literal char* seems like a disadvantage - I'm not fully convinced of the benefits of this patch (were there any places that the macro was specifically avoided because of NUL bytes?) especially given where we are in the release cycle, where external extensions have likely already started updating for PHP 8.5.
There was a problem hiding this comment.
were there any places that the macro was specifically avoided because of NUL bytes?
I did not check. Your comment on the other PR prompted me to look into the definition of the macro and then I noticed the issue.
The dropping of support for non-literal char* seems like a disadvantage
The “literal” in the name of the macro to me is an indication that it was not meant to support non-literal strings. Just like we have zend_string_starts_with_cstr() (char*) + length and zend_string_starts_with_literal().
But I'm seeing that zend_string_starts_with_literal() has the same issue as ZSTR_INIT_LITERAL(). They should all be adjusted at the same time then and then it also makes sense to me to move this to 8.6.
There was a problem hiding this comment.
Ah zend_string_equals_literal() is fine, zend_string_starts_with_literal() and zend_string_starts_with_literal_ci() are not.
TimWolla
force-pushed
the
zstr-init-literal-nul
branch
from
c76cf66 to
d7bfdcc
Compare
TimWolla
changed the title
ZSTR_INIT_LITERAL()ZSTR_*_LITERAL() and zend_string_*literal*()
|
As the person who introduced |
TimWolla
force-pushed
the
zstr-init-literal-nul
branch
from
d7bfdcc to
3ba7252
Compare
Following #19565 (comment).