Skip to content

mark deprecated functions as such #732

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

Merged
merged 6 commits into from
Mar 26, 2024
Merged

Conversation

bradh352
Copy link
Member

Multiple functions have been deprecated over the years, annotate them with attribute deprecated.

When possible show a message about their replacements.

This is a continuation/completion of PR #706

Fix By: Cristian Rodríguez (@crrodriguez)

@bradh352 bradh352 merged commit 5fd3fc3 into c-ares:main Mar 26, 2024
@bradh352 bradh352 deleted the deprecated branch March 27, 2024 15:27
@eriklax
Copy link
Contributor

eriklax commented Mar 30, 2024

What is the short to mid-term plan for the deprecation of various methods not considered the new API? Is it safe to ignore these warnings (CARES_NO_DEPRECATED) for the time being and migrate slowly.

While they are more powerful, it's a lot more verbose API and more error handling needs to be in the application.

Having used the old API, and looking at the source code of eg. ares_parse_mx_reply there seems to be a lot of error handling that need to be moved to the application to have the same logic. While I don't favor the old API per se, there is less moving parts.

Also, my application use ares_create_query, it should be replaced with ares_dns_record_create, however there is some logic that isn't in the ares_dns_record_create (since I need to construct everyhing myself) functions such as .onion checking (which is in ares_dns_record_create_query). Would it make sense to make ares_dns_record_create_query public?

@bradh352
Copy link
Member Author

bradh352 commented Mar 30, 2024 via email

JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to JelteF/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
JelteF added a commit to pgbouncer/pgbouncer that referenced this pull request Apr 10, 2024
The newest version of c-ares has started adding deprecation warnings.
Let's silence those for now to make MacOS builds pass again.

We might want to actually start using the new APIs in the future, but
since we want to support some fairly old systems I'm not sure that's a
good plan now. And given [the library author stated that the old
functions would probably never be removed][1], I think we're fine with
simply ignoring the warnings.

[1]: c-ares/c-ares#732 (comment)
@johnthacker
Copy link

ares_set_servers[_ports] was marked as deprecated in favor of ares_set_servers[_ports]_csv "due to inability to set all available server options," but AFAICT it's impossible to set a server's TCP and UDP ports to be different from each other with ares_set_servers_ports_csv. Is that correct? A rare use case, I suppose.

@bradh352
Copy link
Member Author

I've never seen that be used, and its not possible to represent that via a system configuration on any system I'm aware of so it would be some odd application-specific thing I guess. Do you use this for some purpose? If its really needed we might be able to expand the syntax to allow it in csv format.

@johnthacker
Copy link

I am a Wireshark developer, and we allow people to specify a user configurable table of what DNS servers to use optionally for resolving packets instead of the system default. The table allows for each server specifying the TCP and UDP port separately.

I don't know that anyone actually needs two different ports, it was probably set up that way just to map into the struct used by c-ares.

We are looking into what we would need to do to remove the use of deprecated functions.

On a side note, the ares_queue_wait_empty() function added in 1.27 is useful, because when tshark is run in two-pass mode, we want to wait for all the asynchronous lookups in the queue to complete before printing results. (With two passes we can do asynchronous lookups on the first pass.)

@johnthacker
Copy link

Since we don't yet support entering a device for a link local DNS server, we will probably continue to use the deprecated ares_set_servers_ports() for now.

syzop added a commit to unrealircd/unrealircd that referenced this pull request Jul 1, 2024
…for c-ares API.

Ignore these for entire src/dns.c.
Quoting c-ares/c-ares#732 (comment):
"Those deprecated functions will remain available until there is an ABI
 break, which honestly will likely never happen. It's more to encourage
 integrators to move to the more modern functions."
Also, keep in mind that several of these 'deprecations' happened in early 2024
while the new function was introduced in March 2020, like for ares_getaddrinfo().
That isn't all that long ago, only 4 years. So we would need compatibility code
for both the old and new function for a while.
So: we can look into that in some major new UnrealIRCd version, nothing urgent,
and perhaps by then it is long enough that we don't need the fallback to older
functions.
alexey-tikhonov added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 9, 2024
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 added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 11, 2024
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 added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 11, 2024
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 added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 11, 2024
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 added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 11, 2024
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 added a commit to alexey-tikhonov/sssd that referenced this pull request Sep 13, 2024
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 added a commit to SSSD/sssd that referenced this pull request Sep 18, 2024
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.

Reviewed-by: Alejandro López <allopez@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
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.

3 participants