@irwir does it fixes your problem ?

This complicates code but does not resolve the isssue: 32-bit signed integer vs unsigned 32 and 64-bit requires type casts with potential loss of data.
Proper fix should restore the declaration to
simpleUPnPcommand(SOCKET s, const char * url, const char * service,
and calls should use INVALID_SOCKET constant instead of -1.

@irwir the prototype has been unchanged for at least 13 years and never has been a problem.
https://github.com/miniupnp/miniupnp/blame/master/miniupnpc/include/miniupnpc.h#L40

char *
simpleUPnPcommand(int, const char *, const char *,
                  const char *, struct UPNParg *,
                  int *);

winsock2.h defines SOCKET as follow :
typedef u_int SOCKET; so using int and casting works.

the prototype has been unchanged for at least 13 years and never has been a problem.

For the last 6 years it was SOCKET; the link to the commit was given in my issue.

winsock2.h defines SOCKET as follow :
typedef u_int SOCKET; so using int and casting works.

Where you have got this header?
The correct definition in VS2022 is
typedef UINT_PTR SOCKET;
This is a 64-bit value in 64-bit builds.

the prototype has been unchanged for at least 13 years and never has been a problem. https://github.com/miniupnp/miniupnp/blame/master/miniupnpc/include/miniupnpc.h#L40

IMHO: Would this be a reason for good code quality in a well maintained open source project?

And:

For the last 6 years it was SOCKET; the link to the commit was given in my issue.
5a5b311

😕 ???

the prototype has been unchanged for at least 13 years and never has been a problem.

For the last 6 years it was SOCKET; the link to the commit was given in my issue.

I think there is a confusion between simpleUPnPcommand and simpleUPnPcommand2 which was using a SOCKET argument.
When I removed simpleUPnPcommand2 in c4bd33b, I inadvertently created an inconsistency between the simpleUPnPcommand prototype in the .c and in the .h 😞

winsock2.h defines SOCKET as follow :
typedef u_int SOCKET; so using int and casting works.

Where you have got this header? The correct definition in VS2022 is typedef UINT_PTR SOCKET; This is a 64-bit value in 64-bit builds.

From mingw32.
That's possible that windows is using 64bits in 64 bits builds. But we are not using any negative value in simpleUPnPcommand. No one should call simpleUPnPcommand with -1 or INVALID_SOCKET...

To fix this issue for good the miniupnpc API should be modified to use SOCKET for win32 only.
in POSIX, the type for socket file descriptor is int.
I really don't understand why microsoft hasn't followed the standard in WinSock2 😭

inconsistency between the simpleUPnPcommand prototype in the .c and in the .h

The first argument is used as a socket, hence it must be SOCKET.
Troubles began when directory structure has been changed and miniupnpc_socketdef.h was not included whenever required. Otherwise, there was my old PR, which was manually reimplement by you and everything was good for quite a while. The required additional changes are small.

From mingw32.

The correct file for mingw is _socket_types.h

In fact the 1st argument should be removed. It is not used internally in miniupnpc.
But I don't know how many external projects are using upnpDiscoverDevice() directly :(

In fact the 1st argument should be removed. It is not used internally in miniupnpc.

Things may change in next versions, but this is the current code:

The suggestion would be to add local SOCKET s; variable, and remove condition check at line 200.
Then the first parameter could be removed.
Othewise, the parameter should be renamed, for example, to dummy and (void)dummy; statement has to be added to prevent compiler warnings.

In fact the 1st argument should be removed. It is not used internally in miniupnpc.

Things may change in next versions, but this is the current code:

miniupnp/miniupnpc/src/miniupnpc.c

Lines 200 to 203 in a715632

	if(ISINVALID((SOCKET)s)) {
	s = connecthostport(hostname, port, 0);
	if(ISINVALID((SOCKET)s)) {
	/* failed to connect */

connecthostport returns SOCKET type, and int s is not big enough for a 64-bit value.

that is exactly what this PR is doing : declaring s as SOCKET.

that is exactly what this PR is doing : declaring s as SOCKET.

It does, but in a strange way.
First, explicit use of the 1st parameter that might get dropped in future releases.
Second, the major point of intorducing the SOCKET and related macros was uniform code in WIndows and *nix systems. But this PR again requires a conditional compilation block. If you always use -1 or invalid socket, there is no reason to assign and to check for invalid socket at the line 200.
Finally, a side effect. If a valid socket was passed as the 1st parameter, it might get closed in certain cases. This is not right for 64-bit SOCKET which was passed as an integer.

Yes I see that passing a socket in first argument is indeed bugged.
I guess I had written this function in order to be able to reuse the same socket for several successive SOAP methods calls, but I guess it was never used and the code will probably not work well...