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

Correct typespec for inet_res:getbyname/2,3 (PR #5412, OTP-17986) #5803

Conversation

RaimoNiskanen
Copy link
Contributor

Closes PR #5412.

Instead of changing the #hostent{} record type; correct the return type for inet_res:getbyname/2,3 by creating a parallel tuple type that defines the peculiar use of the #hostent{} record.

@RaimoNiskanen RaimoNiskanen added team:PS Assigned to OTP team PS types The issue is related to types labels Mar 21, 2022
@RaimoNiskanen RaimoNiskanen added this to the OTP-25.0 milestone Mar 21, 2022
@RaimoNiskanen RaimoNiskanen self-assigned this Mar 21, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 2022

CT Test Results

       2 files       65 suites   53m 44s ⏱️
1 316 tests 1 175 ✔️ 140 💤 1
1 471 runs  1 299 ✔️ 171 💤 1

For more details on these failures, see this check.

Results for commit 4e664c1.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@bmk
Copy link
Contributor

bmk commented Mar 23, 2022

getbyname/2,3
I think it looks like the third section (starting with "This function uses resolver...") is the most
general description of the function and therefor should be moved up to start the description.
The second section has "h_addr_list = [dns_data()]" as part of the explanation, but since that is
what the type (hostent()) is, it is not unique.
Maybe make the alternative resolves into a bullet list?

@RaimoNiskanen
Copy link
Contributor Author

I think it looks like the third section (starting with "This function uses resolver...") is the most
general description of the function and therefor should be moved up to start the description.

I think the first paragraph starting with "Resolves a DNS record of the specified Type for the specified host..." is the most general description. That it uses the search list is a detail.

I will rephrase to reduce the cluttering when describing the #hostent{} fields.

@RaimoNiskanen RaimoNiskanen merged commit d35b586 into erlang:master Mar 31, 2022
@RaimoNiskanen RaimoNiskanen deleted the NineFX/kernel/hostent_spec_fix/PR-5412/OTP-17986 branch March 31, 2022 14:33
NelsonVides added a commit to esl/MongooseIM that referenced this pull request May 22, 2024
S2S DNS discovery fix

This PR fixes an issue with S2S connections using DNS discovery where connecting to another server would fail. The root cause was that inet_res:getbyname/3 returns a DNS type instead of the expected inet type needed for establishing connection. The updated logic now includes a proper lookup for the correct inet type to ensure successful connections.

Previously, the Erlang documentation did not clearly specify this behavior, which could be misleading. More details can be found here: erlang/otp#5803.

Testing was performed by mocking the appropriate inet functions. An alternative solution could involve setting up a local DNS server or using external tools like dnsmasq to set up a lightweight DNS resolver. However, this would require installing additional dependencies in the CI environment, so mocking was chosen as a simpler solution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS types The issue is related to types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants