Better 32bit support #63
Conversation
- add 32bit indicative to binding library name, now called esdsnbnd32.dll for x86 builds.
- all local SQLWCHAR buffers of ESODBC_MAX_IDENTIFIER_LEN count or larger are now allocated, due to the relatively large value of the define (SHRT_MAX).
- libcurl expects a 64bit value with _LARGE attribute, which is not supplied in x86 builds.
- on signed/unsigned comparisons.
driver/connect.c
Outdated
| @@ -492,9 +492,21 @@ static BOOL dbc_curl_prepare(esodbc_dbc_st *dbc, SQLULEN tout, | |||
| } | |||
| } | |||
|
|
|||
| /* curl will always expect a 64bit value (curl_off_t), while size_t | |||
| * remains a 32bit type of 32bit platforms (where a long is needed). */ | |||
| #if defined(_WIN64) || defined (WIN64) | |||
There was a problem hiding this comment.
I'm not convinced this change was necessary. If size_t is 32 bit unsigned and curl_off_t is 64 bit signed then why didn't the original line work?
There was a problem hiding this comment.
CURLOPT_POSTFIELDSIZE_LARGE attribute will expect a curl_off_t type, which is always 64 bits (on a x86 as well), while CURLOPT_POSTFIELDSIZE will expect a long.
curl_easy_setopt() is a varadic function and will interpret the passed value depending on the attribute. The compiler won't do a type conversion here. (And this is probably what's missing from the comment to make it clearer.)
Which means that on x86, before the fix, the call would pass 4 bytes (a size_t), but curl read 8 (a curl_off_t).
There was a problem hiding this comment.
Could the need for the #if section be avoided with a local variable? That makes the size correct when the variadic function is called. But the compiler can optimize it in the case where sizeof(size_t) == sizeof(curl_off_t).
curl_off_t curl_len = u8body->cnt;
dbc->curl_err = curl_easy_setopt(dbc->curl, CURLOPT_POSTFIELDSIZE_LARGE,
curl_len);
This is one extra line rather than many, and will work on Linux without any further changes.
There was a problem hiding this comment.
Ah, right!
I initially wanted to avoid the curl_off_t usage on 32 bits, mostly due to libcurl's recommendation to use it if need to go over the 32bit long limit, but I think there are no measurable down-sides if using it.
Anyways, thanks, that's a better solution.
driver/catalogue.c
Outdated
| SQLRETURN ret; | ||
| /* b/c declaring an array with a const doesn't work with MSVC's compiler */ | ||
| enum wbuf_len { wbuf_len = sizeof(SQL_TABLES) | ||
| SQLRETURN ret = SQL_ERROR;; |
There was a problem hiding this comment.
Thanks, addressed.
- also remove a superfluous ';'.
* append 32 suffix to DSN binding library - add 32bit indicative to binding library name, now called esdsnbnd32.dll for x86 builds. * allocate large objects in catalog funcs on heap - all local SQLWCHAR buffers of ESODBC_MAX_IDENTIFIER_LEN count or larger are now allocated, due to the relatively large value of the define (SHRT_MAX). * b/f: only use _LARGE body size option on x64 - libcurl expects a 64bit value with _LARGE attribute, which is not supplied in x86 builds. * fix: 32 bit compilation warnings - on signed/unsigned comparisons. * fix: simpler post size setting for libcurl - also remove a superfluous ';'. (cherry picked from commit 91aadb5)
This PR prepares the project for an x86 build:
32suffix in their naming.