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

c-ares: fix/tidy-up macro initializations, avoid a deprecated function #16131

Closed
wants to merge 4 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jan 30, 2025

  • replace deprecated ares_init() call with ares_init_options().
    Follow-up to 0d4fdbf asyn-thread: use c-ares to resolve HTTPS RR #16054

  • dedupe CARES_STATICLIB initalizations into curl_setup.h, to
    ensure it's defined before the first (and every) ares.h include and
    avoid a potential confusion.

  • move CARES_NO_DEPRECATED from build level to curl_setup.h.
    To work regardless of build system.
    It is necessary because curl calls ares_getsock() from two places,
    of which one feeds a chain of wrappers: Curl_ares_getsock(),
    Curl_resolver_getsock(), Curl_resolv_getsock().

@vszakats vszakats added name lookup DNS and related tech tidy-up labels Jan 30, 2025
@vszakats vszakats marked this pull request as draft January 30, 2025 11:45
@github-actions github-actions bot added the build label Jan 30, 2025
The latter is already used elsewhere in the code.

This is in the HTTPS-RR codepath, I'm not sure it's compiled in CI.
@vszakats
Copy link
Member Author

vszakats commented Jan 30, 2025

Upstream deprecations enforced here (v1.28.0 2024-03-29):
c-ares/c-ares@5fd3fc3
c-ares/c-ares@85566b5
c-ares/c-ares#732
c-ares/c-ares#706
Deprecations commits (v1.24.0 2023-12-17):
c-ares/c-ares@6c91f51
c-ares/c-ares@ae06072

Reason from ares_getsock() manual:
"This function can only return information up to 16 sockets. If more are
in use, they are simply not reported back."

Recommendation from c-ares:
"It is recommended to use socket state callbacks (ARES_OPT_SOCK_STATE_CB)
registered via ares_init_options(3)."

This seems like a significant change to implement.


Apple clang:

../../lib/asyn-ares.c:88:13: error: 'ares_getsock' is deprecated: Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [-Werror,-Wdeprecated-declarations]
  int max = ares_getsock(channel,
            ^
/opt/homebrew/Cellar/c-ares/1.34.4/include/ares.h:901:14: note: 'ares_getsock' has been explicitly marked deprecated here
CARES_EXTERN CARES_DEPRECATED_FOR(
             ^
/opt/homebrew/Cellar/c-ares/1.34.4/include/ares.h:153:22: note: expanded from macro 'CARES_DEPRECATED_FOR'
      __attribute__((deprecated("Use " #f " instead")))
                     ^
In file included from libcurl_unity.c:4:
../../lib/asyn-ares.c:116:13: error: 'ares_getsock' is deprecated: Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [-Werror,-Wdeprecated-declarations]
  bitmask = ares_getsock(channel, socks, ARES_GETSOCK_MAXNUM);
            ^
/opt/homebrew/Cellar/c-ares/1.34.4/include/ares.h:901:14: note: 'ares_getsock' has been explicitly marked deprecated here
CARES_EXTERN CARES_DEPRECATED_FOR(
             ^
/opt/homebrew/Cellar/c-ares/1.34.4/include/ares.h:153:22: note: expanded from macro 'CARES_DEPRECATED_FOR'
      __attribute__((deprecated("Use " #f " instead")))
                     ^
2 errors generated.

https://github.com/curl/curl/actions/runs/13051625348/job/36413034464?pr=16131#step:11:77

mingw-w64:

D:/a/curl/curl/lib/asyn-ares.c: In function 'Curl_ares_getsock':
D:/a/curl/curl/lib/asyn-ares.c:88:3: error: 'ares_getsock' is deprecated: Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [-Werror=deprecated-declarations]
   88 |   int max = ares_getsock(channel,
      |   ^~~
In file included from D:/a/curl/curl/lib/httpsrr.h:30,
                 from D:/a/curl/curl/lib/asyn.h:29,
                 from D:/a/curl/curl/lib/hostip.h:31,
                 from D:/a/curl/curl/lib/urldata.h:160,
                 from D:/a/curl/curl/lib/altsvc.c:32,
                 from D:/a/curl/curl/bld/lib/CMakeFiles/libcurl_object.dir/Unity/unity_0_c.c:4:
D:/a/_temp/msys64/mingw64/include/ares.h:903:31: note: declared here
  903 |   ARES_OPT_SOCK_STATE_CB) int ares_getsock(const ares_channel_t *channel,
      |                               ^~~~~~~~~~~~
D:/a/curl/curl/lib/asyn-ares.c: In function 'Curl_ares_perform':
D:/a/curl/curl/lib/asyn-ares.c:116:3: error: 'ares_getsock' is deprecated: Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [-Werror=deprecated-declarations]
  116 |   bitmask = ares_getsock(channel, socks, ARES_GETSOCK_MAXNUM);
      |   ^~~~~~~
D:/a/_temp/msys64/mingw64/include/ares.h:903:31: note: declared here
  903 |   ARES_OPT_SOCK_STATE_CB) int ares_getsock(const ares_channel_t *channel,
      |                               ^~~~~~~~~~~~

https://github.com/curl/curl/actions/runs/13051625361/job/36413041078?pr=16131#step:10:25

MSVC:

D:\a\curl\curl\lib\asyn-ares.c(116,13): warning C4996: 'ares_getsock': Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]

https://github.com/curl/curl/actions/runs/13051625361/job/36413037518?pr=16131#step:9:32

D:\a\curl\curl\lib\asyn-ares.c(332,13): error C2220: the following warning is treated as an error [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]
D:\a\curl\curl\lib\asyn-ares.c(332,13): warning C4996: 'ares_getsock': Use ARES_OPT_EVENT_THREAD or ARES_OPT_SOCK_STATE_CB instead [D:\a\curl\curl\bld\lib\libcurl_object.vcxproj]

#14202

To fix potential confusion in unity builds.
…ems)

To make it work with other build systems, and to control this from
a single place.
@vszakats vszakats changed the title c-ares: stop suppressing deprecation warnings c-ares: tidy-up macro initializations, avoid a deprecated function Jan 30, 2025
@vszakats vszakats marked this pull request as ready for review January 30, 2025 12:37
@vszakats vszakats changed the title c-ares: tidy-up macro initializations, avoid a deprecated function c-ares: tidy-up/fix macro initializations, avoid a deprecated function Jan 31, 2025
@vszakats vszakats changed the title c-ares: tidy-up/fix macro initializations, avoid a deprecated function c-ares: fix/tidy-up initializations, avoid a deprecated function Jan 31, 2025
@vszakats vszakats changed the title c-ares: fix/tidy-up initializations, avoid a deprecated function c-ares: fix/tidy-up macro initializations, avoid a deprecated function Jan 31, 2025
@vszakats
Copy link
Member Author

This PR would be nice to merge before the release.

@vszakats vszakats closed this in 671e83f Feb 3, 2025
@vszakats vszakats deleted the aresdepr branch February 3, 2025 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant