Re: cvs: php-src /ext/standard string.c

On Aug 16, 2005, at 5:04 AM, Rolland Santimano wrote:

+ if (Z_TYPE_P(delim) != IS_UNICODE && Z_TYPE_P(delim) != IS_BINARY) { + convert_to_string_ex(&delim); + } + Z_TYPE_P(retval) = return_type = Z_TYPE_P(delim); /* ... to start off */

This is not correct. If the delimiter is not Unicode or binary, we can't just convert it to string. It needs to be dependent on UG(unicode) setting. Please use convert_to_text_ex() macro for that.

+ for (i = 1 ; i <= numelems ; i++) { + if (zend_hash_get_current_data_ex(Z_ARRVAL_P(arr), (void **)&tmp, &pos) != SUCCESS) { + /* Shouldn't happen ? */ + return; }

The preferred way of iterating over Zend hashes is exactly as it was before your change, with a while() or for() loop without relying on what zend_hash_num_elements() returns.

zend_hash_move_forward_ex(Z_ARRVAL_P(arr), &pos); + if (Z_TYPE_PP(tmp) != return_type) { + /* Convert to common type, if possible */ + if (return_type == IS_UNICODE) { + if (Z_TYPE_PP(tmp) == IS_BINARY) { + /* ERROR */ + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Mixed string types");

Error message needs to be more explicit, "Cannot mix binary type with other string types" perhaps.

+ /* Append elem */ + if (return_type == IS_UNICODE) { + Z_USTRVAL_P(retval) = eurealloc(Z_USTRVAL_P(retval), + Z_USTRLEN_P(retval)+Z_USTRLEN_PP(tmp)); + memcpy(Z_USTRVAL_P(retval)+Z_USTRLEN_P(retval), + Z_USTRVAL_PP(tmp), Z_USTRLEN_PP(tmp)*sizeof(UChar));

You can use UBYTES() macro here.

/* }}} */ @@ -1178,41 +1257,41 @@

    Joins array elements placing glue string between items and return one string */

PHP_FUNCTION(implode) { - zval **arg1 = NULL, **arg2 = NULL, *delim, *arr; - int argc = ZEND_NUM_ARGS(); + zval **arg1 = NULL, **arg2 = NULL; + zval *delim, *arr; + int argc = ZEND_NUM_ARGS(); - if (argc < 1 || argc > 2 || - zend_get_parameters_ex(argc, &arg1, &arg2) == FAILURE) { + if (argc < 1 || argc > 2) { WRONG_PARAM_COUNT; } + if (zend_get_parameters_ex(argc, &arg1, &arg2) == FAILURE) { + return; + } if (argc == 1) { if (Z_TYPE_PP(arg1) != IS_ARRAY) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Argument to implode must be an array."); - return; + RETURN_FALSE; + } else { + MAKE_STD_ZVAL(delim); + ZVAL_STRINGL(delim, "", sizeof("")-1, 0); + SEPARATE_ZVAL(arg1); + arr = *arg1; }

Why not make a delim of the correct type here right away instead of passing it off to convert_to_text_ex() in php_implode()? -Andrei