Skip to content
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

ext/standard: inet_ntop internal change. #17830

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
Member

The actual buffer size is too small for ipv6, should be INET6_ADDRSTRLEN (46) also the size of the buffer should be set accordingly to the family.

The actual buffer size is too small for ipv6, should be INET6_ADDRSTRLEN
(46) also the size of the buffer should be set accordingly to the
family.
@devnexen
Copy link
Member Author

note that I set on master purposely as it is not really a bug fix per say. let me know if you disagree.

@@ -544,7 +544,8 @@ PHP_FUNCTION(inet_ntop)
char *address;
size_t address_len;
int af = AF_INET;
char buffer[40];
socklen_t buffersize = INET_ADDRSTRLEN;
char buffer[INET6_ADDRSTRLEN];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is anything INET6_* defined when HAVE_IPV6 is not?

Also, is it safe to use shorter buffersize when compiled /wo IPv6 support and then run on system /w IPv6 support?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is anything INET6_* defined when HAVE_IPV6 is not?

Yes it is independant from ipv6 support and, alone, does not indicate the system supports it.

Also, is it safe to use shorter buffersize when compiled /wo IPv6 support and then run on system /w IPv6 support?

Yes in that case the best course is to reverse the logic.

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.

2 participants