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

Already on GitHub? Sign in to your account

Fix dllexport declspec for static/DLL usage on Windows #133

Merged
merged 1 commit into from Apr 8, 2013

Conversation

Projects
None yet
4 participants
Contributor

TTimo commented Apr 8, 2013

I don't think the dllimport is intended here. It causes external programs using the header to try and link the symbols only from a DLL, breaking static linking. Programs that dynamically link against czmq DLL should define DLL_EXPORT and will be fine.

I don't think the dllimport is intended here. It causes external prog…
…rams using the header to try and link the symbols only from a DLL, breaking static linking. Programs that dynamically link against czmq DLL should define DLL_EXPORT and will be fine.

hintjens added a commit that referenced this pull request Apr 8, 2013

Merge pull request #133 from TTimo/master
Fix dllexport declspec for static/DLL usage on Windows

@hintjens hintjens merged commit a980486 into zeromq:master Apr 8, 2013

Contributor

vortechs2000 commented Apr 8, 2013

It's still not quite right...if DLL_EXPORT is defined, you get __declspec(dllexport), not __declspec(dllimport) and the dllimport is required for linking with the DLL version of czmq.

I've got another couple changes for the czmq.sln and project files anyway, so I'll see if I can get this fixed as well.

Contributor

pijyoi commented Apr 8, 2013

A solution can be found in zmq.h. ZMQ_STATIC is defined when static linking
is desired.
On Apr 8, 2013 9:37 PM, "AJ Lewis" notifications@github.com wrote:

It's still not quite right...if DLL_EXPORT is defined, you get
__declspec(dllexport), not __declspec(dllimport) and the dllimport is
required for linking with the DLL version of czmq.

I've got another couple changes for the czmq.sln and project files anyway,
so I'll see if I can get this fixed as well.


Reply to this email directly or view it on GitHubhttps://github.com/zeromq/czmq/pull/133#issuecomment-16050580
.

Contributor

TTimo commented Apr 8, 2013

I think it would be fine, if you are using the DLL version of czmq, you
should define DLL_EXPORT, and your program will correctly look for the
symbols in a DLL.
The __declspec(dllimport) would be needed if you are compiling czmq as a
DLL and want to grab symbols from another DLL? For instance a DLL version
of libzmq?

One thing that isn't clear, is if DLL configuration for czmq should mean
libzmq as a DLL + czmq as a DLL, or just one DLL that is libzmq and czmq
(I'd rather do the second option)

On Mon, Apr 8, 2013 at 8:37 AM, AJ Lewis notifications@github.com wrote:

It's still not quite right...if DLL_EXPORT is defined, you get
__declspec(dllexport), not __declspec(dllimport) and the dllimport is
required for linking with the DLL version of czmq.

I've got another couple changes for the czmq.sln and project files anyway,
so I'll see if I can get this fixed as well.


Reply to this email directly or view it on GitHubhttps://github.com/zeromq/czmq/pull/133#issuecomment-16050580
.

Contributor

pijyoi commented Apr 8, 2013

It's the reverse. dllexport when compiling a DLL in order to export those
symbols. dllimport when using the DLL. The original code was correct.
Please revert.
On Apr 8, 2013 9:41 PM, "TTimo" notifications@github.com wrote:

I think it would be fine, if you are using the DLL version of czmq, you
should define DLL_EXPORT, and your program will correctly look for the
symbols in a DLL.
The __declspec(dllimport) would be needed if you are compiling czmq as a
DLL and want to grab symbols from another DLL? For instance a DLL version
of libzmq?

One thing that isn't clear, is if DLL configuration for czmq should mean
libzmq as a DLL + czmq as a DLL, or just one DLL that is libzmq and czmq
(I'd rather do the second option)

On Mon, Apr 8, 2013 at 8:37 AM, AJ Lewis notifications@github.com
wrote:

It's still not quite right...if DLL_EXPORT is defined, you get
__declspec(dllexport), not __declspec(dllimport) and the dllimport is
required for linking with the DLL version of czmq.

I've got another couple changes for the czmq.sln and project files
anyway,
so I'll see if I can get this fixed as well.


Reply to this email directly or view it on GitHub<
https://github.com/zeromq/czmq/pull/133#issuecomment-16050580>
.


Reply to this email directly or view it on GitHubhttps://github.com/zeromq/czmq/pull/133#issuecomment-16050792
.

Contributor

vortechs2000 commented Apr 8, 2013

I've always had to use __declspec(dllimport) for the code using the library, but only for the header of the DLL.

#   if defined (_WINDLL)
#       if defined LIBNAME_EXPORTS
#        define LIBNAME_EXPORT __declspec(dllexport)
#       else
#        define LIBNAME_EXPORT __declspec(dllimport)
#       endif
#  else
#   define LIB_EXPORT extern
#  endif
LIBNAME_EXPORTS void myfunc(void);

To your second point, I'm fairly certain we can't easily encapsulate the libzmq DLL in the czmq DLL, and I'd rather not see that done. But then, I'm used to SONAME conventions in *NIX and windows doesn't seem to behave well WRT library versioning.

Contributor

vortechs2000 commented Apr 8, 2013

pijyoi, the problem is there isn't a way to shut off __descspec(dllimport) for static linking. There needs to be something like the above quoted code for it to work properly. I'll send a patch shortly.

Contributor

TTimo commented Apr 8, 2013

I'd rather carry one DLL around rather than two. I don't use dynamic
linking atm.

You are right my change broke the dynamic linking, but it fixed the static
linking. The correct solution is somewhere in the middle. If AJ has a
solution incoming I'd rather leave the checkin in?

On Mon, Apr 8, 2013 at 8:47 AM, AJ Lewis notifications@github.com wrote:

I've always had to use __declspec(dllimport) for the code using the
library, but only for the header of the DLL.
if defined (_WINDLL) if defined LIBNAME_EXPORTS define LIBNAME_EXPORT
__declspec(dllexport) else define LIBNAME_EXPORT __declspec(dllimport)
endif else define LIB_EXPORT extern endif

LIBNAME_EXPORTS void myfunc(void);

To your second point, I'm fairly certain we can't easily encapsulate the
libzmq DLL in the czmq DLL, and I'd rather not see that done. But then,
I'm used to SONAME conventions in *NIX and windows doesn't seem to behave
well WRT library versioning.


Reply to this email directly or view it on GitHubhttps://github.com/zeromq/czmq/pull/133#issuecomment-16051131
.

Contributor

pijyoi commented Apr 8, 2013

Someone patched zmq.h not too long ago to do this. Define ZMQ_STATIC to
compile statically. Define DLL_EXPORT to compile as DLL. Define nothing to
use the DLL.
On Apr 8, 2013 9:49 PM, "AJ Lewis" notifications@github.com wrote:

pijyoi, the problem is there isn't a way to shut off __descspec(dllimport)
for static linking. There needs to be something like the above quoted code
for it to work properly. I'll send a patch shortly.


Reply to this email directly or view it on GitHubhttps://github.com/zeromq/czmq/pull/133#issuecomment-16051228
.

Contributor

vortechs2000 commented Apr 8, 2013

Personally, I think checking for the _WINDLL define (which is setup when building as a DLL) is more elegant, but will that cause problems for people using MinGW or other non-Visual Studio setups?

Contributor

vortechs2000 commented Apr 8, 2013

Pull request is sent with a fix (#134). See if that works for y'all.

hintjens added a commit that referenced this pull request Apr 8, 2013

Merge pull request #134 from vortechs2000/fix_windows_dll
Fix windows dll builds - Fixes issue #132, and issue discussed in issue/pull request #133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment