uNetworking / uSockets

Miniscule cross-platform eventing, networking & crypto for async applications
Apache License 2.0
1.29k stars 267 forks source link

please remove the dllexport of static libs #184

Closed z16166 closed 1 year ago

z16166 commented 2 years ago

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

define LIBUS_SOCKET_DESCRIPTOR SOCKET

define WIN32_EXPORT __declspec(dllexport)

else

define LIBUS_SOCKET_DESCRIPTOR int

define WIN32_EXPORT

endif

z16166 commented 2 years ago

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 commented 1 year ago

As the default build system only build static libraries (https://github.com/uNetworking/uSockets/issues/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 commented 1 year ago

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

uNetworkingAB commented 1 year ago

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 commented 1 year ago

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 https://github.com/uNetworking/uSockets/issues/184#issuecomment-1369038030 . Do you prefer to simply define WIN32_EXPORT to empty also on Windows or just get rid of WIN32_EXPORT altogether?

traversaro commented 1 year ago

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 https://github.com/uNetworking/uSockets/commit/b950efd6b10f06dd3ecb5b692e5d415f48474647, thanks!