Skip to content

Conversation

@skypher
Copy link

@skypher skypher commented Nov 29, 2025

Summary

GCC 14+ treats -Wincompatible-pointer-types as an error by default. On Windows, setsockopt() expects const char* for the option value, and socket timeouts (SO_RCVTIMEO/SO_SNDTIMEO) use DWORD (milliseconds) instead of struct timeval.

This PR fixes the miniupnpc connecthostport.c to use the correct types on Windows:

  • Use DWORD instead of struct timeval for the timeout variable
  • Cast to (const char *) when calling setsockopt()
  • Use milliseconds (3000) instead of seconds for the timeout value

Note: This was a latent bug

The original code was actually broken on Windows, not just a type mismatch. The code passed a struct timeval (8 bytes: {tv_sec=3, tv_usec=0}) but Windows expects a DWORD (4 bytes) containing milliseconds.

Windows would read only the first 4 bytes (value 3) as 3 milliseconds instead of the intended 3 seconds. The code appeared to work because connections usually succeeded before the tiny 3ms timeout kicked in, and older GCC versions only warned about the pointer type mismatch rather than erroring.

Test plan

  • Verified fix compiles with MinGW GCC 15.2.0 cross-compile for Windows
  • Full devilutionx Windows build completes successfully

adamierymenko and others added 30 commits September 25, 2024 20:12
Add some padding after non-ASCII comment
add `make docker-release`  command & update dockerfile
Fix build error under certain character sets in Windows
When building via `make core` to make libzerotiercore.a, you can't link unless OSUtils.cpp is also built & linked.
Move osutils/OSUtils.o into CORE_OBJS
Bumps [rustls](https://github.com/rustls/rustls) from 0.23.15 to 0.23.18.
- [Release notes](https://github.com/rustls/rustls/releases)
- [Changelog](https://github.com/rustls/rustls/blob/main/CHANGELOG.md)
- [Commits](rustls/rustls@v/0.23.15...v/0.23.18)

---
updated-dependencies:
- dependency-name: rustls
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
…bits/rustls-0.23.18

Bump rustls from 0.23.15 to 0.23.18 in /rustybits
adamierymenko and others added 18 commits September 11, 2025 13:52
V1.16 kitchen sink branch, 1.16 ready.
Drastically shortened main readme and linked out to build notes instead. Clarified licensing info to just match license.txt.
Shorten/clean README.md. Split README into a few separate, including BUILD. Correctly references licensing information.
_enabled is set to true by setUpPostDecodeReceiveThreads(), so disabled
until then, but the constructor wasn't initializing it. _concurrency is
not being used before being set but for safety's sake, ensure it has a
starting value as well.

Also, remove the vestigial _rxThreadCount, which is no longer used.
Ensure members in PacketMultiplexer are initialized
Copy link
Member

@StephenCWills StephenCWills left a comment

Choose a reason for hiding this comment

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

Looks good, but it feels like we should be sending this upstream.

@StephenCWills StephenCWills changed the base branch from master to devx-1.4.0 November 29, 2025 03:51
@StephenCWills StephenCWills force-pushed the fix/mingw-setsockopt-gcc14 branch 2 times, most recently from 51288c3 to eb56c9f Compare November 29, 2025 03:55
@StephenCWills
Copy link
Member

Rebased onto the devx-1.4.0 branch.

@skypher
Copy link
Author

skypher commented Nov 29, 2025

Looks good, but it feels like we should be sending this upstream.

Agreed, going to file a PR there too.

…C 14+

GCC 14+ treats -Wincompatible-pointer-types as an error by default.
On Windows, setsockopt() expects const char* for the option value, and
socket timeouts (SO_RCVTIMEO/SO_SNDTIMEO) use DWORD (milliseconds)
instead of struct timeval.

This was also a latent bug: the original code passed a struct timeval
(8 bytes: {tv_sec=3, tv_usec=0}) but Windows expects a DWORD (4 bytes)
containing milliseconds. Windows would read only the first 4 bytes
(value 3) as 3 milliseconds instead of the intended 3 seconds.
@skypher skypher force-pushed the fix/mingw-setsockopt-gcc14 branch from eb56c9f to 0ca24aa Compare November 29, 2025 04:52
@skypher
Copy link
Author

skypher commented Nov 29, 2025

Upstream PR: zerotier#2542

@skypher
Copy link
Author

skypher commented Nov 29, 2025

Rebased onto the devx-1.4.0 branch.

Are we good to merge here or should I finish resolving the conflicts?

@StephenCWills
Copy link
Member

It's not possible to merge without resolving conflicts.

Also, if these new commits are from upstream, we should probably consider creating a separate PR or even rebasing our commits on a new branch instead of pulling them into this PR.

Lastly, I noticed you opened a PR with https://github.com/zerotier/ZeroTierOne, but they've actually pulled in miniupnpc from https://github.com/miniupnp/miniupnp. It looks like that project is still active as of three months ago so you might want to also submit your fix there.

@skypher
Copy link
Author

skypher commented Nov 29, 2025

miniupnp/miniupnp#866

Let's wait a bit to get feedback from upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.