Skip to content

Commit

Permalink
Ensure long data column buffers are freed (#306)
Browse files Browse the repository at this point in the history
The bind_type of the column stored in the StatementData object was not
copied over to the ColumnData object created for each row. This means
that the ColumnData destructor will not free the allocated buffer used
with SQLGetData.

Additionally, for SQL_C_BINARY columns, the data was never freed either
since the destructor only checked for SQL_C_CHAR and SQL_C_WCHAR.

Finally, because long data allocations are created using malloc/realloc,
we need to set a flag to use free instead of delete[] in the destructor.
On platforms we support, this shouldn't actually make any difference,
but it is mandated by the spec and reduces noise in valgrind.

Signed-off-by: Kevin Adler <[email protected]>
  • Loading branch information
kadler authored Jan 26, 2023
1 parent a01c1b5 commit 1b25694
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
21 changes: 15 additions & 6 deletions src/odbc.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ typedef struct Parameter {

typedef struct ColumnData {
SQLSMALLINT bind_type;
bool use_free;
union {
SQLCHAR *char_data;
SQLWCHAR *wchar_data;
Expand All @@ -137,13 +138,21 @@ typedef struct ColumnData {
SQLLEN size;

~ColumnData() {
if (bind_type == SQL_C_CHAR) {
delete[] this->char_data;
return;
if (bind_type == SQL_C_CHAR || bind_type == SQL_C_BINARY) {
if (use_free) {
free(this->char_data);
}
else {
delete[] this->char_data;
}
}
if (bind_type == SQL_C_WCHAR) {
delete[] this->wchar_data;
return;
else if (bind_type == SQL_C_WCHAR) {
if (use_free) {
free(this->wchar_data);
}
else {
delete[] this->wchar_data;
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/odbc_connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3589,6 +3589,8 @@ fetch_and_store
// Iterate over each column, putting the data in the row object
for (int column_index = 0; column_index < data->column_count; column_index++)
{
row[column_index].bind_type = data->columns[column_index]->bind_type;

// The column contained SQL_(W)LONG* data, so we didn't call
// SQLBindCol, and therefore there is no data to move from a buffer.
// Instead, call SQLGetData, and adjust buffer size accordingly
Expand All @@ -3600,6 +3602,10 @@ fetch_and_store
SQLLEN string_length_or_indicator;
SQLLEN data_returned_length = 0;

// We're allocating with malloc/realloc here, so the destructor
// needs to use free instead of delete[].
row[column_index].use_free = true;

if (data->columns[column_index]->bind_type == SQL_C_WCHAR)
{
row[column_index].wchar_data = (SQLWCHAR *)malloc(buffer_size);
Expand Down

0 comments on commit 1b25694

Please sign in to comment.