Skip to content

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Jun 23, 2020

atol() returns a long which is not the same as zend_long on
LLP64; we use ZEND_ATOL() instead.

There is no need for a new test case, since filesize_large.phpt already
tests for that behavior; unfortunately, the FTP test suite relies on
pcntl_fork() and therefore cannot be run on Windows.

`atol()` returns a `long` which is not the same as `zend_long` on
LLP64; we use `ZEND_ATOL()` instead.

There is no need for a new test case, since filesize_large.phpt already
tests for that behavior; unfortunately, the FTP test suite relies on
`pcntl_fork()` and therefore cannot be run on Windows.
return -1;
}
return atol(ftp->inbuf);
ZEND_ATOL(res, ftp->inbuf);
Copy link
Member

@nikic nikic Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotta wonder, why the heck does ZEND_ATOL include a variable assignment instead of just returning the value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an unfortunate design decision, but probably too much BC break if we change that.

@carusogabriel carusogabriel added this to the PHP 8.0 milestone Jun 23, 2020
@cmb69 cmb69 added Bug and removed Bug labels Jun 23, 2020
@cmb69 cmb69 removed this from the PHP 8.0 milestone Jun 23, 2020
@cmb69
Copy link
Member Author

cmb69 commented Jun 23, 2020

Thanks! Applied as e94126a

@cmb69 cmb69 closed this Jun 23, 2020
@cmb69 cmb69 deleted the cmb/55857 branch June 23, 2020 14:04
@carusogabriel
Copy link
Contributor

@cmb69 Sorry, I didn't see that the base branch was PHP-7.3 :)

@cmb69
Copy link
Member Author

cmb69 commented Jun 23, 2020

@carusogabriel, no problem :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants