Skip to content

build: enable -Wlogical-op picky warning for GCC 4.4+#21893

Closed
vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:logical-op
Closed

build: enable -Wlogical-op picky warning for GCC 4.4+#21893
vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:logical-op

Conversation

@vszakats

@vszakats vszakats commented Jun 7, 2026

Copy link
Copy Markdown
Member

Follow-up to 879a151 #21992


@github-actions github-actions Bot added the build label Jun 7, 2026
@vszakats vszakats marked this pull request as draft June 7, 2026 23:15
@testclutch

This comment has been minimized.

@vszakats vszakats marked this pull request as ready for review June 10, 2026 11:42
vszakats added a commit that referenced this pull request Jun 12, 2026
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

Closes #21992
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.

2 participants