Skip to content

Fix FD getting code on big endian#17259

Merged
NattyNarwhal merged 2 commits into
php:PHP-8.3from
NattyNarwhal:fix-big-endian-fd-8.3
Dec 30, 2024
Merged

Fix FD getting code on big endian#17259
NattyNarwhal merged 2 commits into
php:PHP-8.3from
NattyNarwhal:fix-big-endian-fd-8.3

Conversation

@NattyNarwhal

Copy link
Copy Markdown
Member

stream casting as FD returns a php_socket_t, which is an int, but zend_long is 64-bit (on those platforms). This works on LE by accidental (unless it forgets to clear the high word), but is fatal on big endian.

fb2443a in master seems to have made this obvious, but the issue exists in earlier versions too. Fixes a regression introduced by that commit on big endian. (see GH-17258 for CI there)

@NattyNarwhal NattyNarwhal marked this pull request as ready for review December 24, 2024 21:01
Comment thread ext/posix/posix.c Outdated
stream casting as FD returns a php_socket_t, which is an int, but
zend_long is 64-bit (on those platforms). This works on LE by
accidental (unless it forgets to clear the high word), but is fatal
on big endian.
Comment thread ext/posix/posix.c Outdated

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@devnexen devnexen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hopefully your ppc CI PR will work out

@NattyNarwhal NattyNarwhal merged commit 7c9f645 into php:PHP-8.3 Dec 30, 2024
NattyNarwhal added a commit that referenced this pull request Dec 30, 2024
* PHP-8.3:
  Fix FD getting code on big endian (#17259)
NattyNarwhal added a commit that referenced this pull request Dec 30, 2024
* PHP-8.4:
  Fix FD getting code on big endian (#17259)
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