curlx_inet_ntop: return CURLcode, drop setting errno

[vszakats]

To simplify and to remove an exception where errno was reused to
return a socket error codes on Windows.

The error was used by one call site (sockaddr2string() in
cf-socket.c), but it was in practice always propagated as
SOCKEAFNOSUPPORT to callers.

Also:

• cf-socket: update 3 error messages to show CURLcode accordingly.
• if2ip: handle curlx_inet_ntop() error in Curl_if2ip().
• dnsd: display CURLcode on two errors.

Follow-up to 39dec13 #22170

---

https://github.com/curl/curl/pull/22229/files?w=1

• perhaps stop returning sockerr from ntop, since the only user
  sockaddr2string() was overwriting the result with the same error
  SOCKEAFNOSUPPORT, prior to this patch.
  It means the code never actually saw the values WSAEINVAL / ENOSPC.
• curlx_inet_pton(): also stop setting errno. Separate PR. → curlx_inet_pton: drop setting errno on error #22607

[Copilot]

Pull request overview

This PR refactors curl’s local curlx_inet_ntop() API to return platform socket errors via an explicit out-parameter instead of using errno, removing a Windows-specific exception and making error handling more consistent across platforms.

Changes:

• Extend curlx_inet_ntop() to accept int *sockerr and propagate SOCKEAFNOSUPPORT / SOCKEINVAL through it (rather than errno / ENOSPC).
• Stop setting errno in curlx_inet_pton() (callers don’t use it).
• Update call sites (notably sockaddr2string() and its unit tests) to pass/handle the new parameter.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/unit/unit1961.c	Updates curlx_inet_ntop() unit test calls for the new sockerr parameter.
tests/unit/unit1609.c	Updates sockaddr2string() unit test call to pass a sockerr out-parameter.
tests/unit/unit1607.c	Updates sockaddr2string() unit test call to pass a sockerr out-parameter.
tests/server/dnsd.c	Updates server-side logging conversions to pass NULL for sockerr.
lib/urlapi.c	Updates IPv6 formatting call site to pass NULL for sockerr.
lib/if2ip.c	Updates interface-to-IP formatting call sites to pass NULL for sockerr.
lib/httpsrr.c	Updates address printing helper to pass NULL for sockerr.
lib/hostip.c	Updates printable-address helper to pass NULL for sockerr.
lib/ftp.c	Updates FTP PORT host formatting to pass NULL for sockerr.
lib/curlx/inet_pton.c	Removes errno assignment and updates related comments.
lib/curlx/inet_ntop.h	Updates curlx_inet_ntop() declaration with int *sockerr.
lib/curlx/inet_ntop.c	Implements sockerr propagation and replaces prior errno behavior.
lib/cf-socket.c	Plumbs sockerr through sockaddr2string() and related connection logging/error paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[vszakats]

Oh, does this mean that the only code using these errnos, was only
ever returning SOCKEAFNOSUPPORT after all? If so, and nobody
noticed and it didn't cause issues, maybe this dance with sockerrs is
superfluous and this could be simplified by dropping it completely?

The store a fixed code in ctx→sockerr in case of any error.

Any second look / opinion on this?

[bagder]

I propose going one step further and make it as we would make a proper curl function and skip the errno. Return zero on success or an error code.

/* returns one of the values as defined */
int curlx_inet_ntop(int af, const void *src, char *buf, size_t size);

#define CURL_INET_OK 0
#define CURL_INET_NOSPACE 1
#define CURL_INET_NOSUPPORT 2
...

[vszakats]

I propose going one step further and make it as we would make a proper curl function and skip the errno. Return zero on success or an error code.

/* returns one of the values as defined */
int curlx_inet_ntop(int af, const void *src, char *buf, size_t size);

#define CURL_INET_OK 0
#define CURL_INET_NOSPACE 1
#define CURL_INET_NOSUPPORT 2
...

I started out from ~there (with CURLcode 2af660f), then went back
to sockerrs, because the only caller interested in this detail
stores this code in the socket context sockerr variable. Then
it turned out that due a minor mistake in the cf-socket wrapper
function (as spotted by Copilot), the actual error registered was
always SOCKEAFNOSUPPORT.

That said we can use dedicated error codes (or CURLcode) and
convert them to sockerr values for the context, but before going
down that road, I'd like to confirm if that's really what we want
given the context above.

[bagder]

