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

don't include winsock2.h in public headers #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ligfx
Copy link

@ligfx ligfx commented Jan 24, 2017

Windows headers have a tendency to pollute the global namespace and can cause issues with code that's not expecting it. enet should be a friendly neighbor and not expose Windows headers to end-users :)

Windows headers have a tendency to pollute the global namespace and
can cause issues with code that's not expecting it. enet should be a
friendly neighbor and not expose Windows headers to end-users.
@ligfx ligfx force-pushed the dont-leak-windows-headers branch from 2e7062d to f9bc5a8 Compare January 24, 2017 22:04
@ligfx
Copy link
Author

ligfx commented Nov 2, 2017

@lsalzman Any thoughts on this?

@lsalzman
Copy link
Owner

lsalzman commented Nov 3, 2017

This won't work because the ENET_SOCKETSET_* and the ENET_HOST_TO_NET/ENET_NET_TO_HOST macros require winsock, and may be used as part of ENet's API.

@ligfx
Copy link
Author

ligfx commented Nov 4, 2017

Dang, I didn't see those. Would you be open to another solution like, forward-declaring the SOCKET/ENetSocket type in the types.h header?

My use-case is I want to keep Windows headers out of my code headers (so they don't propagate), but still be able to use ENet directly in member variables or for function arguments.

@lsalzman
Copy link
Owner

lsalzman commented Nov 4, 2017

I'll work something out with a define you can use before including to stop winsock.

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