Hi!
First thanks for reviewing the patch!
Howard Chu mailto:openldap-its@OpenLDAP.org wrote:
I have not tested this patch, but it does not appear to be correct.
In ldap_cdefs.h, you should not be testing the PIC macro since that controls whether the current source file is being built as position-independent. The macro you actually need to test is LDAP_LIBS_DYNAMIC, just as the regular WIN32 case tests. Note that the current macros work fine for both MSVC and MinGW/gcc; I don't see why you've chosen to change the LBER_F definitions for your wgcc.
Uh.. I'd have to look into that again ;o) Still this works fine since some time now, and the libraries are under heavy usage at our company...
Likewise, your hack to portable.hin should not be necessary, as all of the places that use sockets or timevals already include the appropriate header files.
Again, i'd have to look at this again...
The patch to ure.h seems unnecessary, since the file exists at both locations (the original, and via symlink).
Smylinks are not supported when windows building on interix ;o// This caused some troubles as far as i remember.
Finally, unless you're using a flag to force all symbols to be exported, I think omitting slapd.exp in the slapd/Makefile.in is a mistake.
WGCC automatically exports/imports correctly.
Really, the comments in ldap_cdefs.h are pretty explicit about why each piece of the current Windows support is as it is. Your patch seems to preclude some of the combinations of DLL/static linking that we went to great pains to support.
WGCC claims to support (with this patch) *all* possible static/shared combinations (without pain ;o)). WGCC generates code to support static libraries, even if it was compiled like a shared library (with __declspec(dllimport)'s, etc.), so most of the painfull stuff is not needed. On the other hand to support *all* configurations, one needs to allway dllimport, so both shared and static work ...
For the include/Makefile.in it would probably have made more sense to define a WINPATH macro to either "cygpath -w" or "unixpath2win" and use that, rather than repeating all of the SED invocations.
Yes, it would have been better, but it's a long time since i wrote the patch, and now i'd propably do more things differently ;o)
These days I wonder whether it wouldn't be smarter to just remove all of the special casing for braindead backslash directory separators. Since Windows now supports forward slashes everywhere, it would make life easier to just use forward slashes consistently throughout.
Would be cool, but still path handling functions from windows return paths with backslashes, so those would need to be handled too..
Cheers, Markus