Skip to content

Commit

Permalink
Fix of crashes in the DS setup dialog and in freeing explicit descrip…
Browse files Browse the repository at this point in the history 
…tor,

if statement that used it has been free'd prior to that.
Setup dialog crashed after test of connection. The reason was that
connection function in driver after successful stored pointer to the DSN
structure it used in the DBC object. And setup lib passed pointer to the structure allocated
on stack. The patch makes caller decide whether to store DSN pointer or
not.
Stmt destructor did not check if ard and apd are explicitly allocated, and
always free'd them. That lead to the crash when application attempted to
free such descriptor afterwards.
Also patch contains minor changes such as change of the project name and
amendments to couple of testcases.
  • Loading branch information
@lawrinn
lawrinn committed Feb 1, 2015
1 parent 7f89b0d commit afe145e
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 16 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Expand Up @@ -16,7 +16,7 @@
# or write to the Free Software Foundation, Inc.,
# 51 Franklin St., Fifth Floor, Boston, MA 02110, USA
# *************************************************************************************/
PROJECT(mariadb_connector_c C)
PROJECT(mariadb_connector_odbc C)

cmake_minimum_required(VERSION 2.8)

Expand Down
8 changes: 5 additions & 3 deletions ma_connection.c
View file
Expand Up @@ -529,7 +529,9 @@ SQLRETURN MADB_DbcEndTran(MADB_Dbc *Dbc, SQLSMALLINT CompletionType)
}
/* }}} */

/* {{{ MADB_Dbc_ConnectDB */
/* {{{ MADB_Dbc_ConnectDB
Mind that this function is used for establishing connection from the setup lib
*/
SQLRETURN MADB_DbcConnectDB(MADB_Dbc *Connection,
MADB_Dsn *Dsn)
{
Expand Down Expand Up @@ -661,8 +663,7 @@ SQLRETURN MADB_DbcConnectDB(MADB_Dbc *Connection,
mysql_close(Connection->mariadb);
Connection->mariadb= NULL;
}
else
Connection->Dsn= Dsn;

return Connection->Error.ReturnValue;
}
/* }}} */
Expand Down Expand Up @@ -1587,6 +1588,7 @@ SQLRETURN MADB_DriverConnect(MADB_Dbc *Dbc, SQLHWND WindowHandle, SQLCHAR *InCon
if (!SQL_SUCCEEDED(ret))
goto error;
end:
Dbc->Dsn= Dsn;
Length= MADB_DsnToString(Dsn, (char *)OutConnectionString, BufferLength);
if (StringLength2Ptr)
*StringLength2Ptr= (SQLSMALLINT)Length;
Expand Down
70 changes: 62 additions & 8 deletions ma_statement.c
View file
Expand Up @@ -497,6 +497,7 @@ SQLRETURN MADB_ExecutePositionedUpdate(MADB_Stmt *Stmt)
}
/* }}} */

/* {{{ MADB_GetOutParams */
SQLRETURN MADB_GetOutParams(MADB_Stmt *Stmt, int CurrentOffset)
{
MYSQL_BIND *Bind;
Expand Down Expand Up @@ -536,7 +537,7 @@ SQLRETURN MADB_GetOutParams(MADB_Stmt *Stmt, int CurrentOffset)
MADB_FREE(Bind);
return SQL_SUCCESS;
}

/* }}} */

/* {{{ MADB_StmtExecute */
SQLRETURN MADB_StmtExecute(MADB_Stmt *Stmt)
Expand Down Expand Up @@ -1115,6 +1116,31 @@ SQLRETURN MADB_StmtGetDiagRec(MADB_Stmt *Stmt, SQLSMALLINT RecNumber,
}
/* }}} */

/* {{{ remove_stmt_ref_from_desc
Helper function removing references to the stmt in the descriptor when explisitly allocated descriptor is substituted
by some other descriptor */
void remove_stmt_ref_from_desc(MADB_Desc *desc, MADB_Stmt *Stmt, BOOL all)
{
if (desc->AppType)
{
uint i;
for (i=0; i < desc->Stmts.elements; ++i)
{
MADB_Stmt **refStmt= ((MADB_Stmt **)desc->Stmts.buffer) + i;
if (Stmt == *refStmt)
{
delete_dynamic_element(&desc->Stmts, i);

if (!all)
{
return;
}
}
}
}
}
/* }}} */

/* {{{ MADB_StmtFree */
SQLRETURN MADB_StmtFree(MADB_Stmt *Stmt, SQLUSMALLINT Option)
{
Expand Down Expand Up @@ -1176,9 +1202,24 @@ SQLRETURN MADB_StmtFree(MADB_Stmt *Stmt, SQLUSMALLINT Option)
MADB_FREE(Stmt->Cursor.Name);
MADB_FREE(Stmt->StmtString);
MADB_FREE(Stmt->NativeSql);

MADB_DescFree( Stmt->Apd, FALSE);
MADB_DescFree(Stmt->Ard, FALSE);

/* For explicit descriptors we only remove reference to the stmt*/
if (Stmt->Apd->AppType)
{
remove_stmt_ref_from_desc(Stmt->Apd, Stmt, TRUE);
}
else
{
MADB_DescFree( Stmt->Apd, FALSE);
}
if (Stmt->Ard->AppType)
{
remove_stmt_ref_from_desc(Stmt->Ard, Stmt, TRUE);
}
else
{
MADB_DescFree(Stmt->Ard, FALSE);
}
MADB_DescFree(Stmt->Ipd, FALSE);
MADB_DescFree(Stmt->Ird, FALSE);

Expand Down Expand Up @@ -1620,6 +1661,7 @@ SQLRETURN MADB_StmtGetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER Val
}
/* }}} */


/* {{{ MADB_StmtSetAttr */
SQLRETURN MADB_StmtSetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER ValuePtr, SQLINTEGER StringLength)
{
Expand All @@ -1643,6 +1685,7 @@ SQLRETURN MADB_StmtSetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER Val
MADB_SetError(&Stmt->Error, MADB_ERR_HY024, NULL, 0);
return Stmt->Error.ReturnValue;
}
remove_stmt_ref_from_desc(Stmt->Apd, Stmt, FALSE);
Stmt->Apd= (MADB_Desc *)ValuePtr;
Stmt->Apd->DescType= MADB_DESC_APD;
if (Stmt->Apd != Stmt->IApd)
Expand All @@ -1653,7 +1696,10 @@ SQLRETURN MADB_StmtSetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER Val
}
}
else
{
remove_stmt_ref_from_desc(Stmt->Apd, Stmt, FALSE);
Stmt->Apd= Stmt->IApd;
}
break;
case SQL_ATTR_APP_ROW_DESC:
if (ValuePtr)
Expand All @@ -1669,6 +1715,7 @@ SQLRETURN MADB_StmtSetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER Val
MADB_SetError(&Stmt->Error, MADB_ERR_HY024, NULL, 0);
return Stmt->Error.ReturnValue;
}
remove_stmt_ref_from_desc(Stmt->Ard, Stmt, FALSE);
Stmt->Ard= (MADB_Desc *)ValuePtr;
Stmt->Ard->DescType= MADB_DESC_ARD;
if (Stmt->Ard != Stmt->IArd)
Expand All @@ -1679,7 +1726,10 @@ SQLRETURN MADB_StmtSetAttr(MADB_Stmt *Stmt, SQLINTEGER Attribute, SQLPOINTER Val
}
}
else
{
remove_stmt_ref_from_desc(Stmt->Ard, Stmt, FALSE);
Stmt->Ard= Stmt->IArd;
}
break;

case SQL_ATTR_PARAM_BIND_OFFSET_PTR:
Expand Down Expand Up @@ -2082,18 +2132,22 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,
if (Stmt->stmt->fields[Offset].type == MYSQL_TYPE_BLOB &&
Stmt->stmt->fields[Offset].charsetnr == 63)
{
char *TmpBuffer;
if (!BufferLength && StrLen_or_IndPtr)
{
*StrLen_or_IndPtr= Stmt->stmt->fields[Offset].max_length * 2;
return SQL_SUCCESS_WITH_INFO;
}

/* todo : hex conversion
if (!(TmpBuffer= (char *)MADB_CALLOC(BufferLength)))
#ifdef CONVERSION_TO_HEX_IMPLEMENTED
{
/*TODO: */
char *TmpBuffer;
if (!(TmpBuffer= (char *)MADB_CALLOC(BufferLength)))
{

} */
}
}
#endif
}
ZeroTerminated= 1;

Expand Down
4 changes: 4 additions & 0 deletions odbc_3_api.c
View file
Expand Up @@ -613,6 +613,10 @@ SQLRETURN SQLConnectCommon(SQLHDBC ConnectionHandle,

ret= Connection->Methods->ConnectDB(Connection, Dsn);

if (SQL_SUCCEEDED(ret))
{
Connection->Dsn= Dsn;
}
MDBUG_C_DUMP(Connection, ret, d);
MDBUG_C_RETURN(Connection, ret);
}
Expand Down
4 changes: 2 additions & 2 deletions test/catalog.c
View file
Expand Up @@ -1500,8 +1500,8 @@ ODBC_TEST(t_bug30770)

/* Connect with no default daabase */
sprintf((char *)conn, "DRIVER=%s;SERVER=%s;" \
"UID=%s;PASSWORD=%s;DATABASE=%s", my_drivername, "localhost",
my_uid, my_pwd, my_schema);
"UID=%s;PASSWORD=%s;DATABASE=%s;PORT=%u", my_drivername, "localhost",
my_uid, my_pwd, my_schema, my_port);

is_num(mydrvconnect(&Env1, &Connection1, &Stmt1, conn), OK);

Expand Down
4 changes: 2 additions & 2 deletions test/desc.c
View file
Expand Up @@ -584,8 +584,8 @@ ODBC_TEST(t_desc_curcatalog)
CHECK_ENV_RC(Env, SQLAllocConnect(Env, &Connection1));

/* Connecting not specifying default db */
sprintf((char *)conn_in, "DRIVER=%s;SERVER=localhost;UID=%s;PWD=%s", my_drivername,
my_uid, my_pwd);
sprintf((char *)conn_in, "DRIVER=%s;SERVER=localhost;UID=%s;PWD=%s;PORT=%u", my_drivername,
my_uid, my_pwd, my_port);

CHECK_DBC_RC(Connection1, SQLDriverConnect(Connection1, NULL, conn_in, sizeof(conn_in), NULL,
0, NULL,
Expand Down

0 comments on commit afe145e

Please sign in to comment.