Skip to content

Commit

Permalink
ODBC-380 Memory leak during connect if multistatement option selected
Browse files Browse the repository at this point in the history 
The leak occurred once per connection due to missing
MADB_DynstrFree call(the other small change in commit is not related).

Some cleaning in the testcases, so that their own leaks do not create
any noise.

Fix of the memory leak, if string field is SQLGetData'ed as datetime,
In SQLColumns in case of invalid parameters error,
In most cases of SQLGetData as SQL_NUMERIC,
In execution with parameters array for date/time parameter types.
  • Loading branch information
@lawrinn
lawrinn committed Jan 21, 2023
1 parent 8fc8d33 commit 5379a12
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 38 deletions.
2 changes: 1 addition & 1 deletion ma_bulk.c
View file
Expand Up @@ -85,7 +85,7 @@ void MADB_CleanBulkOperData(MADB_Stmt *Stmt, unsigned int ParamOffset)
switch (CRec->ConciseType)
{
case DATETIME_TYPES:
if (CanUseStructArrForDatetime(Stmt) == FALSE)
if (CanUseStructArrForDatetime(Stmt) == TRUE)
{
MADB_FREE(MaBind->buffer);
break;
Expand Down
5 changes: 3 additions & 2 deletions ma_catalog.c
View file
Expand Up @@ -476,7 +476,7 @@ SQLRETURN MADB_StmtColumns(MADB_Stmt *Stmt,
MADB_DynString StmtStr;
SQLRETURN ret;
size_t Length= strlen(MADB_CATALOG_COLUMNSp3);
char *ColumnsPart= MADB_CALLOC(Length);
char *ColumnsPart= NULL;
unsigned int OctetsPerChar= Stmt->Connection->Charset.cs_info->char_maxlen > 0 && Stmt->Connection->Charset.cs_info->char_maxlen < 10 ? Stmt->Connection->Charset.cs_info->char_maxlen : 1;

MDBUG_C_ENTER(Stmt->Connection, "StmtColumns");
Expand All @@ -485,11 +485,12 @@ SQLRETURN MADB_StmtColumns(MADB_Stmt *Stmt,
{
return MADB_SetError(&Stmt->Error, MADB_ERR_HYC00, "Schemas are not supported. Use CatalogName parameter instead", 0);
}

ColumnsPart = MADB_CALLOC(Length);
if (ColumnsPart == NULL)
{
return MADB_SetError(&Stmt->Error, MADB_ERR_HY001, NULL, 0);
}

_snprintf(ColumnsPart, Length, MADB_CATALOG_COLUMNSp3, OctetsPerChar);

MADB_InitDynamicString(&StmtStr, "", 8192, 1024);
Expand Down
1 change: 1 addition & 0 deletions ma_connection.c
View file
Expand Up @@ -805,6 +805,7 @@ SQLRETURN MADB_DbcConnectDB(MADB_Dbc *Connection,
if (DSN_OPTION(Connection, MADB_OPT_FLAG_MULTI_STATEMENTS))
{
mysql_optionsv(Connection->mariadb, MYSQL_INIT_COMMAND, InitCmd.str);
MADB_DynstrFree(&InitCmd);
}

if (Dsn->ConnectionTimeout)
Expand Down
61 changes: 44 additions & 17 deletions ma_statement.c
View file
Expand Up @@ -1531,7 +1531,7 @@ SQLRETURN MADB_StmtBindCol(MADB_Stmt *Stmt, SQLUSMALLINT ColumnNumber, SQLSMALLI

if ((ColumnNumber < 1 && Stmt->Options.UseBookmarks == SQL_UB_OFF) ||
(mysql_stmt_field_count(Stmt->stmt) &&
Stmt->stmt->state > MYSQL_STMT_PREPARED &&
STMT_WAS_PREPARED(Stmt) &&
ColumnNumber > mysql_stmt_field_count(Stmt->stmt)))
{
MADB_SetError(&Stmt->Error, MADB_ERR_07009, NULL, 0);
Expand Down Expand Up @@ -2929,25 +2929,30 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,

if (IrdRec->ConciseType == SQL_CHAR || IrdRec->ConciseType == SQL_VARCHAR)
{
char *ClientValue= NULL;
BOOL isTime;

Bind.buffer_length = (Stmt->stmt->fields[Offset].max_length != 0 ?
Stmt->stmt->fields[Offset].max_length : Stmt->stmt->fields[Offset].length) + 1;

if (!(ClientValue = (char *)MADB_CALLOC(Bind.buffer_length)))
if (IrdRec->InternalBuffer != NULL)
{
IrdRec->InternalBuffer = (char*)MADB_REALLOC(IrdRec->InternalBuffer, Bind.buffer_length);
}
else
{
IrdRec->InternalBuffer= (char*)MADB_ALLOC(Bind.buffer_length);
}

if (IrdRec->InternalBuffer == NULL)
{
return MADB_SetError(&Stmt->Error, MADB_ERR_HY001, NULL, 0);
}
Bind.buffer= ClientValue;
Bind.buffer= IrdRec->InternalBuffer;
Bind.buffer_type= MYSQL_TYPE_STRING;
mysql_stmt_fetch_column(Stmt->stmt, &Bind, Offset, 0);
RETURN_ERROR_OR_CONTINUE(MADB_Str2Ts(ClientValue, Bind.length_value, &tm, FALSE, &Stmt->Error, &isTime));
RETURN_ERROR_OR_CONTINUE(MADB_Str2Ts(IrdRec->InternalBuffer, Bind.length_value, &tm, FALSE, &Stmt->Error, &isTime));
}
else
{


Bind.buffer_length= sizeof(MYSQL_TIME);
Bind.buffer= (void *)&tm;
/* c/c is too smart to convert hours to days and days to hours, we don't need that */
Expand All @@ -2974,20 +2979,28 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,

if (IrdRec->ConciseType == SQL_CHAR || IrdRec->ConciseType == SQL_VARCHAR)
{
char *ClientValue= NULL;
BOOL isTime;

Bind.buffer_length= (Stmt->stmt->fields[Offset].max_length != 0 ? Stmt->stmt->fields[Offset].max_length :
Stmt->stmt->fields[Offset].length) + 1;

if (!(ClientValue = (char *)MADB_CALLOC(Bind.buffer_length)))
if (IrdRec->InternalBuffer != NULL)
{
IrdRec->InternalBuffer = (char*)MADB_REALLOC(IrdRec->InternalBuffer, Bind.buffer_length);
}
else
{
IrdRec->InternalBuffer = (char*)MADB_ALLOC(Bind.buffer_length);
}

if (IrdRec->InternalBuffer == NULL)
{
return MADB_SetError(&Stmt->Error, MADB_ERR_HY001, NULL, 0);
}
Bind.buffer= ClientValue;
Bind.buffer= IrdRec->InternalBuffer;
Bind.buffer_type= MYSQL_TYPE_STRING;
mysql_stmt_fetch_column(Stmt->stmt, &Bind, Offset, 0);
RETURN_ERROR_OR_CONTINUE(MADB_Str2Ts(ClientValue, Bind.length_value, &tm, TRUE, &Stmt->Error, &isTime));
RETURN_ERROR_OR_CONTINUE(MADB_Str2Ts(IrdRec->InternalBuffer, Bind.length_value, &tm, TRUE, &Stmt->Error, &isTime));
}
else
{
Expand Down Expand Up @@ -3277,13 +3290,28 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,
case SQL_NUMERIC:
{
SQLRETURN rc;
char *tmp;
char *tmp= NULL;
MADB_DescRecord *Ard= MADB_DescGetInternalRecord(Stmt->Ard, Offset, MADB_DESC_READ);

Bind.buffer_length= MADB_DEFAULT_PRECISION + 1/*-*/ + 1/*.*/;
tmp= (char *)MADB_CALLOC(Bind.buffer_length);
Bind.buffer= tmp;
if (IrdRec->InternalBuffer != NULL)
{
tmp= (char*)MADB_REALLOC(IrdRec->InternalBuffer, Bind.buffer_length);
}
else
{
tmp= (char*)MADB_ALLOC(Bind.buffer_length);
}

if (tmp == NULL)
{
return MADB_SetError(&Stmt->Error, MADB_ERR_HY001, NULL, 0);
}
else
{
IrdRec->InternalBuffer= tmp;
}
Bind.buffer= IrdRec->InternalBuffer;
Bind.buffer_type= MadbType;

mysql_stmt_fetch_column(Stmt->stmt, &Bind, Offset, 0);
Expand All @@ -3293,11 +3321,10 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT StatementHandle,
if (Bind.buffer_length < *Bind.length)
{
MADB_SetError(&Stmt->Error, MADB_ERR_22003, NULL, 0);
MADB_FREE(tmp);
return Stmt->Error.ReturnValue;
}

rc= MADB_CharToSQLNumeric(tmp, Stmt->Ard, Ard, TargetValuePtr, 0);
rc= MADB_CharToSQLNumeric(IrdRec->InternalBuffer, Stmt->Ard, Ard, TargetValuePtr, 0);

/* Ugly */
if (rc != SQL_SUCCESS)
Expand Down
32 changes: 15 additions & 17 deletions test/types.c
View file
Expand Up @@ -803,7 +803,7 @@ int sqlnum_test_from_str(SQLHANDLE Stmt,
SQLCHAR sign, SQLCHAR *expdata, int expnum,
SQLRETURN overflow)
{
SQL_NUMERIC_STRUCT *sqlnum= malloc(sizeof(SQL_NUMERIC_STRUCT));
SQL_NUMERIC_STRUCT sqlnum;/*= malloc(sizeof(SQL_NUMERIC_STRUCT));*/
SQLCHAR buf[512];
SQLHANDLE ard;
unsigned long long numval;
Expand All @@ -821,7 +821,7 @@ int sqlnum_test_from_str(SQLHANDLE Stmt,
CHECK_HANDLE_RC(SQL_HANDLE_DESC, ard, SQLSetDescField(ard, 1, SQL_DESC_SCALE,
(SQLPOINTER)(SQLLEN) scale, SQL_IS_INTEGER));
CHECK_HANDLE_RC(SQL_HANDLE_DESC, ard, SQLSetDescField(ard, 1, SQL_DESC_DATA_PTR,
sqlnum, SQL_IS_POINTER));
&sqlnum, SQL_IS_POINTER));
if (overflow != SQL_SUCCESS)
{
EXPECT_STMT(Stmt, SQLFetch(Stmt), overflow);
Expand All @@ -838,12 +838,12 @@ int sqlnum_test_from_str(SQLHANDLE Stmt,
else
CHECK_HANDLE_RC(SQL_HANDLE_STMT, Stmt, SQLFetch(Stmt));

is_num(sqlnum->precision, prec);
is_num(sqlnum->scale, scale);
is_num(sqlnum->sign, sign);
is_num(sqlnum.precision, prec);
is_num(sqlnum.scale, scale);
is_num(sqlnum.sign, sign);
if (expdata)
{
IS(!memcmp(sqlnum->val, expdata, SQL_MAX_NUMERIC_LEN));
IS(!memcmp(sqlnum.val, expdata, SQL_MAX_NUMERIC_LEN));
}
else
{
Expand All @@ -853,7 +853,7 @@ int sqlnum_test_from_str(SQLHANDLE Stmt,
numval= 0;
for (i= 0; i < 8; ++i)
{
singleByte= sqlnum->val[7 - i];
singleByte= sqlnum.val[7 - i];
numval+= singleByte << (8 * (7 - i));
}
if (numval != expnum)
Expand All @@ -863,7 +863,6 @@ int sqlnum_test_from_str(SQLHANDLE Stmt,

CHECK_HANDLE_RC(SQL_HANDLE_STMT, Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));

free(sqlnum);
return OK;
}

Expand Down Expand Up @@ -1037,7 +1036,7 @@ int sqlnum_test_to_str(SQLHANDLE Stmt, SQLCHAR* numdata, SQLCHAR prec,
SQLSCHAR scale, SQLCHAR sign, char* outstr,
char* exptrunc)
{
SQL_NUMERIC_STRUCT* sqlnum = malloc(sizeof(SQL_NUMERIC_STRUCT));
SQL_NUMERIC_STRUCT sqlnum;
SQLCHAR obuf[80]; /*1 sign + 39 + 1 dot + 38 + \0 */
SQLRETURN exprc = SQL_SUCCESS;
SQLLEN len;
Expand All @@ -1056,11 +1055,11 @@ int sqlnum_test_to_str(SQLHANDLE Stmt, SQLCHAR* numdata, SQLCHAR prec,
else if (!strcmp("22003", exptrunc))
exprc= SQL_ERROR;

sqlnum->sign = sign;
memcpy(sqlnum->val, numdata, SQL_MAX_NUMERIC_LEN);
sqlnum.sign = sign;
memcpy(sqlnum.val, numdata, SQL_MAX_NUMERIC_LEN);

EXPECT_STMT(Stmt, SQLBindParameter(Stmt, 1, SQL_PARAM_INPUT, SQL_C_NUMERIC,
SQL_DECIMAL, prec, scale, sqlnum, 0, NULL), scale > 38 ? SQL_SUCCESS_WITH_INFO : SQL_SUCCESS);
SQL_DECIMAL, prec, scale, &sqlnum, 0, NULL), scale > 38 ? SQL_SUCCESS_WITH_INFO : SQL_SUCCESS);

EXPECT_STMT(Stmt, SQLExecDirect(Stmt, "SELECT ?", SQL_NTS), exprc);

Expand All @@ -1076,17 +1075,16 @@ int sqlnum_test_to_str(SQLHANDLE Stmt, SQLCHAR* numdata, SQLCHAR prec,
/* Error heere may occur on execution. Checking SQLFetch result is pretty useless */
CHECK_STMT_RC(Stmt, SQLFetch(Stmt));

is_num(sqlnum->precision, prec);
is_num(sqlnum->scale, (scale > 38 ? 38 : scale));
is_num(sqlnum->sign, sign);
is_num(sqlnum.precision, prec);
is_num(sqlnum.scale, (scale > 38 ? 38 : scale));
is_num(sqlnum.sign, sign);
CHECK_HANDLE_RC(SQL_HANDLE_STMT, Stmt, SQLGetData(Stmt, 1, SQL_C_CHAR, obuf, sizeof(obuf), &len));
IS_STR(obuf, outstr, len + 1);
/* This is seemingly useless check */
FAIL_IF(memcmp(sqlnum->val, numdata, SQL_MAX_NUMERIC_LEN), "memcmp failed");
FAIL_IF(memcmp(sqlnum.val, numdata, SQL_MAX_NUMERIC_LEN), "memcmp failed");

CHECK_HANDLE_RC(SQL_HANDLE_STMT, Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));

free(sqlnum);
return OK;
}

Expand Down
2 changes: 1 addition & 1 deletion test/unicode.c
View file
Expand Up @@ -256,7 +256,7 @@ ODBC_TEST(sqlchar)

FAIL_IF(SQLFetch(hstmt1)!= SQL_NO_DATA_FOUND, "eof expected");

CHECK_STMT_RC(Stmt, SQLFreeStmt(hstmt1, SQL_DROP));
CHECK_STMT_RC(hstmt1, SQLFreeStmt(hstmt1, SQL_DROP));
is_num(SQLFreeHandle(SQL_HANDLE_STMT, hstmt1), SQL_INVALID_HANDLE);

CHECK_DBC_RC(hdbc1, SQLDisconnect(hdbc1));
Expand Down

0 comments on commit 5379a12

Please sign in to comment.