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

RESOLV: add partial support for new c-ares parser #7581

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexey-tikhonov
Copy link
Member

@alexey-tikhonov alexey-tikhonov commented Sep 9, 2024

and suppress other deprecation warnings.

Partially resolves #7571

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Sep 9, 2024
`resolv_gethostbyname_dns_parse()` didn't use `status` and it was
always set to `ARES_SUCCESS` anyway.
instead of deprecated `ares_parse_a/aaaa/_reply()`

This patch doesn't replace `ares_parse_srv_reply()` as benefints
are quite questionable.

Partially resolves SSSD#7571
In theory new API might be somewhat better.

But:

1) it's fairly new: `ares_search_dnsrec()` and `ares_query_dnsrec()`
were introduced in c-ares-1.28 while even CentOS Stream 10 has
c-ares-1.25, so SSSD would need to support (fallback) old API anyway.

2) SSSD doesn't make heavy use of DNS, so potential performance
improvements are really negligible.

On the other hand, old API/ABI will be available for a long time:
c-ares/c-ares#732 (comment)

For those reasons it's not worth the effort to port code to new API
right now.
@alexey-tikhonov alexey-tikhonov changed the title Cares RESOLV: add partial support for new c-ares parser Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update resolv/async_resolv.c to avoid using deprecated stuff
1 participant