Ensure ctype_string is NULL for C locale #5542
Conversation
| @@ -1462,7 +1462,7 @@ PHPAPI zend_string *php_string_tolower(zend_string *s) | |||
| unsigned char *c; | |||
| const unsigned char *e; | |||
|
|
|||
| if (EXPECTED(!BG(locale_changed))) { | |||
| if (EXPECTED(!BG(ctype_string))) { | |||
There was a problem hiding this comment.
Unrelated: What's the reasoning for performing this operation for strtolower() but not strtoupper()?
There was a problem hiding this comment.
Because there is no zend_string_toupper :) We only perform ASCII lowercase internally.
| @@ -4801,7 +4801,12 @@ PHP_FUNCTION(setlocale) | |||
| if (BG(ctype_string)) { | |||
| zend_string_release_ex(BG(ctype_string), 0); | |||
| } | |||
| if (len == ZSTR_LEN(loc) && !memcmp(ZSTR_VAL(loc), retval, len)) { | |||
| if (len == 1 && *retval == 'C') { | |||
There was a problem hiding this comment.
I'm fairly certain that retval was the original locale, not the new locale. - setlocale() and php_my_setlocale() return the original locale if it changed.
This should check ZSTR_VAL(loc) instead, e.g. zend_string_equals_literal(locale, "C")
There was a problem hiding this comment.
setlocale() returns the locale that was set, not the previous one. This is necessary because it may not be the same as what was passed to the setlocale() call, e.g. when using "" environment locale.
There was a problem hiding this comment.
Ok, good. I was mixed up with other C and PHP APIs that did that - the c code is returning the new locale
https://en.cppreference.com/w/c/locale/setlocale
https://www.php.net/manual/en/function.setlocale.php#refsect1-function.setlocale-returnvalues
|
AppVeyor failure is unrelated (and maybe fixed by d38f819, we'll see...) |
We already document that
BG(ctype_locale)is NULL for C locale:php-src/ext/standard/basic_functions.h
Line 65 in 3f76947
This change makes sure that this is always the case, even if
setlocale()is explicitly called with"C"locale. NowBG(ctype_local) != NULLcan be used as a quick check whether the C locale is used.