Skip to content

socket: introduce SOCK_EAGAIN() and use it#21992

Closed
vszakats wants to merge 41 commits into
curl:masterfrom
vszakats:eagain
Closed

socket: introduce SOCK_EAGAIN() and use it#21992
vszakats wants to merge 41 commits into
curl:masterfrom
vszakats:eagain

Conversation

@vszakats

@vszakats vszakats commented Jun 12, 2026

Copy link
Copy Markdown
Member

To contain the logic of checking for both EWOULDBLOCK and/or EAGAIN
depending on platform/availability. Also to avoid checking for both if
they mapp to the same value, and to avoid PP guards around use.

This also ensures EAGAIN is consistently not checked on Windows, where
headers defined it, but SOCKERRNO never returns it, because curl maps
it to WSAGetLastError().

If they map to the same value, checking them both in an if expression
trips GCC warning -Wlogical-op (the same way it triggers duplicate
case value error in switch).

Also:

  • replace two switch() statements with the new macro.
  • tests/server/sws: make two outliers use the new macro that were only
    checking for EWOULDBLOCK before this patch, in connect_to().
  • move variables to the left-side of expressions, where missing.
  • rustls: use a variant of this macro that uses raw EWOULDBLOCK.
    Tried tracing it back to the origins, but I couldn't figure out if
    this is working as expected on all supported Windows versions in
    Rust. It seems to be using GetLastError(), according to
    https://docs.rs/system_error/0.2.0/system_error/, which would be
    probably incorrect.

Notes:

  • it's probably a good idea to assign SOCKERRNO to a variable before
    passing it to this macro.

Cherry-picked from #21893


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

  • better name than SOCK_EWOULDBLOCK_EAGAIN() for the macro? I figure it's nice to include both names
    it's covering, but don't feel strongly about it. → [trying SOCK_EAGAIN() for short]
  • cf-socket: maybe turn a 3-way switch into an if and use this macro? [DONE, also for another instance]
  • there was indication in code/history that some rare platforms may not
    have EWOULDBLOCK but do have EAGAIN. This wasn't consistently
    supported and this patch leaves it as-is. If this causes an issue, it
    should be easy to support this case with the new macro.
    Feature request : Improve portability libssh2/libssh2#171
    Add an EWOULDBLOCK check for better portability libssh2/libssh2#172
    libssh2/libssh2@92f7686
    On a second read this seem overkill, and EWOULDBLOCK was added for VMS-only
    then later extended to more platforms but keeping a check for EWOULDBLOCK.
    POSIX should have this macro. Also curl never handled a build without it.

vszakats added 28 commits June 12, 2026 16:34
```
/Users/runner/work/curl/curl/lib/cf-socket.c: In function 'cf_socket_send':
/Users/runner/work/curl/curl/lib/cf-socket.c:1498:36: error: logical 'or' of equal expressions [-Werror=logical-op]
 1498 |       (SOCKEWOULDBLOCK == sockerr) ||
      |                                    ^~
/Users/runner/work/curl/curl/lib/cf-socket.c: In function 'cf_socket_recv':
/Users/runner/work/curl/curl/lib/cf-socket.c:1564:36: error: logical 'or' of equal expressions [-Werror=logical-op]
 1564 |       (SOCKEWOULDBLOCK == sockerr) ||
      |                                    ^~
In file included from /Users/runner/work/curl/curl/bld/lib/CMakeFiles/libcurl_shared.dir/Unity/unity_0_c.c:331:
/Users/runner/work/curl/curl/lib/socketpair.c: In function 'Curl_wakeup_signal':
/Users/runner/work/curl/curl/lib/socketpair.c:329:35: error: logical 'or' of equal expressions [-Werror=logical-op]
  329 |       if((err == SOCKEWOULDBLOCK) || (err == EAGAIN))
      |                                   ^~
```
```
/Users/runner/work/curl/curl/lib/socketpair.c:329:15: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
      if((err == SOCKEWOULDBLOCK)
          ~~~~^~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/socketpair.c:329:15: note: remove extraneous parentheses around the comparison to silence this warning
      if((err == SOCKEWOULDBLOCK)
         ~    ^                 ~
/Users/runner/work/curl/curl/lib/socketpair.c:329:15: note: use '=' to turn this equality comparison into an assignment
      if((err == SOCKEWOULDBLOCK)
              ^~
              =
/Users/runner/work/curl/curl/lib/socketpair.c:359:21: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
      if((SOCKERRNO == SOCKEWOULDBLOCK)
          ~~~~~~~~~~^~~~~~~~~~~~~~~~~~
/Users/runner/work/curl/curl/lib/socketpair.c:359:21: note: remove extraneous parentheses around the comparison to silence this warning
      if((SOCKERRNO == SOCKEWOULDBLOCK)
         ~          ^                 ~
/Users/runner/work/curl/curl/lib/socketpair.c:359:21: note: use '=' to turn this equality comparison into an assignment
      if((SOCKERRNO == SOCKEWOULDBLOCK)
                    ^~
                    =
2 errors generated.
```
https://github.com/curl/curl/actions/runs/27107980068/job/80000462156?pr=21893
comparing to raw EWOULDBLOCK may not be working as expected on Windows.

Based on this:
https://doc.rust-lang.org/std/io/struct.Error.html#method.last_os_error
Rust doesn't seem to be returning the socket error on Windows, but
a GetLastError() which does not return EWOULDBLOCK or SOCKEWOULDBLOCK.
There was an unprotected unconditional use in src. Also in vquic
and socketpair, on conditions.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread lib/cf-socket.c Outdated
Comment thread lib/cf-socket.c Outdated
Comment thread lib/vquic/vquic.c Outdated
Comment thread lib/vquic/vquic.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread lib/cf-socket.c Outdated
Comment thread lib/cf-socket.c Outdated
Comment thread lib/socketpair.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@vszakats vszakats changed the title socket: introduce SOCK_EWOULDBLOCK_EAGAIN(), use it socket: introduce SOCK_EAGAIN(), use it Jun 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread lib/curl_setup.h
Comment thread lib/vtls/rustls.c
@vszakats vszakats changed the title socket: introduce SOCK_EAGAIN(), use it socket: introduce SOCK_EAGAIN() and use it Jun 12, 2026
@vszakats vszakats closed this in 879a151 Jun 12, 2026
@vszakats vszakats deleted the eagain branch June 12, 2026 21:28
vszakats added a commit that referenced this pull request Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants