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

Sockets: fix getipaddr() #56528

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Sockets: fix getipaddr() #56528

merged 2 commits into from
Nov 15, 2024

Conversation

barucden
Copy link
Contributor

@barucden barucden commented Nov 11, 2024

This PR updates the definition of getipaddr() so that the documentation is respected.

Before this change, getipaddr() would throw an error unless there is an IPv4 address assigned to a network interface. After this change, getipaddr() preferably returns an IPv4 address, but if none exists, it returns an IPv6 address.

Fixes #56520

@martinholters
Copy link
Member

IIUC, the current behavior was deliberately chosen in #30609 to avoid breakage in code that has problems with IPv6. Not sure that's a valid concern still.

That said, isn't

-getipaddr() = getipaddr(IPv4)
+getipaddr() = getipaddr(IPAddr)

all that's needed as a change here?

@barucden
Copy link
Contributor Author

Thank you for your input. I would have never guessed that IPAddr is a valid input for that function! But you are right, your proposal would already fix the linked issue.

So is it correct that the whole findfirst logic is there so that getaddrs(IPAddr) returns an IPv4?

@martinholters
Copy link
Member

So is it correct that the whole findfirst logic is there so that getaddrs(IPAddr) returns an IPv4?

I think so, yes.

This PR updates the definition of `getipaddr()` so that the
documentation is respected.

Before this change, `getipaddr()` would throw an error unless there is
an `IPv4` address assigned to a network interface. After this change,
`getipaddr()` preferably returns an `IPv4` address, but if none exists,
it returns an `IPv6` address.

Fixes JuliaLang#56520
@barucden barucden changed the title Sockets: fix getipaddr(), simplify getipaddr(T) Sockets: fix getipaddr() Nov 12, 2024
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Nov 14, 2024
@vtjnash
Copy link
Member

vtjnash commented Nov 14, 2024

Yes, looks like the findfirst logic was added with the intent of making this work this way, but forgot to make the change to getipaddr (#32179 (comment))

@fatteneder fatteneder merged commit e5f3010 into JuliaLang:master Nov 15, 2024
8 checks passed
@fatteneder fatteneder removed the merge me PR is reviewed. Merge when all tests are passing label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Julia on servers without IPv4
4 participants