Avoid crash for reset/end/next/prev() on ffi classes #9711
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
This will work of course, but I think it's safer to make a change in ext/ffi and return a mutable array. see #9716 |
Member
Member
|
I'll take a deeper look on Monday (January 30). |
Member
As it seems to be fixing crashes I would say yes. |
TysonAndre
force-pushed
the
fix-reset-ffi-crash
branch
from
57af8bc to
9dc675f
Compare
dstogov
requested changes
Jan 31, 2023
Member
dstogov
left a comment
dstogov
left a comment
There was a problem hiding this comment.
All tests are broken because of missing skipif.inc
TysonAndre
force-pushed
the
fix-reset-ffi-crash
branch
from
9dc675f to
787a712
Compare
dstogov
approved these changes
Feb 1, 2023
(And any PECLs returning `zend_empty_array` in the handler->get_properties overrides) Closes phpGH-9697 This is similar to the fix used in d9651a9 for array_walk. This should make it safer for php-src (and PECLs, long-term) to return the empty immutable array in `handler->get_properties` to avoid wasting memory. See php#9697 (comment) The only possible internal iterator position for the empty array is at the end of the empty array (nInternalPointer=0). The `zend_hash*del*` helpers will always set nInternalPointer to 0 when an array becomes empty, regardless of previous insertions/deletions/updates to the array.
TysonAndre
force-pushed
the
fix-reset-ffi-crash
branch
from
787a712 to
b3937f7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(And any PECLs returning
&zend_empty_arrayin the handler->get_properties overrides to save memory)Closes GH-9697
This is similar to the fix used in d9651a9 for array_walk.
This should make it safer for php-src (and PECLs, long-term) to return the empty immutable array in
handler->get_propertiesto avoid wasting memory. See #9697 (comment)The only possible internal iterator position for the empty array is at the end of the empty array (nInternalPointer=0).
The
zend_hash*del*helpers will always set nInternalPointer to 0 when an array becomes empty,regardless of previous insertions/deletions/updates to the array.