com php-src: Changed ArrayIterator implementation using zend_hash_iterator_... API. Allowed modific ation of itterated ArrayObject using the same beh
avior as proposed in `Fix "foreach" behavior `. Removed "Array was modified outside object and internal position is no longer valid" ha ck.: NEWS
ext/spl/spl_array.c ext/spl/tests/array _015.phpt ext/spl/tests/bug31346.phpt ext/spl/tests/bug45614.phpt ext/spl/tests/bug 61527.phpt
ext/spl/tests/bug69335.ph
| From: | Dmitry Stogov | Date: | Thu, 09 Apr 2015 12:40:17 +0000 |
| Subject: | com php-src: Changed ArrayIterator implementation using zend_hash_iterator_... API. Allowed modific ation of itterated ArrayObject using the same beh avior as proposed in `Fix "foreach" behavior `. Removed "Array was modified outside object and internal position is no longer valid" ha ck.: NEWS ext/spl/spl_array.c ext/spl/tests/array _015.phpt ext/spl/tests/bug31346.phpt ext/spl/tests/bug45614.phpt ext/spl/tests/bug 61527.phpt ext/spl/tests/bug69335.ph |
||
| Groups: | php.cvs | ||
| Request: | Send a blank email to php-cvs+get-86145@lists.php.net to get a copy of this message | ||
Commit: 2b6a568df119487f73abb43edd2ae82e83945356
Author: Dmitry Stogov <dmitry@zend.com> Thu, 9 Apr 2015 15:40:17 +0300
Parents: 5fd2f975eee53f379bfe0fc15aa5f9294f767a17
Branches: master
Link: http://git.php.net/?p=php-src.git;a=commitdiff;h=2b6a568df119487f73abb43edd2ae82e83945356
Log:
Changed ArrayIterator implementation using zend_hash_iterator_... API. Allowed modification of
itterated ArrayObject using the same behavior as proposed in
diff --git a/NEWS b/NEWS index 6dfb040..2225432 100644 --- a/NEWS +++ b/NEWS @@ -68,6 +68,7 @@ . Implemented the RFC `Group Use Declarations`. (Marcio) . Implemented the RFC `Continue Output Buffering`. (Mike) . Implemented the RFC `Constructor behaviour of internal classes`. (Dan, Dmitry) + . Implemented the RFC `Fix "foreach" behavior`. (Dmitry) - Curl: . Fixed bug #68937 (Segfault in curl_multi_exec). (Laruence) @@ -163,6 +164,10 @@ . Fixed bug #68361 (Segmentation fault on SoapClient::__getTypes). (Laruence) - SPL: + . Changed ArrayIterator implementation using zend_hash_iterator_... API. + Allowed modification of itterated ArrayObject using the same behavior + as proposed in `Fix "foreach" behavior`. Removed "Array was modified + outside object and internal position is no longer valid" hack. (Dmitry) . Implemented #67886 (SplPriorityQueue/SplHeap doesn't expose extractFlags nor curruption state). (Julien) . Fixed bug #66405 (RecursiveDirectoryIterator::CURRENT_AS_PATHNAME diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 5f6d439..8071a71 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -65,8 +65,7 @@ PHPAPI zend_class_entry *spl_ce_RecursiveArrayIterator; typedef struct _spl_array_object { zval array; zval retval; - HashPosition pos; - zend_ulong pos_h; + uint32_t ht_iter; int ar_flags; int is_self; zend_function *fptr_offset_get; @@ -106,54 +105,22 @@ static inline HashTable *spl_array_get_hash_table(spl_array_object* intern, int } } /* }}} */ -static void spl_array_rewind(spl_array_object *intern); +static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht); -static void spl_array_update_pos(HashTable *ht, spl_array_object* intern) /* {{{ */ +static zend_never_inline void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ { - uint pos = intern->pos; - if (pos != HT_INVALID_IDX) { - intern->pos_h = ht->arData[pos].h; - } -} /* }}} */ - -static void spl_array_set_pos(spl_array_object* intern, HashTable *ht, HashPosition pos) /* {{{ */ -{ - intern->pos = pos; - spl_array_update_pos(ht, intern); -} /* }}} */ + intern->ht_iter = zend_hash_iterator_add(ht, ht->nInternalPointer); + zend_hash_internal_pointer_reset_ex(ht, &EG(ht_iterators)[intern->ht_iter].pos); + spl_array_skip_protected(intern, ht); +} +/* }}} */ -SPL_API int spl_hash_verify_pos_ex(spl_array_object * intern, HashTable * ht) /* {{{ */ +static zend_always_inline uint32_t *spl_array_get_pos_ptr(HashTable *ht, spl_array_object* intern) /* {{{ */ { - uint idx; - -/* IS_CONSISTENT(ht);*/ - -/* HASH_PROTECT_RECURSION(ht);*/ - if (ht->u.flags & HASH_FLAG_PACKED) { - if (intern->pos_h == intern->pos && Z_TYPE(ht->arData[intern->pos_h].val) != IS_UNDEF) { - return SUCCESS; - } - } else { - uint32_t pos = HT_IDX_TO_HASH(intern->pos); - - idx = HT_HASH(ht, intern->pos_h | ht->nTableMask); - while (idx != HT_INVALID_IDX) { - if (idx == pos) { - return SUCCESS; - } - idx = Z_NEXT(HT_HASH_TO_BUCKET(ht, idx)->val); - } + if (UNEXPECTED(intern->ht_iter == (uint32_t)-1)) { + spl_array_create_ht_iter(ht, intern); } -/* HASH_UNPROTECT_RECURSION(ht); */ - spl_array_rewind(intern); - return FAILURE; - -} /* }}} */ - -SPL_API int spl_hash_verify_pos(spl_array_object * intern) /* {{{ */ -{ - HashTable *ht = spl_array_get_hash_table(intern, 0); - return spl_hash_verify_pos_ex(intern, ht); + return &EG(ht_iterators)[intern->ht_iter].pos; } /* }}} */ @@ -271,7 +238,7 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zval * } } - spl_array_rewind(intern); + intern->ht_iter = (uint32_t)-1; return &intern->std; } /* }}} */ @@ -559,10 +526,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } else { -//??? see below -#if 0 - if (zend_symtable_del_ind(ht, Z_STR_P(offset)) == FAILURE) { -#else zval *data = zend_symtable_find(ht, Z_STR_P(offset)); if (data) { @@ -573,14 +536,15 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval } else { zval_ptr_dtor(data); ZVAL_UNDEF(data); + zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern)); + if (Z_TYPE(intern->array) == IS_OBJECT) { + spl_array_skip_protected(intern, ht); + } } -//??? fix for ext/spl/tests/bug45614.phpt (may be fix is wrong) - spl_array_rewind(intern); } else if (zend_symtable_del(ht, Z_STR_P(offset)) == FAILURE) { zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } else { -#endif zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } @@ -613,7 +577,6 @@ num_index: zend_error(E_WARNING, "Illegal offset type"); return; } - spl_hash_verify_pos(intern); /* call rewind on FAILURE */ } /* }}} */ static void spl_array_unset_dimension(zval *object, zval *offset) /* {{{ */ @@ -716,11 +679,6 @@ static inline int spl_array_object_verify_pos_ex(spl_array_object *object, HashT return FAILURE; } - if (object->pos != HT_INVALID_IDX && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, ht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "%sArray was modified outside object and internal position is no longer valid", msg_prefix); - return FAILURE; - } - return SUCCESS; } /* }}} */ @@ -785,11 +743,6 @@ void spl_array_iterator_append(zval *object, zval *append_value) /* {{{ */ } spl_array_write_dimension(object, NULL, append_value); - if (intern->pos == HT_INVALID_IDX) { - if (aht->nNumUsed && !Z_ISUNDEF(aht->arData[aht->nNumUsed-1].val)) { - spl_array_set_pos(intern, aht, aht->nNumUsed - 1); - } - } } /* }}} */ /* {{{ proto void ArrayObject::append(mixed $newval) @@ -936,7 +889,6 @@ static void spl_array_unset_property(zval *object, zval *member, void **cache_sl if ((intern->ar_flags & SPL_ARRAY_ARRAY_AS_PROPS) != 0 && !std_object_handlers.has_property(object, member, 2, cache_slot)) { spl_array_unset_dimension(object, member); - spl_array_rewind(intern); /* because deletion might invalidate position */ return; } std_object_handlers.unset_property(object, member, cache_slot); @@ -971,9 +923,11 @@ static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht) /* zval *data; if (Z_TYPE(intern->array) == IS_OBJECT) { + uint32_t *pos_ptr = spl_array_get_pos_ptr(aht, intern); + do { - if (zend_hash_get_current_key_ex(aht, &string_key, &num_key, &intern->pos) == HASH_KEY_IS_STRING) { - data = zend_hash_get_current_data_ex(aht, &intern->pos); + if (zend_hash_get_current_key_ex(aht, &string_key, &num_key, pos_ptr) == HASH_KEY_IS_STRING) { + data = zend_hash_get_current_data_ex(aht, pos_ptr); if (data && Z_TYPE_P(data) == IS_INDIRECT && Z_TYPE_P(data = Z_INDIRECT_P(data)) == IS_UNDEF) { /* skip */ @@ -983,35 +937,25 @@ static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht) /* } else { return SUCCESS; } - if (zend_hash_has_more_elements_ex(aht, &intern->pos) != SUCCESS) { + if (zend_hash_has_more_elements_ex(aht, pos_ptr) != SUCCESS) { return FAILURE; } - zend_hash_move_forward_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); + zend_hash_move_forward_ex(aht, pos_ptr); } while (1); } return FAILURE; } /* }}} */ -static int spl_array_next_no_verify(spl_array_object *intern, HashTable *aht) /* {{{ */ +static int spl_array_next_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ { - zend_hash_move_forward_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); + uint32_t *pos_ptr = spl_array_get_pos_ptr(aht, intern); + + zend_hash_move_forward_ex(aht, pos_ptr); if (Z_TYPE(intern->array) == IS_OBJECT) { return spl_array_skip_protected(intern, aht); } else { - return zend_hash_has_more_elements_ex(aht, &intern->pos); - } -} /* }}} */ - -static int spl_array_next_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ -{ - if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "Array was modified outside object and internal position is no longer valid"); - return FAILURE; + return zend_hash_has_more_elements_ex(aht, pos_ptr); } - - return spl_array_next_no_verify(intern, aht); } /* }}} */ static int spl_array_next(spl_array_object *intern) /* {{{ */ @@ -1041,7 +985,7 @@ static int spl_array_it_valid(zend_object_iterator *iter) /* {{{ */ return FAILURE; } - return zend_hash_has_more_elements_ex(aht, &object->pos); + return zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, object)); } } /* }}} */ @@ -1054,7 +998,7 @@ static zval *spl_array_it_get_current_data(zend_object_iterator *iter) /* {{{ */ if (object->ar_flags & SPL_ARRAY_OVERLOADED_CURRENT) { return zend_user_it_get_current_data(iter); } else { - zval *data = zend_hash_get_current_data_ex(aht, &object->pos); + zval *data = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, object)); if (Z_TYPE_P(data) == IS_INDIRECT) { data = Z_INDIRECT_P(data); } @@ -1074,7 +1018,7 @@ static void spl_array_it_get_current_key(zend_object_iterator *iter, zval *key) if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): ") == FAILURE) { ZVAL_NULL(key); } else { - zend_hash_get_current_key_zval_ex(aht, key, &object->pos); + zend_hash_get_current_key_zval_ex(aht, key, spl_array_get_pos_ptr(aht, object)); } } } @@ -1094,24 +1038,11 @@ static void spl_array_it_move_forward(zend_object_iterator *iter) /* {{{ */ return; } - if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "ArrayIterator::next(): Array was modified outside object and internal position is no longer valid"); - } else { - spl_array_next_no_verify(object, aht); - } + spl_array_next_ex(object, aht); } } /* }}} */ -static void spl_array_rewind_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ -{ - - zend_hash_internal_pointer_reset_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); - spl_array_skip_protected(intern, aht); - -} /* }}} */ - static void spl_array_rewind(spl_array_object *intern) /* {{{ */ { HashTable *aht = spl_array_get_hash_table(intern, 0); @@ -1121,7 +1052,12 @@ static void spl_array_rewind(spl_array_object *intern) /* {{{ */ return; } - spl_array_rewind_ex(intern, aht); + if (intern->ht_iter == (uint32_t)-1) { + spl_array_get_pos_ptr(aht, intern); + } else { + zend_hash_internal_pointer_reset_ex(aht, spl_array_get_pos_ptr(aht, intern)); + spl_array_skip_protected(intern, aht); + } } /* }}} */ @@ -1174,7 +1110,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } } - spl_array_rewind(intern); + intern->ht_iter = (uint32_t)-1; } /* }}} */ @@ -1391,7 +1327,7 @@ SPL_METHOD(Array, seek) while (position-- > 0 && (result = spl_array_next(intern)) == SUCCESS); - if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS) { + if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, intern)) == SUCCESS) { return; /* ok */ } } @@ -1401,7 +1337,7 @@ SPL_METHOD(Array, seek) int static spl_array_object_count_elements_helper(spl_array_object *intern, zend_long *count) /* {{{ */ { HashTable *aht = spl_array_get_hash_table(intern, 0); - HashPosition pos; + HashPosition pos, *pos_ptr; if (!aht) { php_error_docref(NULL, E_NOTICE, "Array was modified outside object and is no longer an array"); @@ -1412,13 +1348,14 @@ int static spl_array_object_count_elements_helper(spl_array_object *intern, zend if (Z_TYPE(intern->array) == IS_OBJECT) { /* We need to store the 'pos' since we'll modify it in the functions * we're going to call and which do not support 'pos' as parameter. */ - pos = intern->pos; + pos_ptr = spl_array_get_pos_ptr(aht, intern); + pos = *pos_ptr; *count = 0; spl_array_rewind(intern); - while(intern->pos != HT_INVALID_IDX && spl_array_next(intern) == SUCCESS) { + while (*pos_ptr != HT_INVALID_IDX && spl_array_next(intern) == SUCCESS) { (*count)++; } - spl_array_set_pos(intern, aht, pos); + *pos_ptr = pos; return SUCCESS; } else { *count = zend_hash_num_elements(aht); @@ -1565,7 +1502,7 @@ SPL_METHOD(Array, current) return; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { return; } if (Z_TYPE_P(entry) == IS_INDIRECT) { @@ -1598,7 +1535,7 @@ void spl_array_iterator_key(zval *object, zval *return_value) /* {{{ */ return; } - zend_hash_get_current_key_zval_ex(aht, return_value, &intern->pos); + zend_hash_get_current_key_zval_ex(aht, return_value, spl_array_get_pos_ptr(aht, intern)); } /* }}} */ @@ -1618,7 +1555,7 @@ SPL_METHOD(Array, next) return; } - spl_array_next_no_verify(intern, aht); + spl_array_next_ex(intern, aht); } /* }}} */ @@ -1637,7 +1574,7 @@ SPL_METHOD(Array, valid) if (spl_array_object_verify_pos(intern, aht) == FAILURE) { RETURN_FALSE; } else { - RETURN_BOOL(zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS); + RETURN_BOOL(zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, intern)) == SUCCESS); } } /* }}} */ @@ -1658,7 +1595,7 @@ SPL_METHOD(Array, hasChildren) RETURN_FALSE; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { RETURN_FALSE; } @@ -1682,7 +1619,7 @@ SPL_METHOD(Array, getChildren) return; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { return; } diff --git a/ext/spl/tests/array_015.phpt b/ext/spl/tests/array_015.phpt index f0bf9f4..cd3ce5f 100644 --- a/ext/spl/tests/array_015.phpt +++ b/ext/spl/tests/array_015.phpt @@ -56,9 +56,7 @@ object(ArrayObject)#%d (1) { int(5) } } - -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d -int(2) +int(3) object(ArrayObject)#%d (1) { %s"storage"%s"ArrayObject":private]=> array(4) { @@ -84,14 +82,11 @@ object(ArrayObject)#%d (1) { } } 1=>2 - -Notice: main(): ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d -3=>4 - -Notice: main(): ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d object(ArrayObject)#%d (1) { %s"storage"%s"ArrayObject":private]=> - array(0) { + array(1) { + [3]=> + int(4) } } ===DONE=== diff --git a/ext/spl/tests/bug31346.phpt b/ext/spl/tests/bug31346.phpt index 9b5618e..e3bf51f 100644 --- a/ext/spl/tests/bug31346.phpt +++ b/ext/spl/tests/bug31346.phpt @@ -15,5 +15,4 @@ $i->next(); ?> ===DONE=== --EXPECTF-- -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug31346.php on line %d ===DONE=== diff --git a/ext/spl/tests/bug45614.phpt b/ext/spl/tests/bug45614.phpt index 8f99934..9ab3186 100644 --- a/ext/spl/tests/bug45614.phpt +++ b/ext/spl/tests/bug45614.phpt @@ -8,6 +8,7 @@ class C { public $pub1 = 'public1'; public $pub2 = 'public2'; public $pub3 = 'public3'; + public $pub4 = 'public4'; } function showFirstTwoItems($it) { @@ -48,8 +49,8 @@ pub1 => public1 pub2 => public2 --> Invalidate current position and show the first two items: -pub1 => public1 pub3 => public3 +pub4 => public4 --> Rewind, seek and show the first two items: pub1 => public1 diff --git a/ext/spl/tests/bug61527.phpt b/ext/spl/tests/bug61527.phpt index ab13c69..03dbbe3 100644 --- a/ext/spl/tests/bug61527.phpt +++ b/ext/spl/tests/bug61527.phpt @@ -67,26 +67,16 @@ var_dump($ai4->getChildren()); --EXPECTF-- NULL NULL - -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d - -Notice: ArrayIterator::key(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d +int(4) +int(5) NULL - -Notice: ArrayIterator::current(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d NULL -int(5) -int(5) NULL NULL NULL NULL NULL bool(false) - -Notice: RecursiveArrayIterator::hasChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d bool(false) - -Notice: RecursiveArrayIterator::getChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d NULL ==DONE== diff --git a/ext/spl/tests/bug69335.phpt b/ext/spl/tests/bug69335.phpt index 0cb17d0..41f03ef 100644 --- a/ext/spl/tests/bug69335.phpt +++ b/ext/spl/tests/bug69335.phpt @@ -1,7 +1,5 @@ --TEST-- Bug #69335 (SplArray BC break) ---XFAIL-- -see https://bugs.php.net/bug.php?id=69335 --FILE-- <?php $a = array(1=>1, 3=>3, 5=>5, 7=>7);
Fix "foreach"
behavior. Removed "Array was modified outside object and internal position is no longer
valid" hack.
Changed paths:
M NEWS
M ext/spl/spl_array.c
M ext/spl/tests/array_015.phpt
M ext/spl/tests/bug31346.phpt
M ext/spl/tests/bug45614.phpt
M ext/spl/tests/bug61527.phpt
M ext/spl/tests/bug69335.phpt
diff --git a/NEWS b/NEWS index 6dfb040..2225432 100644 --- a/NEWS +++ b/NEWS @@ -68,6 +68,7 @@ . Implemented the RFC `Group Use Declarations`. (Marcio) . Implemented the RFC `Continue Output Buffering`. (Mike) . Implemented the RFC `Constructor behaviour of internal classes`. (Dan, Dmitry) + . Implemented the RFC `Fix "foreach" behavior`. (Dmitry) - Curl: . Fixed bug #68937 (Segfault in curl_multi_exec). (Laruence) @@ -163,6 +164,10 @@ . Fixed bug #68361 (Segmentation fault on SoapClient::__getTypes). (Laruence) - SPL: + . Changed ArrayIterator implementation using zend_hash_iterator_... API. + Allowed modification of itterated ArrayObject using the same behavior + as proposed in `Fix "foreach" behavior`. Removed "Array was modified + outside object and internal position is no longer valid" hack. (Dmitry) . Implemented #67886 (SplPriorityQueue/SplHeap doesn't expose extractFlags nor curruption state). (Julien) . Fixed bug #66405 (RecursiveDirectoryIterator::CURRENT_AS_PATHNAME diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 5f6d439..8071a71 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -65,8 +65,7 @@ PHPAPI zend_class_entry *spl_ce_RecursiveArrayIterator; typedef struct _spl_array_object { zval array; zval retval; - HashPosition pos; - zend_ulong pos_h; + uint32_t ht_iter; int ar_flags; int is_self; zend_function *fptr_offset_get; @@ -106,54 +105,22 @@ static inline HashTable *spl_array_get_hash_table(spl_array_object* intern, int } } /* }}} */ -static void spl_array_rewind(spl_array_object *intern); +static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht); -static void spl_array_update_pos(HashTable *ht, spl_array_object* intern) /* {{{ */ +static zend_never_inline void spl_array_create_ht_iter(HashTable *ht, spl_array_object* intern) /* {{{ */ { - uint pos = intern->pos; - if (pos != HT_INVALID_IDX) { - intern->pos_h = ht->arData[pos].h; - } -} /* }}} */ - -static void spl_array_set_pos(spl_array_object* intern, HashTable *ht, HashPosition pos) /* {{{ */ -{ - intern->pos = pos; - spl_array_update_pos(ht, intern); -} /* }}} */ + intern->ht_iter = zend_hash_iterator_add(ht, ht->nInternalPointer); + zend_hash_internal_pointer_reset_ex(ht, &EG(ht_iterators)[intern->ht_iter].pos); + spl_array_skip_protected(intern, ht); +} +/* }}} */ -SPL_API int spl_hash_verify_pos_ex(spl_array_object * intern, HashTable * ht) /* {{{ */ +static zend_always_inline uint32_t *spl_array_get_pos_ptr(HashTable *ht, spl_array_object* intern) /* {{{ */ { - uint idx; - -/* IS_CONSISTENT(ht);*/ - -/* HASH_PROTECT_RECURSION(ht);*/ - if (ht->u.flags & HASH_FLAG_PACKED) { - if (intern->pos_h == intern->pos && Z_TYPE(ht->arData[intern->pos_h].val) != IS_UNDEF) { - return SUCCESS; - } - } else { - uint32_t pos = HT_IDX_TO_HASH(intern->pos); - - idx = HT_HASH(ht, intern->pos_h | ht->nTableMask); - while (idx != HT_INVALID_IDX) { - if (idx == pos) { - return SUCCESS; - } - idx = Z_NEXT(HT_HASH_TO_BUCKET(ht, idx)->val); - } + if (UNEXPECTED(intern->ht_iter == (uint32_t)-1)) { + spl_array_create_ht_iter(ht, intern); } -/* HASH_UNPROTECT_RECURSION(ht); */ - spl_array_rewind(intern); - return FAILURE; - -} /* }}} */ - -SPL_API int spl_hash_verify_pos(spl_array_object * intern) /* {{{ */ -{ - HashTable *ht = spl_array_get_hash_table(intern, 0); - return spl_hash_verify_pos_ex(intern, ht); + return &EG(ht_iterators)[intern->ht_iter].pos; } /* }}} */ @@ -271,7 +238,7 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zval * } } - spl_array_rewind(intern); + intern->ht_iter = (uint32_t)-1; return &intern->std; } /* }}} */ @@ -559,10 +526,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } else { -//??? see below -#if 0 - if (zend_symtable_del_ind(ht, Z_STR_P(offset)) == FAILURE) { -#else zval *data = zend_symtable_find(ht, Z_STR_P(offset)); if (data) { @@ -573,14 +536,15 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval } else { zval_ptr_dtor(data); ZVAL_UNDEF(data); + zend_hash_move_forward_ex(ht, spl_array_get_pos_ptr(ht, intern)); + if (Z_TYPE(intern->array) == IS_OBJECT) { + spl_array_skip_protected(intern, ht); + } } -//??? fix for ext/spl/tests/bug45614.phpt (may be fix is wrong) - spl_array_rewind(intern); } else if (zend_symtable_del(ht, Z_STR_P(offset)) == FAILURE) { zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } else { -#endif zend_error(E_NOTICE,"Undefined index: %s", Z_STRVAL_P(offset)); } } @@ -613,7 +577,6 @@ num_index: zend_error(E_WARNING, "Illegal offset type"); return; } - spl_hash_verify_pos(intern); /* call rewind on FAILURE */ } /* }}} */ static void spl_array_unset_dimension(zval *object, zval *offset) /* {{{ */ @@ -716,11 +679,6 @@ static inline int spl_array_object_verify_pos_ex(spl_array_object *object, HashT return FAILURE; } - if (object->pos != HT_INVALID_IDX && (object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, ht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "%sArray was modified outside object and internal position is no longer valid", msg_prefix); - return FAILURE; - } - return SUCCESS; } /* }}} */ @@ -785,11 +743,6 @@ void spl_array_iterator_append(zval *object, zval *append_value) /* {{{ */ } spl_array_write_dimension(object, NULL, append_value); - if (intern->pos == HT_INVALID_IDX) { - if (aht->nNumUsed && !Z_ISUNDEF(aht->arData[aht->nNumUsed-1].val)) { - spl_array_set_pos(intern, aht, aht->nNumUsed - 1); - } - } } /* }}} */ /* {{{ proto void ArrayObject::append(mixed $newval) @@ -936,7 +889,6 @@ static void spl_array_unset_property(zval *object, zval *member, void **cache_sl if ((intern->ar_flags & SPL_ARRAY_ARRAY_AS_PROPS) != 0 && !std_object_handlers.has_property(object, member, 2, cache_slot)) { spl_array_unset_dimension(object, member); - spl_array_rewind(intern); /* because deletion might invalidate position */ return; } std_object_handlers.unset_property(object, member, cache_slot); @@ -971,9 +923,11 @@ static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht) /* zval *data; if (Z_TYPE(intern->array) == IS_OBJECT) { + uint32_t *pos_ptr = spl_array_get_pos_ptr(aht, intern); + do { - if (zend_hash_get_current_key_ex(aht, &string_key, &num_key, &intern->pos) == HASH_KEY_IS_STRING) { - data = zend_hash_get_current_data_ex(aht, &intern->pos); + if (zend_hash_get_current_key_ex(aht, &string_key, &num_key, pos_ptr) == HASH_KEY_IS_STRING) { + data = zend_hash_get_current_data_ex(aht, pos_ptr); if (data && Z_TYPE_P(data) == IS_INDIRECT && Z_TYPE_P(data = Z_INDIRECT_P(data)) == IS_UNDEF) { /* skip */ @@ -983,35 +937,25 @@ static int spl_array_skip_protected(spl_array_object *intern, HashTable *aht) /* } else { return SUCCESS; } - if (zend_hash_has_more_elements_ex(aht, &intern->pos) != SUCCESS) { + if (zend_hash_has_more_elements_ex(aht, pos_ptr) != SUCCESS) { return FAILURE; } - zend_hash_move_forward_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); + zend_hash_move_forward_ex(aht, pos_ptr); } while (1); } return FAILURE; } /* }}} */ -static int spl_array_next_no_verify(spl_array_object *intern, HashTable *aht) /* {{{ */ +static int spl_array_next_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ { - zend_hash_move_forward_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); + uint32_t *pos_ptr = spl_array_get_pos_ptr(aht, intern); + + zend_hash_move_forward_ex(aht, pos_ptr); if (Z_TYPE(intern->array) == IS_OBJECT) { return spl_array_skip_protected(intern, aht); } else { - return zend_hash_has_more_elements_ex(aht, &intern->pos); - } -} /* }}} */ - -static int spl_array_next_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ -{ - if ((intern->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(intern, aht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "Array was modified outside object and internal position is no longer valid"); - return FAILURE; + return zend_hash_has_more_elements_ex(aht, pos_ptr); } - - return spl_array_next_no_verify(intern, aht); } /* }}} */ static int spl_array_next(spl_array_object *intern) /* {{{ */ @@ -1041,7 +985,7 @@ static int spl_array_it_valid(zend_object_iterator *iter) /* {{{ */ return FAILURE; } - return zend_hash_has_more_elements_ex(aht, &object->pos); + return zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, object)); } } /* }}} */ @@ -1054,7 +998,7 @@ static zval *spl_array_it_get_current_data(zend_object_iterator *iter) /* {{{ */ if (object->ar_flags & SPL_ARRAY_OVERLOADED_CURRENT) { return zend_user_it_get_current_data(iter); } else { - zval *data = zend_hash_get_current_data_ex(aht, &object->pos); + zval *data = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, object)); if (Z_TYPE_P(data) == IS_INDIRECT) { data = Z_INDIRECT_P(data); } @@ -1074,7 +1018,7 @@ static void spl_array_it_get_current_key(zend_object_iterator *iter, zval *key) if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): ") == FAILURE) { ZVAL_NULL(key); } else { - zend_hash_get_current_key_zval_ex(aht, key, &object->pos); + zend_hash_get_current_key_zval_ex(aht, key, spl_array_get_pos_ptr(aht, object)); } } } @@ -1094,24 +1038,11 @@ static void spl_array_it_move_forward(zend_object_iterator *iter) /* {{{ */ return; } - if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht) == FAILURE) { - php_error_docref(NULL, E_NOTICE, "ArrayIterator::next(): Array was modified outside object and internal position is no longer valid"); - } else { - spl_array_next_no_verify(object, aht); - } + spl_array_next_ex(object, aht); } } /* }}} */ -static void spl_array_rewind_ex(spl_array_object *intern, HashTable *aht) /* {{{ */ -{ - - zend_hash_internal_pointer_reset_ex(aht, &intern->pos); - spl_array_update_pos(aht, intern); - spl_array_skip_protected(intern, aht); - -} /* }}} */ - static void spl_array_rewind(spl_array_object *intern) /* {{{ */ { HashTable *aht = spl_array_get_hash_table(intern, 0); @@ -1121,7 +1052,12 @@ static void spl_array_rewind(spl_array_object *intern) /* {{{ */ return; } - spl_array_rewind_ex(intern, aht); + if (intern->ht_iter == (uint32_t)-1) { + spl_array_get_pos_ptr(aht, intern); + } else { + zend_hash_internal_pointer_reset_ex(aht, spl_array_get_pos_ptr(aht, intern)); + spl_array_skip_protected(intern, aht); + } } /* }}} */ @@ -1174,7 +1110,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar } } - spl_array_rewind(intern); + intern->ht_iter = (uint32_t)-1; } /* }}} */ @@ -1391,7 +1327,7 @@ SPL_METHOD(Array, seek) while (position-- > 0 && (result = spl_array_next(intern)) == SUCCESS); - if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS) { + if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, intern)) == SUCCESS) { return; /* ok */ } } @@ -1401,7 +1337,7 @@ SPL_METHOD(Array, seek) int static spl_array_object_count_elements_helper(spl_array_object *intern, zend_long *count) /* {{{ */ { HashTable *aht = spl_array_get_hash_table(intern, 0); - HashPosition pos; + HashPosition pos, *pos_ptr; if (!aht) { php_error_docref(NULL, E_NOTICE, "Array was modified outside object and is no longer an array"); @@ -1412,13 +1348,14 @@ int static spl_array_object_count_elements_helper(spl_array_object *intern, zend if (Z_TYPE(intern->array) == IS_OBJECT) { /* We need to store the 'pos' since we'll modify it in the functions * we're going to call and which do not support 'pos' as parameter. */ - pos = intern->pos; + pos_ptr = spl_array_get_pos_ptr(aht, intern); + pos = *pos_ptr; *count = 0; spl_array_rewind(intern); - while(intern->pos != HT_INVALID_IDX && spl_array_next(intern) == SUCCESS) { + while (*pos_ptr != HT_INVALID_IDX && spl_array_next(intern) == SUCCESS) { (*count)++; } - spl_array_set_pos(intern, aht, pos); + *pos_ptr = pos; return SUCCESS; } else { *count = zend_hash_num_elements(aht); @@ -1565,7 +1502,7 @@ SPL_METHOD(Array, current) return; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { return; } if (Z_TYPE_P(entry) == IS_INDIRECT) { @@ -1598,7 +1535,7 @@ void spl_array_iterator_key(zval *object, zval *return_value) /* {{{ */ return; } - zend_hash_get_current_key_zval_ex(aht, return_value, &intern->pos); + zend_hash_get_current_key_zval_ex(aht, return_value, spl_array_get_pos_ptr(aht, intern)); } /* }}} */ @@ -1618,7 +1555,7 @@ SPL_METHOD(Array, next) return; } - spl_array_next_no_verify(intern, aht); + spl_array_next_ex(intern, aht); } /* }}} */ @@ -1637,7 +1574,7 @@ SPL_METHOD(Array, valid) if (spl_array_object_verify_pos(intern, aht) == FAILURE) { RETURN_FALSE; } else { - RETURN_BOOL(zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS); + RETURN_BOOL(zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, intern)) == SUCCESS); } } /* }}} */ @@ -1658,7 +1595,7 @@ SPL_METHOD(Array, hasChildren) RETURN_FALSE; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { RETURN_FALSE; } @@ -1682,7 +1619,7 @@ SPL_METHOD(Array, getChildren) return; } - if ((entry = zend_hash_get_current_data_ex(aht, &intern->pos)) == NULL) { + if ((entry = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, intern))) == NULL) { return; } diff --git a/ext/spl/tests/array_015.phpt b/ext/spl/tests/array_015.phpt index f0bf9f4..cd3ce5f 100644 --- a/ext/spl/tests/array_015.phpt +++ b/ext/spl/tests/array_015.phpt @@ -56,9 +56,7 @@ object(ArrayObject)#%d (1) { int(5) } } - -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d -int(2) +int(3) object(ArrayObject)#%d (1) { %s"storage"%s"ArrayObject":private]=> array(4) { @@ -84,14 +82,11 @@ object(ArrayObject)#%d (1) { } } 1=>2 - -Notice: main(): ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d -3=>4 - -Notice: main(): ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sarray_015.php on line %d object(ArrayObject)#%d (1) { %s"storage"%s"ArrayObject":private]=> - array(0) { + array(1) { + [3]=> + int(4) } } ===DONE=== diff --git a/ext/spl/tests/bug31346.phpt b/ext/spl/tests/bug31346.phpt index 9b5618e..e3bf51f 100644 --- a/ext/spl/tests/bug31346.phpt +++ b/ext/spl/tests/bug31346.phpt @@ -15,5 +15,4 @@ $i->next(); ?> ===DONE=== --EXPECTF-- -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug31346.php on line %d ===DONE=== diff --git a/ext/spl/tests/bug45614.phpt b/ext/spl/tests/bug45614.phpt index 8f99934..9ab3186 100644 --- a/ext/spl/tests/bug45614.phpt +++ b/ext/spl/tests/bug45614.phpt @@ -8,6 +8,7 @@ class C { public $pub1 = 'public1'; public $pub2 = 'public2'; public $pub3 = 'public3'; + public $pub4 = 'public4'; } function showFirstTwoItems($it) { @@ -48,8 +49,8 @@ pub1 => public1 pub2 => public2 --> Invalidate current position and show the first two items: -pub1 => public1 pub3 => public3 +pub4 => public4 --> Rewind, seek and show the first two items: pub1 => public1 diff --git a/ext/spl/tests/bug61527.phpt b/ext/spl/tests/bug61527.phpt index ab13c69..03dbbe3 100644 --- a/ext/spl/tests/bug61527.phpt +++ b/ext/spl/tests/bug61527.phpt @@ -67,26 +67,16 @@ var_dump($ai4->getChildren()); --EXPECTF-- NULL NULL - -Notice: ArrayIterator::next(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d - -Notice: ArrayIterator::key(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d +int(4) +int(5) NULL - -Notice: ArrayIterator::current(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d NULL -int(5) -int(5) NULL NULL NULL NULL NULL bool(false) - -Notice: RecursiveArrayIterator::hasChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d bool(false) - -Notice: RecursiveArrayIterator::getChildren(): Array was modified outside object and internal position is no longer valid in %sbug61527.php on line %d NULL ==DONE== diff --git a/ext/spl/tests/bug69335.phpt b/ext/spl/tests/bug69335.phpt index 0cb17d0..41f03ef 100644 --- a/ext/spl/tests/bug69335.phpt +++ b/ext/spl/tests/bug69335.phpt @@ -1,7 +1,5 @@ --TEST-- Bug #69335 (SplArray BC break) ---XFAIL-- -see https://bugs.php.net/bug.php?id=69335 --FILE-- <?php $a = array(1=>1, 3=>3, 5=>5, 7=>7);