Rework internals to pass around ares_dns_record_t instead of binary data
#730
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
@oliverwelsh don't know if you want to review any progress, but I'm working on implementing what was discussed as the next step in #719 |
Member
Author
|
Other than missing manpages, I think the code for this is done... |
oliverwelsh
reviewed
Mar 24, 2024
Contributor
oliverwelsh
left a comment
oliverwelsh
left a comment
There was a problem hiding this comment.
Hi Brad, thank you for following up on this. It's great to see the DNS record object and parser used throughout the codebase as standard!
I have taken a look through of the code and left a couple of review comments. Overall I think this looks great. I especially like ares__search_name_list() - it feels a lot cleaner to calculate the search domains up front, which simplifies the logic when doing multiple searches.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
c-ares has historically passed around raw dns packets in binary form. Now that we have a new parser, and messages are already parsed internally, lets pass around that parsed message rather than requiring multiple parse attempts on the same message. Also add a new
ares_send_dnsrec()andares_query_dnsrec()similar toares_search_dnsrec()added with PR #719 that can return the pointer to theares_dns_record_tto the caller enqueuing queries and reworkares_search_dnsrec()to useares_send_dnsrec()internally.