From: Robin Fernandes Date: Mon, 4 Aug 2008 11:45:15 +0000 (+0000) Subject: Improve ArrayObject::exchangeArray() behaviour with objects and CoW references (see... X-Git-Tag: BEFORE_HEAD_NS_CHANGE~880 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4ef68b9627f10306d6ff49adf19b0cfb8f3e1099;p=php Improve ArrayObject::exchangeArray() behaviour with objects and CoW references (see http://turl.ca/exarr ). --- diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 64b7fff280..e3f370f200 100755 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -899,6 +899,51 @@ static void spl_array_it_rewind(zend_object_iterator *iter TSRMLS_DC) /* {{{ */ } /* }}} */ +/* {{{ spl_array_set_array */ +static void spl_array_set_array(zval *object, spl_array_object *intern, zval **array, long ar_flags, int just_array TSRMLS_DC) { + + if (Z_TYPE_PP(array) == IS_ARRAY) { + SEPARATE_ZVAL_IF_NOT_REF(array); + } + + if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) == &spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) { + zval_ptr_dtor(&intern->array); + if (just_array) { + spl_array_object *other = (spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC); + ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK; + } + ar_flags |= SPL_ARRAY_USE_OTHER; + intern->array = *array; + } else { + if (Z_TYPE_PP(array) != IS_OBJECT && Z_TYPE_PP(array) != IS_ARRAY) { + php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC); + zend_throw_exception(spl_ce_InvalidArgumentException, "Passed variable is not an array or object, using empty array instead", 0 TSRMLS_CC); + return; + } + zval_ptr_dtor(&intern->array); + intern->array = *array; + } + if (object == *array) { + intern->ar_flags |= SPL_ARRAY_IS_SELF; + intern->ar_flags &= ~SPL_ARRAY_USE_OTHER; + } else { + intern->ar_flags &= ~SPL_ARRAY_IS_SELF; + } + intern->ar_flags |= ar_flags; + Z_ADDREF_P(intern->array); + if (Z_TYPE_PP(array) == IS_OBJECT) { + zend_object_get_properties_t handler = Z_OBJ_HANDLER_PP(array, get_properties); + if ((handler != std_object_handlers.get_properties && handler != spl_array_get_properties) + || !spl_array_get_hash_table(intern, 0 TSRMLS_CC)) { + php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC); + zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0 TSRMLS_CC, "Overloaded object of type %v is not compatible with %v", Z_OBJCE_PP(array)->name, intern->std.ce->name); + } + } + + spl_array_rewind(intern TSRMLS_CC); +} +/* }}} */ + /* {{{ iterator handler table */ zend_object_iterator_funcs spl_array_it_funcs = { spl_array_it_dtor, @@ -954,53 +999,13 @@ SPL_METHOD(Array, __construct) return; } - if (Z_TYPE_PP(array) == IS_ARRAY) { - SEPARATE_ZVAL_IF_NOT_REF(array); - } - if (ZEND_NUM_ARGS() > 2) { intern->ce_get_iterator = ce_get_iterator; } ar_flags &= ~SPL_ARRAY_INT_MASK; - if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) == &spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) { - zval_ptr_dtor(&intern->array); - if (ZEND_NUM_ARGS() == 1) - { - spl_array_object *other = (spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC); - ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK; - } - ar_flags |= SPL_ARRAY_USE_OTHER; - intern->array = *array; - } else { - if (Z_TYPE_PP(array) != IS_OBJECT && Z_TYPE_PP(array) != IS_ARRAY) { - php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC); - zend_throw_exception(spl_ce_InvalidArgumentException, "Passed variable is not an array or object, using empty array instead", 0 TSRMLS_CC); - return; - } - zval_ptr_dtor(&intern->array); - intern->array = *array; - } - if (object == *array) { - intern->ar_flags |= SPL_ARRAY_IS_SELF; - intern->ar_flags &= ~SPL_ARRAY_USE_OTHER; - } else { - intern->ar_flags &= ~SPL_ARRAY_IS_SELF; - } - intern->ar_flags |= ar_flags; - Z_ADDREF_P(intern->array); - if (Z_TYPE_PP(array) == IS_OBJECT) { - zend_object_get_properties_t handler = Z_OBJ_HANDLER_PP(array, get_properties); - if ((handler != std_object_handlers.get_properties && handler != spl_array_get_properties) - || !spl_array_get_hash_table(intern, 0 TSRMLS_CC)) { - php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC); - zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0 TSRMLS_CC, "Overloaded object of type %v is not compatible with %v", Z_OBJCE_PP(array)->name, intern->std.ce->name); - return; - } - } - - spl_array_rewind(intern TSRMLS_CC); + spl_array_set_array(object, intern, array, ar_flags, ZEND_NUM_ARGS() == 1 TSRMLS_CC); php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC); } @@ -1074,31 +1079,9 @@ SPL_METHOD(Array, exchangeArray) if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z", &array) == FAILURE) { return; } - if (Z_TYPE_PP(array) == IS_OBJECT && intern == (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC)) { - zval_ptr_dtor(&intern->array); - array = &object; - intern->array = object; - } else if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) == &spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) { - spl_array_object *other = (spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC); - zval_ptr_dtor(&intern->array); - intern->array = other->array; - } else { - if (Z_TYPE_PP(array) != IS_OBJECT && !HASH_OF(*array)) { - zend_throw_exception(spl_ce_InvalidArgumentException, "Passed variable is not an array or object, using empty array instead", 0 TSRMLS_CC); - return; - } - zval_ptr_dtor(&intern->array); - intern->array = *array; - intern->ar_flags &= ~SPL_ARRAY_USE_OTHER; - } - if (object == *array) { - intern->ar_flags |= SPL_ARRAY_IS_SELF; - } else { - intern->ar_flags &= ~SPL_ARRAY_IS_SELF; - } - Z_ADDREF_P(intern->array); - spl_array_rewind(intern TSRMLS_CC); + spl_array_set_array(object, intern, array, 0L, 1 TSRMLS_CC); + } /* }}} */ diff --git a/ext/spl/tests/arrayObject_exchangeArray_basic1.phpt b/ext/spl/tests/arrayObject_exchangeArray_basic1.phpt new file mode 100644 index 0000000000..71550268a7 --- /dev/null +++ b/ext/spl/tests/arrayObject_exchangeArray_basic1.phpt @@ -0,0 +1,41 @@ +--TEST-- +SPL: ArrayObject::exchangeArray() and copy-on-write references +--FILE-- +exchangeArray($swapIn); + +$ao['a'] = 'adding element to $ao'; +$swapIn['b'] = 'adding element to $swapIn'; +$ao['c'] = 'adding another element to $ao'; + +echo "\n--> swapIn: "; +var_dump($swapIn); + +echo "\n--> cowRef: "; +var_dump($cowRef); + +echo "\n--> ao: "; +var_dump($ao); +?> +--EXPECTF-- + +--> swapIn: array(1) { + [u"b"]=> + unicode(25) "adding element to $swapIn" +} + +--> cowRef: array(0) { +} + +--> ao: object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + array(2) { + [u"a"]=> + unicode(21) "adding element to $ao" + [u"c"]=> + unicode(29) "adding another element to $ao" + } +} \ No newline at end of file diff --git a/ext/spl/tests/arrayObject_exchangeArray_basic2.phpt b/ext/spl/tests/arrayObject_exchangeArray_basic2.phpt new file mode 100644 index 0000000000..e008f13c14 --- /dev/null +++ b/ext/spl/tests/arrayObject_exchangeArray_basic2.phpt @@ -0,0 +1,97 @@ +--TEST-- +SPL: ArrayObject::exchangeArray() with various object arguments +--FILE-- + exchangeArray(array):\n"; +$ao = new ArrayObject(); +$ao->exchangeArray(array('key'=>'original')); +var_dump($ao['key']); +var_dump($ao); + +echo "\n--> exchangeArray(normal object):\n"; +$obj = new stdClass; +$obj->key = 'normal object prop'; +$ao->exchangeArray($obj); +var_dump($ao['key']); +var_dump($ao); + +echo "\n--> exchangeArray(ArrayObject):\n"; +$obj = new ArrayObject(array('key'=>'ArrayObject element')); +$ao->exchangeArray($obj); +var_dump($ao['key']); +var_dump($ao); + +echo "\n--> exchangeArray(ArrayIterator):\n"; +$obj = new ArrayIterator(array('key'=>'ArrayIterator element')); +$ao->exchangeArray($obj); +var_dump($ao['key']); +var_dump($ao); + +echo "\n--> exchangeArray(nested ArrayObject):\n"; +$obj = new ArrayObject(new ArrayObject(array('key'=>'nested ArrayObject element'))); +$ao->exchangeArray($obj); +var_dump($ao['key']); +var_dump($ao); +?> +--EXPECTF-- +--> exchangeArray(array): +unicode(8) "original" +object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + array(1) { + [u"key"]=> + unicode(8) "original" + } +} + +--> exchangeArray(normal object): +unicode(18) "normal object prop" +object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + object(stdClass)#%d (1) { + [u"key"]=> + unicode(18) "normal object prop" + } +} + +--> exchangeArray(ArrayObject): +unicode(19) "ArrayObject element" +object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + array(1) { + [u"key"]=> + unicode(19) "ArrayObject element" + } + } +} + +--> exchangeArray(ArrayIterator): +unicode(21) "ArrayIterator element" +object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + object(ArrayIterator)#%d (1) { + [u"storage":u"ArrayIterator":private]=> + array(1) { + [u"key"]=> + unicode(21) "ArrayIterator element" + } + } +} + +--> exchangeArray(nested ArrayObject): +unicode(26) "nested ArrayObject element" +object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + object(ArrayObject)#%d (1) { + [u"storage":u"ArrayObject":private]=> + array(1) { + [u"key"]=> + unicode(26) "nested ArrayObject element" + } + } + } +} \ No newline at end of file