build: always use local inet_pton()/inet_ntop() implementations#22170
Closed
vszakats wants to merge 9 commits into
Closed
build: always use local inet_pton()/inet_ntop() implementations#22170vszakats wants to merge 9 commits into
inet_pton()/inet_ntop() implementations#22170vszakats wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes curl’s behavior across build systems and platforms by always using curl’s local curlx_inet_pton() / curlx_inet_ntop() implementations, removing the prior autotools/CMake feature-detection (and runtime verification) logic that could diverge—especially for cross-builds.
Changes:
- Remove autotools and CMake detection/verification of system
inet_pton()/inet_ntop()and related config defines. - Add unit test coverage (unit1961 + test1961) to verify
curlx_inet_pton()/curlx_inet_ntop()behavior. - Enforce
inet_pton()/inet_ntop()as banned functions viachecksrc, with an explicit allowlist in theblock_ip.cexample.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/unit1961.c | Adds a new unit test validating curlx_inet_ntop() / curlx_inet_pton() formatting/parsing behavior. |
| tests/unit/Makefile.inc | Registers the new unit test source in the unit test build. |
| tests/server/dnsd.c | Avoids logging NULL from curlx_inet_ntop() by handling failures explicitly. |
| tests/libtest/Makefile.inc | Ensures curlx inet implementations are linked into libtests where needed (notably for shared builds). |
| tests/libtest/lib1960.c | Switches test1960 to always use curlx_inet_pton() and removes conditional compilation around system availability. |
| tests/libtest/first.h | Includes curlx/inet_ntop.h and curlx/inet_pton.h for libtest compilation. |
| tests/data/test1961 | Adds a new test definition for running unit1961 via the test harness. |
| tests/data/Makefile.am | Adds test1961 to the distributed test data list. |
| scripts/checksrc.pl | Bans inet_pton / inet_ntop usage across the tree via checksrc. |
| m4/curl-functions.m4 | Removes autotools include helper and the inet_pton / inet_ntop detection + runtime verification macros. |
| lib/curlx/inet_pton.h | Removes system-function macro mapping; always declares curlx_inet_pton(). |
| lib/curlx/inet_pton.c | Builds the local curlx_inet_pton() implementation unconditionally (no longer gated on HAVE_INET_PTON). |
| lib/curlx/inet_ntop.h | Removes system-function macro mapping; always declares curlx_inet_ntop(). |
| lib/curlx/inet_ntop.c | Builds the local curlx_inet_ntop() implementation unconditionally (no longer gated on HAVE_INET_NTOP). |
| lib/curl_config-cmake.h.in | Removes HAVE_INET_PTON / HAVE_INET_NTOP config defines from the CMake config header template. |
| lib/config-win32.h | Removes stale comments/undefs related to HAVE_INET_PTON / HAVE_INET_NTOP. |
| docs/internals/CODE_STYLE.md | Documents inet_pton / inet_ntop as banned functions. |
| docs/examples/block_ip.c | Adds checksrc allowlist directives for intentional system inet_pton / inet_ntop usage in the example. |
| configure.ac | Stops invoking the autotools checks for system inet_pton / inet_ntop. |
| CMakeLists.txt | Stops probing for system inet_pton / inet_ntop in non-Windows builds. |
| CMake/win32-cache.cmake | Removes cached HAVE_INET_* entries that are no longer used. |
| CMake/unix-cache.cmake | Removes cached HAVE_INET_* entries that are no longer used. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if(memcmp(ipv6res, "fe80::214:4fff:fe0b:76c8", 24)) | ||
| return 1; /* fail */ | ||
|
|
||
| /* verify working RFC 4291 zero prefixed IPv4 - mapped format */ |
Member
Author
There was a problem hiding this comment.
(Pre-existing / unrelated.)
bagder
approved these changes
Jun 25, 2026
5 tasks
```
home/runner/work/curl/curl/tests/server/dnsd.c:498:37: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
498 | logmsg("[%d] response A (%x) '%s'", qid, (unsigned int)QTYPE_A,
| ^~
```
Ref: https://github.com/curl/curl/actions/runs/28167757541/job/83423904656?pr=22170
d5819ba to
3bafca8
Compare
1 task
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Also repurpose existing build-time feature checks into unit test 1961,
to verify.
Prior to this patch these functions were auto-detected with both
autotools and cmake. In case of autotools there was an extra
verification phase ensuring the functions work as expected. This step
required running the function, thus was limited to non-cross-builds. For
cross-builds and CMake it always used the system implementation if
present. On Windows it always used the local implementation, because
availability/use is complicated there.
After this patch all platforms, always use the local implementation,
which is known to be accurate. This makes curl behave more consistently,
and simplifies the build process, a fixes cross-builds and CMake
auto-detection differences.
Also:
block_ip.cexample.Ref: #22137 (comment)
Ref: #22137
Ref: 8537a5b #16577
(such as including headers that are now unused)
possibly separate PR.
errno,and passing a socket error through it.
→ Check if
errnois used at all. [YES for inet_ntop only: cf-socket.c /sockaddr2string()does use it]→ curlx_inet_ntop: return socket error via argument #22229
to select IPv4 vs IPv6. That'll also need untangling it from
AF_INET[6]in some callers that pass through that value from structs/variables.
It'd be useful because IPv6 support is also maintained here in builds/envs
with IPv6 support missing/disabled, and in such case there is local hack
adding a surrogate
AF_INET6macro, with fragile detection, and stomping ona system symbol.
→ figure out which headers define it, and perhaps introduce a local macro
on top of external ones?