Menu
▾
▴
1 Attachments
#1207 libcurl compilation on Windows with unicode support enabled.
libcurl ldap.c fails to compile properly on Windows when Unicode support is enabled. See attached patch; basically WIN32 API functions should be suffixed by 'A' to use 'char' implementations rather than 'wchar' implementations.
Discussion
Forgot to mention version; I downloaded curl 7.29.0.
See the discussion beginning http://curl.haxx.se/mail/lib-2011-10/0072.html and http://curl.haxx.se/mail/lib-2011-11/0010.html
Please clarify why we would want this patch and the advantages this change has to us or to users of libcurl.
This patch does not bring any changes to users; it only helps developers who happens to build libcurl with unicode support enabled.
The patch replaces calls to ldap_init() win32 macro (which is equal to function ldap_initA() or ldap_initW() depending on the compilation options) by calls to ldap_initA().
This A/W function mecanism is well explained on this page : http://msdn.microsoft.com/en-us/library/windows/desktop/dd317766(v=vs.85).aspx
Feel free to include it or reject it.
My 2 cents:
You could get advantages for users if unicode should be supported for username and password. It looks like a similar issue like there was with NTLM proxy authentication some while ago.
The solution there was to use the ANSI-Version when UNICODE was not defined and use the Unicode-Version of the Windows-function when UNICODE was defined. In the Unicode case you treat username and other strings as UTF8 encoded and these strings get translated into wchar_t* for the call of the Windows-function. (curl_multibyte.h provides these functions)
Can someone clarify if this patch has any downside? It seems so very unconditional in that it switches other API calls in many places. For example, which windows versions does it work on and does it impose any requirements on the compiler/build environment?
I will close this report unless I see a significant support and will to get it merged.
See commit https://github.com/bagder/curl/commit/46480bb9a1569eaf156012f33e3e7e8c3de18f87, file getenv.c, where 'ExpandEnvironmentString' was replaced by 'ExpandEnvironmentStringA'.
Sorry, I don't understand what you're saying with that link. Please clarify what the benefits of your suggested patch is, and the possible downsides it has. As I already asked for above. Just handing me a link to our own source code does not answer these questions.
In fact, it seems that ldap support may be enabled on Linux using './configuration --enable-ldap' and that the ldap library to use may also be chosen on the configure command line.
In that case, I can not test all configuration options, so the patch should be dropped.
No convincing information provided, closing. Will re-open if someone can explain this better to me.