Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zmq.h should not include winsock2.h #110

Closed
hintjens opened this issue Nov 3, 2010 · 7 comments
Closed

zmq.h should not include winsock2.h #110

hintjens opened this issue Nov 3, 2010 · 7 comments

Comments

@hintjens
Copy link
Member

hintjens commented Nov 3, 2010

zmq.h includes winsock2.h which has the side-effect of defining FD_SETSIZE to be 64 for all programs that include zmq.h. This inclusion should be removed.

@sustrik
Copy link
Member

sustrik commented Nov 4, 2010

Problem is getting the SOCKET type defined to be used in zmq_pollitem_t structure. Either we include winsock2.h from zmq.h or make it obligatory for the user to include winsock2.h before including zmq.h.

@hintjens
Copy link
Member Author

hintjens commented Nov 7, 2010

Or you just use:

#ifdef _WIN32
typedef u_int SOCKET;
#endif

And then in sources like zmq.cpp you'd include windows.hpp instead of using the winsock.h definitions from zmq.h.

@sustrik
Copy link
Member

sustrik commented Nov 8, 2010

SOCKET is not an u_int. The actual type depends on platform (32/64) and IIRC the definition contains some MSVC-specific constructs. Thus, to make it work, the definition should be two layers of ifdefs checking for platform and for compiler (at least msvc, gcc/mingw and icc).

@hintjens
Copy link
Member Author

hintjens commented Nov 8, 2010

Well. The MSVC winsock.h definition is 'unsigned int', which it typedefs as 'u_int'. On 64-bit compilers I assume it is a 'unsigned short'?

I'd suggest that the simplest way to fix this is to (post 2.1) just make the change and then allow people who have icc and mingw to help define the right conditionals. This will be better than including winsock.h here (in any case the code already seems fragile, why is it including winsock.h and not doing the same as in windows.hpp:

//  Enable winsock (not included when WIN32_LEAN_AND_MEAN is defined).
#if(_WIN32_WINNT >= 0x0400)
#include <winsock2.h>
#include <mswsock.h>
#else
#include <winsock.h>
#endif

?

@TobiSchluter
Copy link

TobiSchluter commented Sep 13, 2019

Can I revive this? Including winsock2.h puts ordering constraints on users, but only on Windows. These can break cross-platform builds and other libraries which may be unwise enough to impose similar constraints.

winsock2.h is only needed for the definition of SOCKET used on line 509 of zmq.h (in release 4.3.2, line 500 on the master). SOCKET evaluates to UINT_PTR which by definition is equivalent to uintptr_t (*) and which is defined to unsigned __int64 on win64, and unsigned int on WIN32. Everything seems to work flawlessly if I remove #include <winsock2.h> and replace SOCKET with uintptr_t.

Would a pull request doing the little bit of #ifdeffery to get the same behavior without including winsock2.h be accepted?

(*) UINT_PTR is required to be the same size as a pointer, which is strictly speaking different from uintptr_t, which is only guaranteed to be able to hold a pointer.

@TobiSchluter
Copy link

ps I added a pull request to that effect because I need to do it anyway locally. #3681

@sigiesec
Copy link
Member

Fixed by #3681

benjdero pushed a commit to benjdero/libzmq that referenced this issue Feb 20, 2023
Forgot to fix zmq_poll set size, causing everything to crash
bluca pushed a commit that referenced this issue Oct 31, 2023
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

No branches or pull requests

4 participants