[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freetds
Subject:    Re: [freetds] 0.64rc2 Debian prerelease packages available
From:       Frediano Ziglio <freddyz77 () tin ! it>
Date:       2006-04-02 7:23:40
Message-ID: 1143962620.2594.25.camel () freddy
[Download RAW message or body]

Il giorno sab, 01/04/2006 alle 12.46 -0800, Steve Langasek ha scritto:
> On Sat, Apr 01, 2006 at 10:27:35AM +0200, Frediano Ziglio wrote:
> > Mmmm... from MS documentation it seems that row numbers, data length
> > pointer and offsets became 64-bit on 64-bit platforms so from sqltypes.h
> 
> > #if (SIZEOF_LONG == 8)
> > #ifndef BUILD_REAL_64_BIT_MODE
> > typedef int             SQLINTEGER;
> > typedef unsigned int    SQLUINTEGER;
> > #define SQLLEN          SQLINTEGER
> > #define SQLULEN         SQLUINTEGER
> > #define SQLSETPOSIROW   SQLUSMALLINT
> > typedef SQLULEN         SQLROWCOUNT;
> > typedef SQLULEN         SQLROWSETSIZE;
> > typedef SQLULEN         SQLTRANSID;
> > typedef SQLLEN          SQLROWOFFSET;
> > #else
> > typedef int             SQLINTEGER;
> > typedef unsigned int    SQLUINTEGER;
> > typedef long            SQLLEN;
> > typedef unsigned long   SQLULEN;
> > typedef unsigned long   SQLSETPOSIROW;
> > /*
> >  * These are not supprted on 64bit ODBC according to MS
> >  * typedef SQLULEN SQLTRANSID;
> >  */
> > typedef SQLULEN SQLROWCOUNT;
> > typedef SQLUINTEGER SQLROWSETSIZE;
> > typedef SQLLEN SQLROWOFFSET;
> > #endif
> > #else
> 
> > I don't agree SQLROWSETSIZE is 32 bit on clean 64 bit, should be
> > SQLULEN, not SQLUINTEGER
> 
> Well, that still means the current FreeTDS isn't compatible with UnixODBC
> 2.2.11 when building in 64-bit mode, right?  The typedefs of SQLROWOFFSET
> and SQLROWSETSIZE in UnixODBC may not match the MS 64-bit spec, but it looks
> like SQLExtendedFetch() is still supposed to *use* SQLROWOFFSET and
> SQLROWSETSIZE, so there's a (minor) bug in FreeTDS wrt the spec. :)
> 

I would like to see just yes but the problem is a bit complicated...
let's take an example with SQLRowCount and cursors. If a driver do not
support all cursors types unixODBC wrap it with a cursor type library.
This means that every driver function get captured by this library in
order to change semantic adding some cursor feature (client cursors).
SQLRowCount in DriverManager/SQLRowCount.c is declared

SQLRETURN SQLRowCount( SQLHSTMT statement_handle,
       SQLLEN *rowcount )

note that SQLLEN is 64-bit for you. This function calls CLRowCount in
cur/SQLRowCount.c which is declared as

SQLRETURN CLRowCount( SQLHSTMT statement_handle,
       SQLINTEGER *rowcount )

note that SQLULEN* rowcount get silently converted to SQLINTEGER* and
also deferenced in CLRowCount

SQLRETURN CLRowCount( SQLHSTMT statement_handle,
       SQLINTEGER *rowcount )
{
    CLHSTMT cl_statement = (CLHSTMT) statement_handle;

    if ( cl_statement -> first_fetch_done )
    {
        if ( rowcount )
        {
            *rowcount = cl_statement -> rowset_count;
        }
        return SQL_SUCCESS;
    }
    else
    {
        return DEF_SQLROWCOUNT( cl_statement -> cl_connection,
            cl_statement -> driver_stmt,
            rowcount );
    }
}

Assuming that the original rowcount (in the application) assume
0x1234567812345678 calling and that we had 4 rows SQLRowCount can return
0x1234567800000004 which is a bit different from 0x0000000000000004.
So at least defining BUILD_REAL_64_BIT_MODE break all cursors library...

> > Also note that pirow argument of SQLParamOptions is used for a pointer
> > to row number so even this should be 64 bit so declaration in sqlext.h
> > should be 
> 
> > SQLRETURN SQL_API SQLParamOptions(
> >     SQLHSTMT           hstmt,
> >     SQLULEN            crow,
> >     SQLULEN            *pirow);
> 
> That explains why the pirow argument might need to be SQLULEN*, but it
> doesn't explain why crow would?  It's only the difference in the size of
> crow that breaks the ABI, AFAICS.
> 

No, it doesn't explain it but ABI is changed even changing SQLUINTEGER*
to SQLULEN* so is better to fix both. Yes, for each pointer we could
have a flag that tell this is 32-bit or this is 64-bit but is a dirty
way, don't you think so ??

> > So I think that unixODBC should be fixed instead of FreeTDS (unixODBC
> > 2.2.12 already contains this change). Changing only FreeTDS declaration
> > fix compile problems but lead to data corruptions or possible cores so I
> > think is better to fix the problem in a definitive way...
> 
> > You can get unixODBC 2.2.12 (which is not a release!!) at
> > ftp://ftp.easysoft.com/pub/unixODBC/
> 
> Hrm.  Well, I'm not happy with the idea of freetds 0.64's ODBC driver only
> being compatible with 64-bit systems when using an unreleased version of
> unixodbc; but if this ABI change has already been made in unixodbc 2.2.12, I
> guess we'd be better off making this same ABI change in Debian sooner rather
> than later.
> 

I think there is some works to do :) I hope we have some time before
next debian release. I think that unixODBC is the peace that require
more works. The first thing I would change are defines (drivermanager.h)
like

#define SQLGETDIAGFIELD(con,typ,han,rn,di,dip,bl,slp)\
                                    (con->functions[42].func)\
                                        (typ,han,rn,di,dip,bl,slp)

too unsafe...
Next is cursor library defines.

freddy77


_______________________________________________
FreeTDS mailing list
FreeTDS@lists.ibiblio.org
http://lists.ibiblio.org/mailman/listinfo/freetds
[prev in list] [next in list] [prev in thread] [next in thread]