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 downstream project (devilutionx) Windows build completes successfully

…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.
@CLAassistant
Copy link

CLAassistant commented Nov 29, 2025

CLA assistant check
All committers have signed the CLA.

@skypher
Copy link
Author

skypher commented Nov 30, 2025

The fix has already been merged in upstream miniupnp:

miniupnp/miniupnp#866

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.

2 participants