Skip to content

Refactor code handling file.current_zval #8934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jul 6, 2022

Code refactoring while I'm attempting to get a better grasp of this code.

@twose you wouldn't have a test case which would hit the:

if (SPL_HAS_FLAG(intern->flags, SPL_FILE_OBJECT_READ_CSV)
				&& zend_hash_num_elements(Z_ARRVAL(intern->u.file.current_zval)) == 1) {
			ZEND_ASSERT(false && "Can this happen?");

code path do you? As I don't understand what this branch is attempting to do.

@cmb69
Copy link
Member

cmb69 commented Jul 6, 2022

It seems to me there is the invariant !Z_ISUNDEF(intern->u.file.current_zval) → intern->u.file.current_line. If true, that code is dead (and some other code would be unnecessary, too).

@twose
Copy link
Member

twose commented Jul 11, 2022

Sorry, I have never used fgetcsv() via SPL APIs...

The Zval is always an array
@Girgias Girgias force-pushed the spl-dir-redundant-silent-arg branch from 295d4ae to 3ad303d Compare July 28, 2022 14:46
@Girgias Girgias changed the title Cleaning up more SPL Directory code Refactor code handling file.current_zval Jul 28, 2022
@Girgias Girgias merged commit 7ab22aa into php:master Jul 28, 2022
@Girgias Girgias deleted the spl-dir-redundant-silent-arg branch July 28, 2022 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants