From 9020ca9a1ed70b7e19c0079693969f66da1b3cdf Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 15:35:02 -0400 Subject: [PATCH 1/8] Attempt to make the ODBC driver thread-safe I'm not sure if this is a complete solution - some of the global state has been moved to thread-local storage. So passing an ODBC handle across threads may have unexpected results. But at least it's not global state. Each ODBC handle now has its own iconv_t object. See #23 --- include/mdbfakeglib.h | 2 +- src/libmdb/fakeglib.c | 3 ++- src/odbc/connectparams.c | 25 ++++++------------------ src/odbc/mdbodbc.h | 4 ++++ src/odbc/odbc.c | 42 +++++++++++++++------------------------- 5 files changed, 29 insertions(+), 47 deletions(-) diff --git a/include/mdbfakeglib.h b/include/mdbfakeglib.h index 59b9a2d..0a34263 100644 --- a/include/mdbfakeglib.h +++ b/include/mdbfakeglib.h @@ -121,7 +121,7 @@ typedef struct GOptionContext { /* string functions */ void *g_memdup(const void *src, size_t len); int g_str_equal(const void *str1, const void *str2); -char **g_strsplit(const char *haystack, const char *needle, int something); +char **g_strsplit(const char *haystack, const char *needle, int max_tokens); void g_strfreev(char **dir); char *g_strconcat(const char *first, ...); char *g_strdup(const char *src); diff --git a/src/libmdb/fakeglib.c b/src/libmdb/fakeglib.c index 7195e57..6de90f1 100644 --- a/src/libmdb/fakeglib.c +++ b/src/libmdb/fakeglib.c @@ -22,7 +22,8 @@ int g_str_equal(const void *str1, const void *str2) { return strcmp(str1, str2) == 0; } -char **g_strsplit(const char *haystack, const char *needle, int something) { +// max_tokens not yet implemented +char **g_strsplit(const char *haystack, const char *needle, int max_tokens) { char **ret = NULL; char *found = NULL; size_t components = 2; // last component + terminating NULL diff --git a/src/odbc/connectparams.c b/src/odbc/connectparams.c index 259aeb7..922f1b6 100644 --- a/src/odbc/connectparams.c +++ b/src/odbc/connectparams.c @@ -43,9 +43,6 @@ #define FILENAME_MAX 512 #endif -#define max_line 256 -static char line[max_line]; - static guint HashFunction (gconstpointer key); static void visit (gpointer key, gpointer value, gpointer user_data); @@ -224,17 +221,12 @@ gchar* ExtractDSN (ConnectParams* params, const gchar* connectString) q++; while (isspace(*q)) q++; - /* - * Copy the DSN value to a buffer - */ - s = line; - while (*q && *q != ';') - *s++ = *q++; - *s = '\0'; /* * Save it as a string in the params object */ - params->dsnName = g_string_assign (params->dsnName, line); + char **components = g_strsplit(q, ";", 2); + params->dsnName = g_string_assign(params->dsnName, components[0]); + g_strfreev(components); return params->dsnName->str; } @@ -261,17 +253,12 @@ gchar* ExtractDBQ (ConnectParams* params, const gchar* connectString) q++; while (isspace(*q)) q++; - /* - * Copy the DSN value to a buffer - */ - s = line; - while (*q && *q != ';') - *s++ = *q++; - *s = '\0'; /* * Save it as a string in the params object */ - params->dsnName = g_string_assign (params->dsnName, line); + char **components = g_strsplit(q, ";", 2); + params->dsnName = g_string_assign(params->dsnName, components[0]); + g_strfreev(components); return params->dsnName->str; } diff --git a/src/odbc/mdbodbc.h b/src/odbc/mdbodbc.h index ce4409b..eec4c39 100644 --- a/src/odbc/mdbodbc.h +++ b/src/odbc/mdbodbc.h @@ -43,6 +43,10 @@ struct _hdbc { MdbSQL *sqlconn; ConnectParams* params; GPtrArray *statements; +#ifdef ENABLE_ODBC_W + iconv_t iconv_in; + iconv_t iconv_out; +#endif }; struct _hstmt { MdbSQL *sql; diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index 3359d11..bb5b3de 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -31,10 +31,6 @@ //#define TRACE(x) fprintf(stderr,"Function %s\n", x); #define TRACE(x) -#ifdef ENABLE_ODBC_W -static iconv_t iconv_in,iconv_out; -#endif //ENABLE_ODBC_W - static SQLSMALLINT _odbc_get_client_type(MdbColumn *col); static const char * _odbc_get_client_type_name(MdbColumn *col); static int _odbc_fix_literals(struct _hstmt *stmt); @@ -50,8 +46,8 @@ static void unbind_columns (struct _hstmt*); #define MIN(a,b) (a>b ? b : a) #endif #define _MAX_ERROR_LEN 255 -static char lastError[_MAX_ERROR_LEN+1]; -static char sqlState[6]; +static __thread char lastError[_MAX_ERROR_LEN+1]; +static __thread char sqlState[6]; typedef struct { SQLCHAR *type_name; @@ -94,11 +90,9 @@ TypeInfo type_info[] = { #define MAX_TYPE_INFO 11 #ifdef ENABLE_ODBC_W -void my_fini(void); - -MDB_CONSTRUCTOR(my_init) +static void _init_iconv(struct _hdbc* dbc) { { - TRACE("my_init"); + TRACE("_init_iconv"); int endian = 1; const char* wcharset; if (sizeof(SQLWCHAR) == 2) @@ -113,26 +107,16 @@ MDB_CONSTRUCTOR(my_init) wcharset = "UCS-4BE"; else fprintf(stderr, "Unsupported SQLWCHAR width %zd\n", sizeof(SQLWCHAR)); -//fprintf(stderr,"charset %s\n", wcharset); - //fprintf(stderr, "SQLWCHAR width %d\n", sizeof(SQLWCHAR)); -/* -#if __SIZEOF_WCHAR_T__ == 4 || __WCHAR_MAX__ > 0x10000 - #define WCHAR_CHARSET "UCS-4LE" -#else - #define WCHAR_CHARSET "UCS-2LE" -#endif -*/ - iconv_out = iconv_open(wcharset, "UTF-8"); - iconv_in = iconv_open("UTF-8", wcharset); - atexit(my_fini); + dbc->iconv_out = iconv_open(wcharset, "UTF-8"); + dbc->iconv_in = iconv_open("UTF-8", wcharset); } -void my_fini() +static void _free_iconv(struct _hdbc *dbc) { - TRACE("my_fini"); - if(iconv_out != (iconv_t)-1)iconv_close(iconv_out); - if(iconv_in != (iconv_t)-1)iconv_close(iconv_in); + TRACE("_free_iconv"); + if(dbc->iconv_out != (iconv_t)-1)iconv_close(dbc->iconv_out); + if(dbc->iconv_in != (iconv_t)-1)iconv_close(dbc->iconv_in); } static int unicode2ascii(char *_in, size_t *_lin, char *_out, size_t *_lout){ @@ -500,6 +484,9 @@ struct _hdbc* dbc; dbc->params = NewConnectParams (); dbc->statements = g_ptr_array_new(); dbc->sqlconn = mdb_sql_init(); +#ifdef ENABLE_ODBC_W + _init_iconv(dbc); +#endif *phdbc=dbc; return SQL_SUCCESS; @@ -1115,6 +1102,9 @@ SQLRETURN SQL_API SQLFreeConnect( FreeConnectParams(dbc->params); g_ptr_array_free(dbc->statements, TRUE); mdb_sql_exit(dbc->sqlconn); +#ifdef ENABLE_ODBC_W + _free_iconv(dbc); +#endif g_free(dbc); return SQL_SUCCESS; From 684fa557817218139bb1bbe5cfc13bdb9f1d2dbe Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 15:50:57 -0400 Subject: [PATCH 2/8] Fix unused variable warning --- src/odbc/connectparams.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/odbc/connectparams.c b/src/odbc/connectparams.c index 922f1b6..bd39613 100644 --- a/src/odbc/connectparams.c +++ b/src/odbc/connectparams.c @@ -201,7 +201,7 @@ void DumpParams (ConnectParams* params, FILE* output) gchar* ExtractDSN (ConnectParams* params, const gchar* connectString) { - char *p, *q, *s; + char *p, *q; if (!params) return NULL; @@ -233,7 +233,7 @@ gchar* ExtractDSN (ConnectParams* params, const gchar* connectString) gchar* ExtractDBQ (ConnectParams* params, const gchar* connectString) { - char *p, *q, *s; + char *p, *q; if (!params) return NULL; From 3d63f16fc0092dc928082cb026c238e4cf4494b2 Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 16:16:25 -0400 Subject: [PATCH 3/8] Fix errors It helps to enable ODBC when testing ODBC :-P --- src/odbc/odbc.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index bb5b3de..ae7d6f0 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -90,7 +90,7 @@ TypeInfo type_info[] = { #define MAX_TYPE_INFO 11 #ifdef ENABLE_ODBC_W -static void _init_iconv(struct _hdbc* dbc) { +static void _init_iconv(struct _hdbc* dbc) { TRACE("_init_iconv"); int endian = 1; @@ -119,21 +119,21 @@ static void _free_iconv(struct _hdbc *dbc) if(dbc->iconv_in != (iconv_t)-1)iconv_close(dbc->iconv_in); } -static int unicode2ascii(char *_in, size_t *_lin, char *_out, size_t *_lout){ +static int unicode2ascii(struct _hdbc* dbc, char *_in, size_t *_lin, char *_out, size_t *_lout){ char *in=_in, *out=_out; size_t lin=*_lin, lout=*_lout; - int ret = iconv(iconv_in, &in, &lin, &out, &lout); + int ret = iconv(dbc->iconv_in, &in, &lin, &out, &lout); *_lin -= lin; *_lout -= lout; return ret; } -static int ascii2unicode(char *_in, size_t *_lin, char *_out, size_t *_lout){ +static int ascii2unicode(struct _hdbc* dbc, char *_in, size_t *_lin, char *_out, size_t *_lout){ //fprintf(stderr,"ascii2unicode %08x %08x %08x %08x\n",_in,_lin,_out,_lout); char *in=_in, *out=_out; size_t lin=*_lin, lout=*_lout; //fprintf(stderr,"ascii2unicode %zd %zd\n",lin,lout); - int ret = iconv(iconv_out, &in, &lin, &out, &lout); + int ret = iconv(dbc->iconv_out, &in, &lin, &out, &lout); *_lin -= lin; *_lout -= lout; return ret; @@ -227,7 +227,7 @@ SQLRETURN SQL_API SQLDriverConnectW( size_t l = cbConnStrIn*sizeof(SQLWCHAR), z = (cbConnStrIn+1)*3; SQLCHAR *tmp = malloc(z); SQLRETURN ret; - unicode2ascii((char*)szConnStrIn, &l, (char*)tmp, &z); + unicode2ascii((struct _hdbc *)hdbc, (char*)szConnStrIn, &l, (char*)tmp, &z); tmp[z] = 0; ret = SQLDriverConnect(hdbc,hwnd,tmp,SQL_NTS,NULL,0,pcbConnStrOut,fDriverCompletion); free(tmp); @@ -651,9 +651,9 @@ SQLRETURN SQL_API SQLConnectW( size_t l2=cbUID*4,z2=cbUID*2; size_t l3=cbAuthStr*4,z3=cbAuthStr*2; SQLRETURN ret; - unicode2ascii((char*)szDSN, &z1, (char*)tmp1, &l1); - unicode2ascii((char*)szUID, &z2, (char*)tmp2, &l2); - unicode2ascii((char*)szAuthStr, &z3, (char*)tmp3, &l3); + unicode2ascii((struct _hdbc *)hdbc, (char*)szDSN, &z1, (char*)tmp1, &l1); + unicode2ascii((struct _hdbc *)hdbc, (char*)szUID, &z2, (char*)tmp2, &l2); + unicode2ascii((struct _hdbc *)hdbc, (char*)szAuthStr, &z3, (char*)tmp3, &l3); ret = SQLConnect(hdbc, tmp1, l1, tmp2, l2, tmp3, l3); free(tmp1),free(tmp2),free(tmp3); return ret; @@ -747,7 +747,7 @@ SQLRETURN SQL_API SQLDescribeColW( SQLCHAR *tmp=calloc(cbColNameMax*4,1); size_t l=cbColNameMax*4; SQLRETURN ret = SQLDescribeCol(hstmt, icol, tmp, cbColNameMax*4, (SQLSMALLINT*)&l, pfSqlType, pcbColDef, pibScale, pfNullable); - ascii2unicode((char*)tmp, &l, (char*)szColName, (size_t*)pcbColName); + ascii2unicode(((struct _hstmt*)hstmt)->hdbc, (char*)tmp, &l, (char*)szColName, (size_t*)pcbColName); *pcbColName/=sizeof(SQLWCHAR); free(tmp); return ret; @@ -877,7 +877,7 @@ SQLRETURN SQL_API SQLColAttributesW( SQLCHAR *tmp=calloc(cbDescMax*4,1); size_t l=cbDescMax*4; SQLRETURN ret=SQLColAttributes(hstmt,icol,fDescType,tmp,cbDescMax*4,(SQLSMALLINT*)&l,pfDesc); - ascii2unicode((char*)tmp, &l, (char*)rgbDesc, (size_t*)pcbDesc); + ascii2unicode(((struct _hstmt *)hstmt)->hdbc, (char*)tmp, &l, (char*)rgbDesc, (size_t*)pcbDesc); *pcbDesc/=sizeof(SQLWCHAR); free(tmp); return ret; @@ -952,9 +952,9 @@ SQLRETURN SQL_API SQLErrorW( result = SQLError(henv, hdbc, hstmt, szSqlState8, pfNativeError, szErrorMsg8, 3*cbErrorMsgMax+1, &pcbErrorMsg8); if (result == SQL_SUCCESS) { size_t l=6, z=6*sizeof(SQLWCHAR); - ascii2unicode((char*)szSqlState8, &l, (char*)szSqlState, &z); + ascii2unicode((struct _hdbc *)hdbc, (char*)szSqlState8, &l, (char*)szSqlState, &z); l = cbErrorMsgMax; - ascii2unicode((char*)szErrorMsg8, (size_t*)&pcbErrorMsg8, (char*)szErrorMsg, &l); + ascii2unicode((struct _hdbc *)hdbc, (char*)szErrorMsg8, (size_t*)&pcbErrorMsg8, (char*)szErrorMsg, &l); if (pcbErrorMsg) *pcbErrorMsg = l; } @@ -1004,7 +1004,7 @@ SQLRETURN SQL_API SQLExecDirectW( SQLCHAR *tmp=calloc(cbSqlStr*4,1); size_t l=cbSqlStr*4,z=cbSqlStr*2; SQLRETURN ret; - unicode2ascii((char*)szSqlStr, &z, (char*)tmp, &l); + unicode2ascii(((struct _hstmt *)hstmt)->hdbc, (char*)szSqlStr, &z, (char*)tmp, &l); ret = SQLExecDirect(hstmt, tmp, l); TRACE("SQLExecDirectW end"); free(tmp); @@ -1386,7 +1386,7 @@ SQLRETURN SQL_API SQLColumnsW( SQLCHAR *tmp=calloc(cbTableName*4,1); size_t l=cbTableName*4,z=cbTableName*2; SQLRETURN ret; - unicode2ascii((char*)szTableName, &z, (char*)tmp, &l); + unicode2ascii(((struct _hstmt* )hstmt)->hdbc, (char*)szTableName, &z, (char*)tmp, &l); ret = SQLColumns(hstmt, NULL, 0, NULL, 0, tmp, l, NULL, 0); free(tmp); return ret; @@ -1686,7 +1686,7 @@ SQLRETURN SQL_API SQLGetDataW( SQLCHAR *tmp=calloc(cbValueMax*4,1); size_t l=cbValueMax*4; SQLRETURN ret = SQLGetData(hstmt, icol, fCType, tmp, cbValueMax*4, (SQLLEN*)&l); - ascii2unicode((char*)tmp, &l, (char*)rgbValue, (size_t*)pcbValue); + ascii2unicode(((struct _hstmt *)hstmt)->hdbc, (char*)tmp, &l, (char*)rgbValue, (size_t*)pcbValue); *pcbValue/=sizeof(SQLWCHAR); free(tmp); return ret; @@ -1944,7 +1944,7 @@ SQLRETURN SQL_API SQLGetInfoW( size_t l=cbInfoValueMax*4; SQLRETURN ret = SQLGetInfo(hdbc, fInfoType, tmp, cbInfoValueMax*4,(SQLSMALLINT*)&l); size_t pcb=cbInfoValueMax; - ascii2unicode((char*)tmp, &l, (char*)rgbInfoValue, &pcb); + ascii2unicode((struct _hdbc *)hdbc, (char*)tmp, &l, (char*)rgbInfoValue, &pcb); pcb/=sizeof(SQLWCHAR); if(pcbInfoValue)*pcbInfoValue=pcb; free(tmp); From e67c8e6d52f1a67b069e36a20950b87fbd278067 Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 16:32:13 -0400 Subject: [PATCH 4/8] Remove obsolete comment about thread safety --- src/odbc/odbc.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index ae7d6f0..05eab37 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -147,10 +147,6 @@ static int sqlwlen(SQLWCHAR *p){ } #endif // ENABLE_ODBC_W -/* The SQL engine is presently non-reenterrant and non-thread safe. - See SQLExecute for details. -*/ - static void LogError (const char* format, ...) { /* From a84c07980f476e0035f0b4011bd0c41de0eeba7c Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 17:03:06 -0400 Subject: [PATCH 5/8] Make lastError thread-safe --- src/odbc/mdbodbc.h | 1 + src/odbc/odbc.c | 30 ++++++++++++++---------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/odbc/mdbodbc.h b/src/odbc/mdbodbc.h index eec4c39..5216e92 100644 --- a/src/odbc/mdbodbc.h +++ b/src/odbc/mdbodbc.h @@ -43,6 +43,7 @@ struct _hdbc { MdbSQL *sqlconn; ConnectParams* params; GPtrArray *statements; + char lastError[256]; #ifdef ENABLE_ODBC_W iconv_t iconv_in; iconv_t iconv_out; diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index 05eab37..93f1c59 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -45,8 +45,6 @@ static void unbind_columns (struct _hstmt*); #ifndef MIN #define MIN(a,b) (a>b ? b : a) #endif -#define _MAX_ERROR_LEN 255 -static __thread char lastError[_MAX_ERROR_LEN+1]; static __thread char sqlState[6]; typedef struct { @@ -147,14 +145,14 @@ static int sqlwlen(SQLWCHAR *p){ } #endif // ENABLE_ODBC_W -static void LogError (const char* format, ...) +static void LogError(struct _hdbc* dbc, const char* format, ...) { /* * Someday, I might make this store more than one error. */ va_list argp; va_start(argp, format); - vsnprintf(lastError, _MAX_ERROR_LEN, format, argp); + vsnprintf(dbc->lastError, sizeof(dbc->lastError), format, argp); va_end(argp); } @@ -185,14 +183,14 @@ SQLRETURN SQL_API SQLDriverConnect( SQLRETURN ret; TRACE("SQLDriverConnect"); - strcpy (lastError, ""); + strcpy(((struct _hdbc*)hdbc)->lastError, ""); params = ((struct _hdbc*) hdbc)->params; if (ExtractDSN(params, (gchar*)szConnStrIn)) { SetConnectString (params, (gchar*)szConnStrIn); if (!(database = GetConnectParam (params, "Database"))){ - LogError ("Could not find Database parameter in '%s'", szConnStrIn); + LogError(hdbc, "Could not find Database parameter in '%s'", szConnStrIn); return SQL_ERROR; } ret = do_connect (hdbc, database); @@ -202,7 +200,7 @@ SQLRETURN SQL_API SQLDriverConnect( ret = do_connect (hdbc, database); return ret; } - LogError ("Could not find DSN nor DBQ in connect string '%s'", szConnStrIn); + LogError(hdbc, "Could not find DSN nor DBQ in connect string '%s'", szConnStrIn); return SQL_ERROR; } @@ -286,7 +284,7 @@ SQLRETURN SQL_API SQLExtendedFetch( TRACE("SQLExtendedFetch"); if (fFetchType!=SQL_FETCH_NEXT) { - LogError("Fetch type not supported in SQLExtendedFetch"); + LogError(stmt->hdbc, "Fetch type not supported in SQLExtendedFetch"); return SQL_ERROR; } if (pcrow) @@ -611,7 +609,7 @@ SQLRETURN SQL_API SQLConnect( SQLRETURN ret; TRACE("SQLConnect"); - strcpy (lastError, ""); + strcpy(((struct _hdbc*)hdbc)->lastError, ""); params = ((struct _hdbc*) hdbc)->params; @@ -619,7 +617,7 @@ SQLRETURN SQL_API SQLConnect( if (!(database = GetConnectParam (params, "Database"))) { - LogError ("Could not find Database parameter in '%s'", szDSN); + LogError(hdbc, "Could not find Database parameter in '%s'", szDSN); return SQL_ERROR; } @@ -911,17 +909,17 @@ SQLRETURN SQL_API SQLError( TRACE("SQLError"); //if(pfNativeError)fprintf(stderr,"NativeError %05d\n", *pfNativeError); - if (strlen (lastError) > 0) + if (strlen(((struct _hdbc *)hdbc)->lastError) > 0) { strcpy ((char*)szSqlState, "08001"); - strcpy ((char*)szErrorMsg, lastError); + int l = snprintf((char*)szErrorMsg, cbErrorMsgMax, "%s", ((struct _hdbc *)hdbc)->lastError); if (pcbErrorMsg) - *pcbErrorMsg = strlen (lastError); + *pcbErrorMsg = l; if (pfNativeError) *pfNativeError = 1; result = SQL_SUCCESS; - strcpy (lastError, ""); + strcpy(((struct _hdbc *)hdbc)->lastError, ""); } return result; @@ -971,7 +969,7 @@ SQLRETURN SQL_API SQLExecute(SQLHSTMT hstmt) mdb_sql_run_query(stmt->sql, stmt->query); if (mdb_sql_has_error(stmt->sql)) { - LogError("Couldn't parse SQL\n"); + LogError(stmt->hdbc, "Couldn't parse SQL\n"); mdb_sql_reset(stmt->sql); return SQL_ERROR; } else { @@ -1325,7 +1323,7 @@ SQLRETURN SQL_API SQLColumns( table = mdb_read_table(entry); if ( !table ) { - LogError ("Could not read table '%s'", szTableName); + LogError(stmt->hdbc, "Could not read table '%s'", szTableName); return SQL_ERROR; } mdb_read_columns(table); From 00b3ab9571571d83f06c64f8d7d4f83ff674b697 Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 17:22:54 -0400 Subject: [PATCH 6/8] Store statement and handle errors separately --- src/odbc/mdbodbc.h | 1 + src/odbc/odbc.c | 60 ++++++++++++++++++++++++++-------------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/odbc/mdbodbc.h b/src/odbc/mdbodbc.h index 5216e92..35c2ba4 100644 --- a/src/odbc/mdbodbc.h +++ b/src/odbc/mdbodbc.h @@ -56,6 +56,7 @@ struct _hstmt { * please make dynamic before checking in */ char query[4096]; + char lastError[256]; struct _sql_bind_info *bind_head; int rows_affected; int icol; /* SQLGetData: last column */ diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index 93f1c59..9a774dc 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -42,9 +42,6 @@ static void unbind_columns (struct _hstmt*); #define FILL_FIELD(f,v,s) mdb_fill_temp_field(f,v,s,0,0,0,0) -#ifndef MIN -#define MIN(a,b) (a>b ? b : a) -#endif static __thread char sqlState[6]; typedef struct { @@ -145,17 +142,22 @@ static int sqlwlen(SQLWCHAR *p){ } #endif // ENABLE_ODBC_W -static void LogError(struct _hdbc* dbc, const char* format, ...) +static void LogHandleError(struct _hdbc* dbc, const char* format, ...) { - /* - * Someday, I might make this store more than one error. - */ va_list argp; va_start(argp, format); vsnprintf(dbc->lastError, sizeof(dbc->lastError), format, argp); va_end(argp); } +static void LogStatementError(struct _hstmt* stmt, const char* format, ...) +{ + va_list argp; + va_start(argp, format); + vsnprintf(stmt->lastError, sizeof(stmt->lastError), format, argp); + va_end(argp); +} + static SQLRETURN do_connect ( SQLHDBC hdbc, char *database) @@ -190,7 +192,7 @@ SQLRETURN SQL_API SQLDriverConnect( if (ExtractDSN(params, (gchar*)szConnStrIn)) { SetConnectString (params, (gchar*)szConnStrIn); if (!(database = GetConnectParam (params, "Database"))){ - LogError(hdbc, "Could not find Database parameter in '%s'", szConnStrIn); + LogHandleError(hdbc, "Could not find Database parameter in '%s'", szConnStrIn); return SQL_ERROR; } ret = do_connect (hdbc, database); @@ -200,7 +202,7 @@ SQLRETURN SQL_API SQLDriverConnect( ret = do_connect (hdbc, database); return ret; } - LogError(hdbc, "Could not find DSN nor DBQ in connect string '%s'", szConnStrIn); + LogHandleError(hdbc, "Could not find DSN nor DBQ in connect string '%s'", szConnStrIn); return SQL_ERROR; } @@ -284,7 +286,7 @@ SQLRETURN SQL_API SQLExtendedFetch( TRACE("SQLExtendedFetch"); if (fFetchType!=SQL_FETCH_NEXT) { - LogError(stmt->hdbc, "Fetch type not supported in SQLExtendedFetch"); + LogStatementError(stmt, "Fetch type not supported in SQLExtendedFetch"); return SQL_ERROR; } if (pcrow) @@ -617,7 +619,7 @@ SQLRETURN SQL_API SQLConnect( if (!(database = GetConnectParam (params, "Database"))) { - LogError(hdbc, "Could not find Database parameter in '%s'", szDSN); + LogHandleError(hdbc, "Could not find Database parameter in '%s'", szDSN); return SQL_ERROR; } @@ -909,18 +911,23 @@ SQLRETURN SQL_API SQLError( TRACE("SQLError"); //if(pfNativeError)fprintf(stderr,"NativeError %05d\n", *pfNativeError); - if (strlen(((struct _hdbc *)hdbc)->lastError) > 0) - { - strcpy ((char*)szSqlState, "08001"); - int l = snprintf((char*)szErrorMsg, cbErrorMsgMax, "%s", ((struct _hdbc *)hdbc)->lastError); - if (pcbErrorMsg) - *pcbErrorMsg = l; - if (pfNativeError) - *pfNativeError = 1; + char *src = NULL; + if (hstmt) { + src = ((struct _hstmt *)hstmt)->lastError; + } else if (hdbc) { + src = ((struct _hdbc *)hdbc)->lastError; + } + if (src && src[0]) { + strcpy ((char*)szSqlState, "08001"); + int l = snprintf((char*)szErrorMsg, cbErrorMsgMax, "%s", src); + if (pcbErrorMsg) + *pcbErrorMsg = l; + if (pfNativeError) + *pfNativeError = 1; - result = SQL_SUCCESS; - strcpy(((struct _hdbc *)hdbc)->lastError, ""); - } + result = SQL_SUCCESS; + strcpy(src, ""); + } return result; } @@ -945,10 +952,11 @@ SQLRETURN SQL_API SQLErrorW( result = SQLError(henv, hdbc, hstmt, szSqlState8, pfNativeError, szErrorMsg8, 3*cbErrorMsgMax+1, &pcbErrorMsg8); if (result == SQL_SUCCESS) { + struct _hdbc *dbc = hstmt ? ((struct _hstmt *)hstmt)->hdbc : hdbc; size_t l=6, z=6*sizeof(SQLWCHAR); - ascii2unicode((struct _hdbc *)hdbc, (char*)szSqlState8, &l, (char*)szSqlState, &z); + ascii2unicode(dbc, (char*)szSqlState8, &l, (char*)szSqlState, &z); l = cbErrorMsgMax; - ascii2unicode((struct _hdbc *)hdbc, (char*)szErrorMsg8, (size_t*)&pcbErrorMsg8, (char*)szErrorMsg, &l); + ascii2unicode(dbc, (char*)szErrorMsg8, (size_t*)&pcbErrorMsg8, (char*)szErrorMsg, &l); if (pcbErrorMsg) *pcbErrorMsg = l; } @@ -969,7 +977,7 @@ SQLRETURN SQL_API SQLExecute(SQLHSTMT hstmt) mdb_sql_run_query(stmt->sql, stmt->query); if (mdb_sql_has_error(stmt->sql)) { - LogError(stmt->hdbc, "Couldn't parse SQL\n"); + LogStatementError(stmt, "Couldn't parse SQL\n"); mdb_sql_reset(stmt->sql); return SQL_ERROR; } else { @@ -1323,7 +1331,7 @@ SQLRETURN SQL_API SQLColumns( table = mdb_read_table(entry); if ( !table ) { - LogError(stmt->hdbc, "Could not read table '%s'", szTableName); + LogStatementError(stmt, "Could not read table '%s'", szTableName); return SQL_ERROR; } mdb_read_columns(table); From 55e0d4ff0745b851434d92b868d3c647fa08c2ab Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 17:34:57 -0400 Subject: [PATCH 7/8] Store sqlState separtely on ODBC env / handle / stmts Also fix the fact that "08001" was always returned by SQLError instead of the actual SQL state. --- src/odbc/mdbodbc.h | 3 +++ src/odbc/odbc.c | 62 +++++++++++++++++++++++++--------------------- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/odbc/mdbodbc.h b/src/odbc/mdbodbc.h index 35c2ba4..690c668 100644 --- a/src/odbc/mdbodbc.h +++ b/src/odbc/mdbodbc.h @@ -37,6 +37,7 @@ extern "C" { struct _henv { GPtrArray *connections; + char sqlState[6]; }; struct _hdbc { struct _henv *henv; @@ -44,6 +45,7 @@ struct _hdbc { ConnectParams* params; GPtrArray *statements; char lastError[256]; + char sqlState[6]; #ifdef ENABLE_ODBC_W iconv_t iconv_in; iconv_t iconv_out; @@ -57,6 +59,7 @@ struct _hstmt { */ char query[4096]; char lastError[256]; + char sqlState[6]; struct _sql_bind_info *bind_head; int rows_affected; int icol; /* SQLGetData: last column */ diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index 9a774dc..66326a4 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -42,8 +42,6 @@ static void unbind_columns (struct _hstmt*); #define FILL_FIELD(f,v,s) mdb_fill_temp_field(f,v,s,0,0,0,0) -static __thread char sqlState[6]; - typedef struct { SQLCHAR *type_name; SQLSMALLINT data_type; @@ -678,7 +676,7 @@ SQLRETURN SQL_API SQLDescribeCol( TRACE("SQLDescribeCol"); if (icol<1 || icol>sql->num_columns) { - strcpy(sqlState, "07009"); // Invalid descriptor index + strcpy(stmt->sqlState, "07009"); // Invalid descriptor index return SQL_ERROR; } sqlcol = g_ptr_array_index(sql->columns,icol - 1); @@ -691,7 +689,7 @@ SQLRETURN SQL_API SQLDescribeCol( } if (i==table->num_cols) { fprintf(stderr, "Column %s lost\n", (char*)sqlcol->name); - strcpy(sqlState, "07009"); // Invalid descriptor index + strcpy(stmt->sqlState, "07009"); // Invalid descriptor index return SQL_ERROR; } @@ -700,11 +698,11 @@ SQLRETURN SQL_API SQLDescribeCol( *pcbColName=strlen(sqlcol->name); if (szColName) { if (cbColNameMax < 0) { - strcpy(sqlState, "HY090"); // Invalid string or buffer length + strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length return SQL_ERROR; } if (snprintf((char *)szColName, cbColNameMax, "%s", sqlcol->name) + 1 > cbColNameMax) { - strcpy(sqlState, "01004"); // String data, right truncated + strcpy(stmt->sqlState, "01004"); // String data, right truncated ret = SQL_SUCCESS_WITH_INFO; } } @@ -782,7 +780,7 @@ SQLRETURN SQL_API SQLColAttributes( } if (icol<1 || icol>sql->num_columns) { - strcpy(sqlState, "07009"); // Invalid descriptor index + strcpy(stmt->sqlState, "07009"); // Invalid descriptor index return SQL_ERROR; } @@ -796,7 +794,7 @@ SQLRETURN SQL_API SQLColAttributes( } } if (i==table->num_cols) { - strcpy(sqlState, "07009"); // Invalid descriptor index + strcpy(stmt->sqlState, "07009"); // Invalid descriptor index return SQL_ERROR; } @@ -806,11 +804,11 @@ SQLRETURN SQL_API SQLColAttributes( case SQL_COLUMN_NAME: case SQL_DESC_NAME: case SQL_COLUMN_LABEL: /* = SQL_DESC_LABEL */ if (cbDescMax < 0) { - strcpy(sqlState, "HY090"); // Invalid string or buffer length + strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length return SQL_ERROR; } if (snprintf(rgbDesc, cbDescMax, "%s", sqlcol->name) + 1 > cbDescMax) { - strcpy(sqlState, "01004"); // String data, right truncated + strcpy(stmt->sqlState, "01004"); // String data, right truncated ret = SQL_SUCCESS_WITH_INFO; } break; @@ -849,7 +847,7 @@ SQLRETURN SQL_API SQLColAttributes( *pfDesc = SQL_ATTR_READONLY; break; default: - strcpy(sqlState, "HYC00"); // Driver not capable + strcpy(stmt->sqlState, "HYC00"); // Driver not capable ret = SQL_ERROR; break; } @@ -912,13 +910,21 @@ SQLRETURN SQL_API SQLError( TRACE("SQLError"); //if(pfNativeError)fprintf(stderr,"NativeError %05d\n", *pfNativeError); char *src = NULL; + char *state = NULL; if (hstmt) { src = ((struct _hstmt *)hstmt)->lastError; + state = ((struct _hstmt *)hstmt)->sqlState; } else if (hdbc) { src = ((struct _hdbc *)hdbc)->lastError; + state = ((struct _hdbc *)hdbc)->sqlState; + } else if (henv) { + state = ((struct _henv *)henv)->sqlState; } + + if (state) + strcpy((char*)szSqlState, state); + if (src && src[0]) { - strcpy ((char*)szSqlState, "08001"); int l = snprintf((char*)szErrorMsg, cbErrorMsgMax, "%s", src); if (pcbErrorMsg) *pcbErrorMsg = l; @@ -1095,7 +1101,7 @@ SQLRETURN SQL_API SQLFreeConnect( if (dbc->statements->len) { // Function sequence error - strcpy(sqlState, "HY010"); + strcpy(dbc->sqlState, "HY010"); return SQL_ERROR; } if (!g_ptr_array_remove(env->connections, dbc)) @@ -1121,7 +1127,7 @@ SQLRETURN SQL_API SQLFreeEnv( if (env->connections->len) { // Function sequence error - strcpy(sqlState, "HY010"); + strcpy(env->sqlState, "HY010"); return SQL_ERROR; } g_ptr_array_free(env->connections, TRUE); @@ -1427,7 +1433,7 @@ SQLRETURN SQL_API SQLGetData( mdb = sql->mdb; if (icol<1 || icol>sql->num_columns) { - strcpy(sqlState, "07009"); + strcpy(stmt->sqlState, "07009"); return SQL_ERROR; } @@ -1448,7 +1454,7 @@ SQLRETURN SQL_API SQLGetData( } if (!rgbValue) { - strcpy(sqlState, "HY009"); + strcpy(stmt->sqlState, "HY009"); return SQL_ERROR; } @@ -1463,7 +1469,7 @@ SQLRETURN SQL_API SQLGetData( /* When NULL data is retrieved, non-null pcbValue is required */ if (!pcbValue) { - strcpy(sqlState, "22002"); + strcpy(stmt->sqlState, "22002"); return SQL_ERROR; } *pcbValue = SQL_NULL_DATA; @@ -1479,7 +1485,7 @@ SQLRETURN SQL_API SQLGetData( goto found_bound_type; } } - strcpy(sqlState, "07009"); + strcpy(stmt->sqlState, "07009"); return SQL_ERROR; } found_bound_type: @@ -1501,7 +1507,7 @@ SQLRETURN SQL_API SQLGetData( switch (fCType) { case SQL_C_UTINYINT: if (intValue<0 || intValue>UCHAR_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLCHAR*)rgbValue = (SQLCHAR)intValue; @@ -1511,7 +1517,7 @@ SQLRETURN SQL_API SQLGetData( case SQL_C_TINYINT: case SQL_C_STINYINT: if (intValueSCHAR_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLSCHAR*)rgbValue = (SQLSCHAR)intValue; @@ -1521,7 +1527,7 @@ SQLRETURN SQL_API SQLGetData( case SQL_C_USHORT: case SQL_C_SHORT: if (intValue<0 || intValue>USHRT_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLSMALLINT*)rgbValue = (SQLSMALLINT)intValue; @@ -1530,7 +1536,7 @@ SQLRETURN SQL_API SQLGetData( break; case SQL_C_SSHORT: if (intValueSHRT_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLSMALLINT*)rgbValue = (SQLSMALLINT)intValue; @@ -1539,7 +1545,7 @@ SQLRETURN SQL_API SQLGetData( break; case SQL_C_ULONG: if (intValue<0 || intValue>UINT_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLUINTEGER*)rgbValue = (SQLINTEGER)intValue; @@ -1549,7 +1555,7 @@ SQLRETURN SQL_API SQLGetData( case SQL_C_LONG: case SQL_C_SLONG: if (intValueINT_MAX) { - strcpy(sqlState, "22003"); // Numeric value out of range + strcpy(stmt->sqlState, "22003"); // Numeric value out of range return SQL_ERROR; } *(SQLINTEGER*)rgbValue = intValue; @@ -1557,7 +1563,7 @@ SQLRETURN SQL_API SQLGetData( *pcbValue = sizeof(SQLINTEGER); break; default: - strcpy(sqlState, "HYC00"); // Not implemented + strcpy(stmt->sqlState, "HYC00"); // Not implemented return SQL_ERROR; } break; @@ -1627,7 +1633,7 @@ SQLRETURN SQL_API SQLGetData( return SQL_NO_DATA; } if (cbValueMax < 0) { - strcpy(sqlState, "HY090"); // Invalid string or buffer length + strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length free(str); str = NULL; return SQL_ERROR; @@ -1662,7 +1668,7 @@ SQLRETURN SQL_API SQLGetData( if (partsRemain) { stmt->pos += cbValueMax - ( needsTerminator ? 1 : 0 ); if (col->col_type != MDB_OLE) { free(str); str = NULL; } - strcpy(sqlState, "01004"); // truncated + strcpy(stmt->sqlState, "01004"); // truncated return SQL_SUCCESS_WITH_INFO; } stmt->pos = len; @@ -1922,7 +1928,7 @@ SQLRETURN SQL_API SQLGetInfo( default: if (pcbInfoValue) *pcbInfoValue = 0; - strcpy(sqlState, "HYC00"); + strcpy(((struct _hdbc *)hdbc)->sqlState, "HYC00"); return SQL_ERROR; } From 673c7700b9f33dd22f5fc9009386786b3526e861 Mon Sep 17 00:00:00 2001 From: Evan Miller Date: Wed, 19 Aug 2020 19:24:40 -0400 Subject: [PATCH 8/8] Store OLE strings on the statement object Also cut MDB_OLE out into its own code path. This removes the last of the global / thread-local storage in the ODBC driver, I think. --- src/odbc/mdbodbc.h | 2 ++ src/odbc/odbc.c | 89 ++++++++++++++++++++++++++++------------------ 2 files changed, 57 insertions(+), 34 deletions(-) diff --git a/src/odbc/mdbodbc.h b/src/odbc/mdbodbc.h index 690c668..9df48a0 100644 --- a/src/odbc/mdbodbc.h +++ b/src/odbc/mdbodbc.h @@ -60,6 +60,8 @@ struct _hstmt { char query[4096]; char lastError[256]; char sqlState[6]; + char *ole_str; + size_t ole_len; struct _sql_bind_info *bind_head; int rows_affected; int icol; /* SQLGetData: last column */ diff --git a/src/odbc/odbc.c b/src/odbc/odbc.c index 66326a4..3244c81 100644 --- a/src/odbc/odbc.c +++ b/src/odbc/odbc.c @@ -1158,6 +1158,8 @@ SQLRETURN SQL_API SQLFreeStmt( /* Bound parameters not currently implemented */ } else { } + free(stmt->ole_str); + stmt->ole_str = NULL; return SQL_SUCCESS; } @@ -1611,33 +1613,63 @@ SQLRETURN SQL_API SQLGetData( break; } #endif + case MDB_OLE: + if (cbValueMax < 0) { + strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length + return SQL_ERROR; + } + if (stmt->pos == 0) { + if (stmt->ole_str) { + free(stmt->ole_str); + } + stmt->ole_str = mdb_ole_read_full(mdb, col, &stmt->ole_len); + } + if (stmt->pos >= stmt->ole_len) { + return SQL_NO_DATA; + } + if (pcbValue) { + *pcbValue = stmt->ole_len - stmt->pos; + } + if (cbValueMax == 0) { + return SQL_SUCCESS_WITH_INFO; + } + /* if the column type is OLE, then we don't add terminators + see https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetdata-function?view=sql-server-ver15 + and https://www.ibm.com/support/knowledgecenter/SSEPEK_11.0.0/odbc/src/tpc/db2z_fngetdata.html + + "The buffer that the rgbValue argument specifies contains nul-terminated values, unless you retrieve + binary data, or the SQL data type of the column is graphic (DBCS) and the C buffer type is SQL_C_CHAR." + */ + const int totalSizeRemaining = stmt->ole_len - stmt->pos; + const int partsRemain = cbValueMax < totalSizeRemaining; + const int sizeToReadThisPart = partsRemain ? cbValueMax : totalSizeRemaining; + memcpy(rgbValue, stmt->ole_str + stmt->pos, sizeToReadThisPart); + + if (partsRemain) { + stmt->pos += cbValueMax; + strcpy(stmt->sqlState, "01004"); // truncated + return SQL_SUCCESS_WITH_INFO; + } + stmt->pos = stmt->ole_len; + free(stmt->ole_str); + stmt->ole_str = NULL; + break; default: /* FIXME here we assume fCType == SQL_C_CHAR */ to_c_char: { - static __thread size_t len = 0; - static __thread char *str = NULL; - - if (col->col_type == MDB_OLE) { - if (stmt->pos == 0) { - str = mdb_ole_read_full(mdb, col, &len); - } - } else { - str = mdb_col_to_string(mdb, mdb->pg_buf, - col->cur_value_start, col->col_type, col->cur_value_len); - len = strlen(str); + if (cbValueMax < 0) { + strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length + return SQL_ERROR; } + char *str = mdb_col_to_string(mdb, mdb->pg_buf, + col->cur_value_start, col->col_type, col->cur_value_len); + size_t len = strlen(str); if (stmt->pos >= len) { free(str); str = NULL; return SQL_NO_DATA; } - if (cbValueMax < 0) { - strcpy(stmt->sqlState, "HY090"); // Invalid string or buffer length - free(str); - str = NULL; - return SQL_ERROR; - } if (pcbValue) { *pcbValue = len + 1 - stmt->pos; } @@ -1647,27 +1679,16 @@ SQLRETURN SQL_API SQLGetData( return SQL_SUCCESS_WITH_INFO; } - /* if the column type is OLE, then we don't add terminators - see https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetdata-function?view=sql-server-ver15 - and https://www.ibm.com/support/knowledgecenter/SSEPEK_11.0.0/odbc/src/tpc/db2z_fngetdata.html - - "The buffer that the rgbValue argument specifies contains nul-terminated values, unless you retrieve - binary data, or the SQL data type of the column is graphic (DBCS) and the C buffer type is SQL_C_CHAR." - */ - const int needsTerminator = (col->col_type != MDB_OLE); - const int totalSizeRemaining = len - stmt->pos; - const int partsRemain = cbValueMax - ( needsTerminator ? 1 : 0 ) < totalSizeRemaining; - const int sizeToReadThisPart = partsRemain ? cbValueMax - ( needsTerminator ? 1 : 0 ) : totalSizeRemaining; + const int partsRemain = cbValueMax - 1 < totalSizeRemaining; + const int sizeToReadThisPart = partsRemain ? cbValueMax - 1 : totalSizeRemaining; memcpy(rgbValue, str + stmt->pos, sizeToReadThisPart); - if (needsTerminator) - { - ((char *)rgbValue)[sizeToReadThisPart] = '\0'; - } + ((char *)rgbValue)[sizeToReadThisPart] = '\0'; + if (partsRemain) { - stmt->pos += cbValueMax - ( needsTerminator ? 1 : 0 ); - if (col->col_type != MDB_OLE) { free(str); str = NULL; } + stmt->pos += cbValueMax - 1; + free(str); str = NULL; strcpy(stmt->sqlState, "01004"); // truncated return SQL_SUCCESS_WITH_INFO; }