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

please remove the dllexport of static libs #184

Closed
z16166 opened this issue Jul 28, 2022 · 6 comments
Closed

please remove the dllexport of static libs #184

z16166 opened this issue Jul 28, 2022 · 6 comments

Comments

@z16166
Copy link

z16166 commented Jul 28, 2022

hi,

Currently uWebSockets and its dependency "uSockets" define a macro "WIN32_EXPORT" in header file "libusockets.h", as the following.
So all uSockets api functions and a uWebSockets class named "struct WIN32_EXPORT WebSocketProtocol" will appear in exe/dll export tables even if we use their static libs.
Please remove "__declspec(dllexport)" for static lib build configuration. Thanks.

#ifdef _WIN32
#ifndef NOMINMAX
#define NOMINMAX
#endif
#include <winsock2.h>
#define LIBUS_SOCKET_DESCRIPTOR SOCKET
#define WIN32_EXPORT __declspec(dllexport)
#else
#define LIBUS_SOCKET_DESCRIPTOR int
#define WIN32_EXPORT
#endif

@z16166
Copy link
Author

z16166 commented Aug 2, 2022

Maybe we can use MSVC macro "_DLL" to detect dll build vs static lib build.

#ifdef _WIN32

#ifdef _DLL
#define WIN32_EXPORT __declspec(dllexport)
#else
#define WIN32_EXPORT
#endif

#endif

@traversaro
Copy link

As the default build system only build static libraries (#99), the easiest fix for this issue is just to define WIN32_EXPORT to an empty string also on Windows. However, at that point WIN32_EXPORT would be empty on all platforms, so probably it could make sense to just remove it altogether. I would be happy to provide a PR to remove it if that is the decision of the mantainers of the library.

@uNetworkingAB
Copy link
Contributor

if you want to add that _DLL macro check under _WIN32 that's fine with me, that sounds very reasonable make PR plaes

@uNetworkingAB
Copy link
Contributor

Now that I think of it, who is really building DLL of uSockets? That's really crappy for performance and should never be done. uSockets should always be a static lib

@traversaro
Copy link

Now that I think of it, who is really building DLL of uSockets? That's really crappy for performance and should never be done. uSockets should always be a static lib

Indeed, that is what I mentioned in #184 (comment) . Do you prefer to simply define WIN32_EXPORT to empty also on Windows or just get rid of WIN32_EXPORT altogether?

@traversaro
Copy link

Now that I think of it, who is really building DLL of uSockets? That's really crappy for performance and should never be done. uSockets should always be a static lib

Indeed, that is what I mentioned in #184 (comment) . Do you prefer to simply define WIN32_EXPORT to empty also on Windows or just get rid of WIN32_EXPORT altogether?

This was handled in b950efd, thanks!

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 a pull request may close this issue.

3 participants