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.

Author: nikic

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
  *

ext/intl/collator/collator_convert.h

@@ -23,14 +23,14 @@ 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 );
 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

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;
 }

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;