From 5c5727a57bbe4db322e93f03c3bf3b61f78f1ff2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 10 Jun 2021 10:01:14 +0200 Subject: [PATCH] Avoid make_printable_zval() in intl collator Use zval_get_string() instead. This code is a real mess though, and could use some more thorough cleanup. --- ext/intl/collator/collator_convert.c | 72 +++++++++------------------- ext/intl/collator/collator_convert.h | 4 +- ext/intl/collator/collator_sort.c | 22 ++++----- ext/intl/intl_common.h | 4 +- 4 files changed, 35 insertions(+), 67 deletions(-) diff --git a/ext/intl/collator/collator_convert.c b/ext/intl/collator/collator_convert.c index f570bc4985e..55f2995895b 100644 --- a/ext/intl/collator/collator_convert.c +++ b/ext/intl/collator/collator_convert.c @@ -174,39 +174,27 @@ zval* collator_convert_zstr_utf16_to_utf8( zval* utf16_zval, zval *rv ) } /* }}} */ -/* {{{ collator_convert_zstr_utf8_to_utf16 - * - * Convert string from utf8 to utf16. - * - * @param zval* utf8_zval String to convert. - * - * @return zval* Converted string. - */ -zval* collator_convert_zstr_utf8_to_utf16( zval* utf8_zval, zval *rv ) +zend_string *collator_convert_zstr_utf8_to_utf16(zend_string *utf8_str) { - zval* zstr = NULL; - UChar* ustr = NULL; - int32_t ustr_len = 0; + UChar *ustr = NULL; + int32_t ustr_len = 0; UErrorCode status = U_ZERO_ERROR; /* Convert the string to UTF-16. */ intl_convert_utf8_to_utf16( &ustr, &ustr_len, - Z_STRVAL_P( utf8_zval ), Z_STRLEN_P( utf8_zval ), - &status ); + ZSTR_VAL(utf8_str), ZSTR_LEN(utf8_str), + &status); // FIXME Or throw error or use intl internal error handler - if( U_FAILURE( status ) ) - php_error( E_WARNING, "Error casting object to string in collator_convert_zstr_utf8_to_utf16()" ); + if (U_FAILURE(status)) { + php_error(E_WARNING, + "Error casting object to string in collator_convert_zstr_utf8_to_utf16()"); + } - /* Set string. */ - zstr = rv; - ZVAL_STRINGL( zstr, (char*)ustr, UBYTES(ustr_len)); - //??? + zend_string *zstr = zend_string_init((char *) ustr, UBYTES(ustr_len), 0); efree((char *)ustr); - return zstr; } -/* }}} */ /* {{{ collator_convert_object_to_string * Convert object to UTF16-encoded string. @@ -346,42 +334,26 @@ zval* collator_convert_string_to_number_if_possible( zval* str, zval *rv ) } /* }}} */ -/* {{{ collator_make_printable_zval - * - * Returns string from input zval. +/* Returns string from input zval. * * @param zval* arg zval to get string from * - * @return zval* UTF16 string. + * @return zend_string* UTF16 string. */ -zval* collator_make_printable_zval( zval* arg, zval *rv) +zend_string *collator_zval_to_string(zval *arg) { - zval arg_copy; - zval* str = NULL; - - if( Z_TYPE_P(arg) != IS_STRING ) - { - - int use_copy = zend_make_printable_zval(arg, &arg_copy); - - if( use_copy ) - { - str = collator_convert_zstr_utf8_to_utf16( &arg_copy, rv ); - zval_ptr_dtor_str( &arg_copy ); - } - else - { - str = collator_convert_zstr_utf8_to_utf16( arg, rv ); - } - } - else - { - COLLATOR_CONVERT_RETURN_FAILED( arg ); + // TODO: This is extremely weird in that it leaves pre-existing strings alone and does not + // perform a UTF-8 to UTF-16 conversion for them. The assumption is that values that are + // already strings have already been converted beforehand. It would be good to clean this up. + if (Z_TYPE_P(arg) == IS_STRING) { + return zend_string_copy(Z_STR_P(arg)); } - return str; + zend_string *utf8_str = zval_get_string(arg); + zend_string *utf16_str = collator_convert_zstr_utf8_to_utf16(utf8_str); + zend_string_release(utf8_str); + return utf16_str; } -/* }}} */ /* {{{ collator_normalize_sort_argument * diff --git a/ext/intl/collator/collator_convert.h b/ext/intl/collator/collator_convert.h index 32f2b594659..1d585609643 100644 --- a/ext/intl/collator/collator_convert.h +++ b/ext/intl/collator/collator_convert.h @@ -23,7 +23,7 @@ void collator_convert_hash_from_utf8_to_utf16( HashTable* hash, UErrorCode* stat void collator_convert_hash_from_utf16_to_utf8( HashTable* hash, UErrorCode* status ); zval* collator_convert_zstr_utf16_to_utf8( zval* utf16_zval, zval *rv ); -zval* collator_convert_zstr_utf8_to_utf16( zval* utf8_zval, zval *rv ); +zend_string *collator_convert_zstr_utf8_to_utf16(zend_string *utf8_str); zval* collator_normalize_sort_argument( zval* arg, zval *rv ); zval* collator_convert_object_to_string( zval* obj, zval *rv ); @@ -31,6 +31,6 @@ zval* collator_convert_string_to_number( zval* arg, zval *rv ); zval* collator_convert_string_to_number_if_possible( zval* str, zval *rv ); zval* collator_convert_string_to_double( zval* str, zval *rv ); -zval* collator_make_printable_zval( zval* arg, zval *rv ); +zend_string *collator_zval_to_string(zval *arg); #endif // COLLATOR_CONVERT_H diff --git a/ext/intl/collator/collator_sort.c b/ext/intl/collator/collator_sort.c index 6add83424dc..0634e68fc7a 100644 --- a/ext/intl/collator/collator_sort.c +++ b/ext/intl/collator/collator_sort.c @@ -73,8 +73,8 @@ static int collator_regular_compare_function(zval *result, zval *op1, zval *op2) ZEND_ASSERT(INTL_G(current_collator) != NULL); ZVAL_LONG(result, ucol_strcoll( INTL_G(current_collator), - INTL_Z_STRVAL_P(str1_p), INTL_Z_STRLEN_P(str1_p), - INTL_Z_STRVAL_P(str2_p), INTL_Z_STRLEN_P(str2_p) )); + INTL_ZSTR_VAL(Z_STR_P(str1_p)), INTL_ZSTR_LEN(Z_STR_P(str1_p)), + INTL_ZSTR_VAL(Z_STR_P(str2_p)), INTL_ZSTR_LEN(Z_STR_P(str2_p)) )); } else { @@ -167,23 +167,19 @@ static int collator_numeric_compare_function(zval *result, zval *op1, zval *op2) */ static int collator_icu_compare_function(zval *result, zval *op1, zval *op2) { - zval str1, str2; - int rc = SUCCESS; - zval *str1_p = NULL; - zval *str2_p = NULL; - - str1_p = collator_make_printable_zval( op1, &str1); - str2_p = collator_make_printable_zval( op2, &str2 ); + int rc = SUCCESS; + zend_string *str1 = collator_zval_to_string(op1); + zend_string *str2 = collator_zval_to_string(op2); /* Compare the strings using ICU. */ ZEND_ASSERT(INTL_G(current_collator) != NULL); ZVAL_LONG(result, ucol_strcoll( INTL_G(current_collator), - INTL_Z_STRVAL_P(str1_p), INTL_Z_STRLEN_P(str1_p), - INTL_Z_STRVAL_P(str2_p), INTL_Z_STRLEN_P(str2_p) )); + INTL_ZSTR_VAL(str1), INTL_ZSTR_LEN(str1), + INTL_ZSTR_VAL(str2), INTL_ZSTR_LEN(str2) )); - zval_ptr_dtor( str1_p ); - zval_ptr_dtor( str2_p ); + zend_string_release(str1); + zend_string_release(str2); return rc; } diff --git a/ext/intl/intl_common.h b/ext/intl/intl_common.h index ce421660192..cef96325e9a 100644 --- a/ext/intl/intl_common.h +++ b/ext/intl/intl_common.h @@ -38,8 +38,8 @@ END_EXTERN_C() #define USIZE(data) sizeof((data))/sizeof(UChar) #define UCHARS(len) ((len) / sizeof(UChar)) -#define INTL_Z_STRVAL_P(str) (UChar*) Z_STRVAL_P(str) -#define INTL_Z_STRLEN_P(str) UCHARS( Z_STRLEN_P(str) ) +#define INTL_ZSTR_VAL(str) (UChar*) ZSTR_VAL(str) +#define INTL_ZSTR_LEN(str) UCHARS(ZSTR_LEN(str)) BEGIN_EXTERN_C() extern zend_class_entry *IntlException_ce_ptr;