FreeTDS Development Group

[Richard Hughes <cyreve AT gmail.com>]

On 2015-03-19 at 11:12, Frediano Ziglio wrote:
> 2015-03-17 22:40 GMT+00:00 Richard Hughes <cyreve AT gmail.com>:
>> I'm beginning to consider my patch nearly 'done', in that it works
>> more-or-less reliably as a standalone piece of functionality (and
>> includes at least the functionality that I need for my application). I
>> expect one more version with more tests (as mentioned above) and that
>> might cause a little surgical bug fixing but no massive overhauls.
>>
>> If you could do a full review of this version and tell me where I can
>> tidy it up, I'd appreciate it. I'm not too optimistic about being able
>> to de-duplicate this bit of functionality because, while the
>> high-level look is the same as dblib, just about every detail has had
>> some amount of tweaking. When file I/O comes in then that will no
>> longer be true, but currently it's a tricky problem.
>
> Looks good. It's getting more into shape. Do you remember from which
> changeset you start with your change? Do you have a git branch
> somewhere?

My git-fu is poor, I'm ashamed to admit. The clone I took was
2844e0df4d7c7cbccbd60dcf02e371135b42deec (the top line in the output
of git log), but I'm still using git like a non-distributed VCS and
keeping everything on my local disk. I really need to fix my lack of
knowledge one day.

> About the linking stuff I have some idea. One is define bcp functions
> with a macro that translate the header, something like
>
> #define bcp_done(dbc) tdsodbc_bcp_done(tdsodbc_bcp_driver_hdbc(dbc))
> static inline HDBC
> tdsodbc_bcp_driver_dbc(HDBC dbc)
> {
>    SQLSMALLINT handle_len;
>    if (SQLGetInfo(dbc, SQL_DRIVER_HDBC, &dbc, sizeof(dbc),
> &handle_len) != SQL_SUCCESS)
>       return NULL;
>    return dbc;
> }
>
> With the function defined in the header to so the SQLGetInfo call is
> made from the application

Cunning. I like it.

> However I think that the way you call tdsodbc_bcp_* functions is not
> safe. In Unix there is a different concept. While on Windows you say
> (this is done implicitly by linker) I want function X from library Y
> in Unix usually you just say I want function X from global symbols.
> Now the problem is that if you link the DM (like unixODBC) and
> libtdsodbc and you call a function like SQLGetInfo are you calling
> unixODBC or FreeTDS? Well... depends from linker order,
> initializations and so on. Not safe. In Microsoft documentation (like
> https://msdn.microsoft.com/en-us/library/ms130922.aspx) is explicitly
> said that you cannot link different drivers, similar linker problem.
> The DM solve the problem as it loads libraries with LOCAL flag so
> symbols do not get into global list but you just use dlsym to manually
> get them. Not a big deal as you can use dlopen(..., LOCAL|NOLOAD) to
> get library header and dlsym to get symbols. However you have to know
> library name which could be tricky in a portable way (path change
> based on configuration flags and filename change from system to
> system). This was why I had the idea to incapsulate calls using
> SQLSetConnectAttr (or similar) as DM will convert the handle and call
> proper driver function no matter what. You could also extend
> SQLGetInfo to return information you need directly from driver (like
> handle, function pointers and so on).

I realised while thinking about this that Windows also has the same
linker order problem, despite its different runtime behaviour, because
at (static) link time the list of symbols being resolved is still put
in to one global bucket. I guess it usually gets away with it because
odbc32.lib is on the list of 'magic' libraries which always get linked
in (whether you asked for it or not) and hence always gets searched
first.

My current favorite approach is a combination of your inline function
and SQLSetConnectAttr tricks, which ends up as basically identical to
your bcp_collen, except with "static __inline__" bolted on the front
(and an extra struct member for passing the return value back, because
bcp doesn't use ODBC return values):

>>>>> typedef struct  {
>>>>>   DBINT cbData;
>>>>>   INT idxServerCol;
>>>>> } bcp_collen_params;
>>>>>
>>>>> RETCODE bcp_collen (
>>>>>         HDBC hdbc,
>>>>>         DBINT cbData,
>>>>>         INT idxServerCol)
>>>>> {
>>>>>    bcp_collen_params params = { cbData, idxServerCol };
>>>>>     return SQLSetConnectAttr(hdbc, SQL_SS_FUNC_BCP_COLLEN, &params,
>>>>> sizeof(params));
>>>>> }

This has the advantage that there's no need to mess about with linker
command lines or static libraries, and the only disadvantage that I
can think of is microscopically bulkier application binaries.

Richard.