I'm not following. If we completely drop the errno handling and don't even pass back any extra value in an argument, what breaks?

[vszakats]

Indeed, since in practice the returned sockerr wasn't used, I went
ahead a dropped the errcode return value.

In the single call where that return value was used previously, but
in practice was always SOCKEAFNOSUPPORT, the code continues
to set it on failure.

(sorry for the delay with this.)

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/cf-socket.c:1120

• This error path now reports only "inet_ntop() failed" and always sets ctx->sockerr to SOCKEAFNOSUPPORT, which makes the failure hard to diagnose. Even if you want to keep a fixed sockerr value, the message should at least include the address family and the chosen sockerr value so logs remain actionable.

    ctx->sockerr = SOCKEAFNOSUPPORT;
    /* malformed address or bug in inet_ntop, try next address */
    failf(data, "curl_sa_addr inet_ntop() failed");
    return CURLE_FAILED_INIT;

lib/cf-socket.c:2167

• This message was simplified to omit errno/strerror, but it is now too generic and attributes the failure specifically to inet_ntop(). Since the failure comes from sockaddr2string(), include at least the address family so debugging logs can distinguish unsupported families from other conversion failures.

    failf(data, "ssrem inet_ntop() failed");

lib/cf-socket.c:1099

• The new log line loses context (errno/strerror) and also hard-codes the failure as an inet_ntop() problem, but the failure actually comes from sockaddr2string() (which can fail for reasons other than inet_ntop). Including the address family makes this actionable when debugging.

This issue also appears on line 2167 of the same file.

      infof(data, "ssloc inet_ntop() failed");

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[vszakats]

Copilot (low-confidence) reports that after this patch there is one infof()
and two failf() calls no longer reporting low-level details (no-memory vs.
unsupported.) It's true. I can't judge how significant it is. If it is, I can re-add
the error code to display more. I'd go with CURLcode to keep it simpler.

[bagder]

I'm still in favor of "just return CURLcode" (and avoid errno).

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lib/curlx/inet_ntop.c:188

• The comment still says this function returns the destination pointer, but the new signature returns only a CURLcode. Remove the pointer-return sentence so the documented contract is unambiguous.

 * Returns pointer to presentation format address ('buf').
 * Returns CURLcode.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lib/cf-socket.c:1129

• This user-facing failure now reports only an unlabeled numeric CURLcode, losing the explanation previously supplied by curlx_strerror(). Include curl_easy_strerror(result) (and label the number as a CURLcode) so users can identify the actual conversion failure.

    failf(data, "curl_sa_addr inet_ntop() failed with %d", (int)result);

lib/cf-socket.c:2179

• The revised message emits only an unlabeled integer for the CURLcode, unlike the previous diagnostic which included readable error text. Add curl_easy_strerror(result) and identify the numeric value as a CURLcode.

    failf(data, "ssrem inet_ntop() failed with %d", (int)result);

lib/if2ip.c:158

• The return value is ignored even though ipstr is uninitialized until conversion succeeds. If an unsupported address family reaches this path, curlx_inet_ntop() returns an error without writing ipstr, and the following %s reads indeterminate data while the function reports IF2IP_FOUND. Handle the error before formatting the result.

            (void)curlx_inet_ntop(af, addr, ipstr, sizeof(ipstr));
            curl_msnprintf(buf, buf_size, "%s%s", ipstr, scope);

lib/cf-socket.c:1107

• This updated diagnostic replaces the prior descriptive socket error with an unlabeled numeric CURLcode, producing messages such as failed with 100. Include the code type and curl_easy_strerror(result) so the failure remains actionable.

This issue also appears in the following locations of the same file:

• line 1129
• line 2179

      if(result)
        infof(data, "ssloc inet_ntop() failed with %d", (int)result);

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lib/if2ip.c:159

• On conversion failure this still sets res to IF2IP_FOUND, even though the contract says that result means an address was stored in buf; the buffer instead receives only scope (or an empty string). The ioctl implementation below returns IF2IP_NOT_FOUND for the same failure. Set IF2IP_FOUND and format the address only after a successful conversion.

            res = IF2IP_FOUND;
            result = curlx_inet_ntop(af, addr, ipstr, sizeof(ipstr));
            curl_msnprintf(buf, buf_size, "%s%s", result ? "" : ipstr, scope);

[vszakats]

Reworked to return CURLcode, and ready from my end.