Skip to content

Commit 247174e

Browse files
gargsaumyagargsaumya
authored
FIX: Fixing Level 3 and level 4 compiler warnings. (#72)
[AB#37476](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/37476) This pull request focuses on improving type safety, enhancing error handling, and adding utility functions in the `mssql_python/pybind` module. The changes primarily involve the use of explicit type casting, the introduction of a `WideToUTF8` utility function for string conversion, and minor fixes to improve code clarity and correctness. ### Type Safety Improvements: * Updated `std::make_shared<SqlHandle>` calls across multiple functions in `connection.cpp` to use `static_cast<SQLSMALLINT>` for handle types (`SQL_HANDLE_ENV`, `SQL_HANDLE_DBC`, `SQL_HANDLE_STMT`) to ensure explicit type casting. [[1]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL30-R30) [[2]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL57-R57) [[3]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL151-R151) * Added explicit type casting for SQL date, time, and timestamp fields in `BindParameters` to use `SQLSMALLINT` or `SQLUSMALLINT` instead of `int`. [[1]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L323-R325) [[2]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L336-R338) [[3]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L349-R354) * Updated various `SQLSetStmtAttr_ptr` and `SQLSetConnectAttr_ptr` calls to use `reinterpret_cast` or `static_cast` for pointer conversions, ensuring type correctness. [[1]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL125-R125) [[2]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L1705-R1714) [[3]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L1793-R1802) ### Error Handling Enhancements: * Replaced manual wide-to-narrow string conversion in `LoadDriverOrThrowException` with the new `WideToUTF8` utility function for better readability and consistency. * Introduced the `WideToUTF8` function in `ddbc_bindings.cpp` and declared it in `ddbc_bindings.h` to handle `std::wstring` to `std::string` conversions. [[1]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1R475-R482) [[2]](diffhunk://#diff-85167a2d59779df18704284ab7ce46220c3619408fbf22c631ffdf29f794d635R180-R181) * Enhanced error message handling in `Connection::checkError` by using `WideToUTF8` for converting wide error messages to UTF-8. ### Code Fixes and Cleanup: * Fixed a typo in `std::memset` in `BindParameters` to correct the namespace usage. * Corrected redundant semicolon in `SQLGetData_wrap` initialization. * Improved comparison logic in `SQLGetData_wrap` and `FetchBatchData` to use `static_cast<size_t>` for comparing `dataLen` with `columnSize`. [[1]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L1140-R1149) [[2]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L1554-R1563) These changes collectively enhance the robustness, maintainability, and readability of the code. ### Checklist - [ ] **Tests Passed** (if applicable) : All pytests passed.
1 parent 0130211 commit 247174e

File tree

3 files changed

+40
-30
lines changed

3 files changed

+40
-30
lines changed

‎mssql_python/pybind/connection/connection.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ static SqlHandlePtr getEnvHandle() {
2727
if (!SQL_SUCCEEDED(ret)) {
2828
ThrowStdException("Failed to set environment attributes");
2929
}
30-
return std::make_shared<SqlHandle>(SQL_HANDLE_ENV, env);
30+
return std::make_shared<SqlHandle>(static_cast<SQLSMALLINT>(SQL_HANDLE_ENV), env);
3131
}();
3232

3333
return envHandle;
@@ -54,7 +54,7 @@ void Connection::allocateDbcHandle() {
5454
LOG("Allocate SQL Connection Handle");
5555
SQLRETURN ret = SQLAllocHandle_ptr(SQL_HANDLE_DBC, _envHandle->get(), &dbc);
5656
checkError(ret);
57-
_dbcHandle = std::make_shared<SqlHandle>(SQL_HANDLE_DBC, dbc);
57+
_dbcHandle = std::make_shared<SqlHandle>(static_cast<SQLSMALLINT>(SQL_HANDLE_DBC), dbc);
5858
}
5959

6060
void Connection::connect(const py::dict& attrs_before) {
@@ -91,7 +91,7 @@ void Connection::disconnect() {
9191
void Connection::checkError(SQLRETURN ret) const{
9292
if (!SQL_SUCCEEDED(ret)) {
9393
ErrorInfo err = SQLCheckError_Wrap(SQL_HANDLE_DBC, _dbcHandle, ret);
94-
std::string errorMsg = std::string(err.ddbcErrorMsg.begin(), err.ddbcErrorMsg.end());
94+
std::string errorMsg = WideToUTF8(err.ddbcErrorMsg);
9595
ThrowStdException(errorMsg);
9696
}
9797
}
@@ -122,7 +122,7 @@ void Connection::setAutocommit(bool enable) {
122122
}
123123
SQLINTEGER value = enable ? SQL_AUTOCOMMIT_ON : SQL_AUTOCOMMIT_OFF;
124124
LOG("Set SQL Connection Attribute");
125-
SQLRETURN ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_AUTOCOMMIT, (SQLPOINTER)value, 0);
125+
SQLRETURN ret = SQLSetConnectAttr_ptr(_dbcHandle->get(), SQL_ATTR_AUTOCOMMIT, reinterpret_cast<SQLPOINTER>(static_cast<SQLULEN>(value)), 0);
126126
checkError(ret);
127127
_autocommit = enable;
128128
}
@@ -148,7 +148,7 @@ SqlHandlePtr Connection::allocStatementHandle() {
148148
SQLHANDLE stmt = nullptr;
149149
SQLRETURN ret = SQLAllocHandle_ptr(SQL_HANDLE_STMT, _dbcHandle->get(), &stmt);
150150
checkError(ret);
151-
return std::make_shared<SqlHandle>(SQL_HANDLE_STMT, stmt);
151+
return std::make_shared<SqlHandle>(static_cast<SQLSMALLINT>(SQL_HANDLE_STMT), stmt);
152152
}
153153

154154

@@ -214,7 +214,6 @@ bool Connection::reset() {
214214
ThrowStdException("Connection handle not allocated");
215215
}
216216
LOG("Resetting connection via SQL_ATTR_RESET_CONNECTION");
217-
SQLULEN reset = SQL_TRUE;
218217
SQLRETURN ret = SQLSetConnectAttr_ptr(
219218
_dbcHandle->get(),
220219
SQL_ATTR_RESET_CONNECTION,

‎mssql_python/pybind/ddbc_bindings.cpp‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -320,9 +320,9 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
320320
}
321321
// TODO: can be moved to python by registering SQL_DATE_STRUCT in pybind
322322
SQL_DATE_STRUCT* sqlDatePtr = AllocateParamBuffer<SQL_DATE_STRUCT>(paramBuffers);
323-
sqlDatePtr->year = param.attr("year").cast<int>();
324-
sqlDatePtr->month = param.attr("month").cast<int>();
325-
sqlDatePtr->day = param.attr("day").cast<int>();
323+
sqlDatePtr->year = static_cast<SQLSMALLINT>(param.attr("year").cast<int>());
324+
sqlDatePtr->month = static_cast<SQLUSMALLINT>(param.attr("month").cast<int>());
325+
sqlDatePtr->day = static_cast<SQLUSMALLINT>(param.attr("day").cast<int>());
326326
dataPtr = static_cast<void*>(sqlDatePtr);
327327
break;
328328
}
@@ -333,9 +333,9 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
333333
}
334334
// TODO: can be moved to python by registering SQL_TIME_STRUCT in pybind
335335
SQL_TIME_STRUCT* sqlTimePtr = AllocateParamBuffer<SQL_TIME_STRUCT>(paramBuffers);
336-
sqlTimePtr->hour = param.attr("hour").cast<int>();
337-
sqlTimePtr->minute = param.attr("minute").cast<int>();
338-
sqlTimePtr->second = param.attr("second").cast<int>();
336+
sqlTimePtr->hour = static_cast<SQLUSMALLINT>(param.attr("hour").cast<int>());
337+
sqlTimePtr->minute = static_cast<SQLUSMALLINT>(param.attr("minute").cast<int>());
338+
sqlTimePtr->second = static_cast<SQLUSMALLINT>(param.attr("second").cast<int>());
339339
dataPtr = static_cast<void*>(sqlTimePtr);
340340
break;
341341
}
@@ -346,12 +346,12 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
346346
}
347347
SQL_TIMESTAMP_STRUCT* sqlTimestampPtr =
348348
AllocateParamBuffer<SQL_TIMESTAMP_STRUCT>(paramBuffers);
349-
sqlTimestampPtr->year = param.attr("year").cast<int>();
350-
sqlTimestampPtr->month = param.attr("month").cast<int>();
351-
sqlTimestampPtr->day = param.attr("day").cast<int>();
352-
sqlTimestampPtr->hour = param.attr("hour").cast<int>();
353-
sqlTimestampPtr->minute = param.attr("minute").cast<int>();
354-
sqlTimestampPtr->second = param.attr("second").cast<int>();
349+
sqlTimestampPtr->year = static_cast<SQLSMALLINT>(param.attr("year").cast<int>());
350+
sqlTimestampPtr->month = static_cast<SQLUSMALLINT>(param.attr("month").cast<int>());
351+
sqlTimestampPtr->day = static_cast<SQLUSMALLINT>(param.attr("day").cast<int>());
352+
sqlTimestampPtr->hour = static_cast<SQLUSMALLINT>(param.attr("hour").cast<int>());
353+
sqlTimestampPtr->minute = static_cast<SQLUSMALLINT>(param.attr("minute").cast<int>());
354+
sqlTimestampPtr->second = static_cast<SQLUSMALLINT>(param.attr("second").cast<int>());
355355
// SQL server supports in ns, but python datetime supports in µs
356356
sqlTimestampPtr->fraction = static_cast<SQLUINTEGER>(
357357
param.attr("microsecond").cast<int>() * 1000); // Convert µs to ns
@@ -372,7 +372,7 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
372372
decimalPtr->scale = decimalParam.scale;
373373
decimalPtr->sign = decimalParam.sign;
374374
// Convert the integer decimalParam.val to char array
375-
std:memset(static_cast<void*>(decimalPtr->val), 0, sizeof(decimalPtr->val));
375+
std::memset(static_cast<void*>(decimalPtr->val), 0, sizeof(decimalPtr->val));
376376
std::memcpy(static_cast<void*>(decimalPtr->val),
377377
reinterpret_cast<char*>(&decimalParam.val),
378378
sizeof(decimalParam.val));
@@ -395,8 +395,11 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
395395
assert(SQLBindParameter_ptr && SQLGetStmtAttr_ptr && SQLSetDescField_ptr);
396396

397397
RETCODE rc = SQLBindParameter_ptr(
398-
hStmt, paramIndex + 1 /* 1-based indexing */, paramInfo.inputOutputType,
399-
paramInfo.paramCType, paramInfo.paramSQLType, paramInfo.columnSize,
398+
hStmt,
399+
static_cast<SQLUSMALLINT>(paramIndex + 1), /* 1-based indexing */
400+
static_cast<SQLUSMALLINT>(paramInfo.inputOutputType),
401+
static_cast<SQLSMALLINT>(paramInfo.paramCType),
402+
static_cast<SQLSMALLINT>(paramInfo.paramSQLType), paramInfo.columnSize,
400403
paramInfo.decimalDigits, dataPtr, bufferLength, strLenOrIndPtr);
401404
if (!SQL_SUCCEEDED(rc)) {
402405
LOG("Error when binding parameter - {}", paramIndex);
@@ -406,7 +409,7 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
406409
// https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/retrieve-numeric-data-sql-numeric-struct-kb222831?view=sql-server-ver16#sql_c_numeric-overview
407410
if (paramInfo.paramCType == SQL_C_NUMERIC) {
408411
SQLHDESC hDesc = nullptr;
409-
RETCODE rc = SQLGetStmtAttr_ptr(hStmt, SQL_ATTR_APP_PARAM_DESC, &hDesc, 0, NULL);
412+
rc = SQLGetStmtAttr_ptr(hStmt, SQL_ATTR_APP_PARAM_DESC, &hDesc, 0, NULL);
410413
if(!SQL_SUCCEEDED(rc)) {
411414
LOG("Error when getting statement attribute - {}", paramIndex);
412415
return rc;
@@ -469,6 +472,14 @@ void LOG(const std::string& formatString, Args&&... args) {
469472
logging.attr("debug")(message);
470473
}
471474

475+
std::string WideToUTF8(const std::wstring& wstr) {
476+
if (wstr.empty()) return {};
477+
int size_needed = WideCharToMultiByte(CP_UTF8, 0, wstr.data(), (int)wstr.size(), nullptr, 0, nullptr, nullptr);
478+
std::string result(size_needed, 0);
479+
WideCharToMultiByte(CP_UTF8, 0, wstr.data(), (int)wstr.size(), result.data(), size_needed, nullptr, nullptr);
480+
return result;
481+
}
482+
472483
// TODO: Add more nuanced exception classes
473484
void ThrowStdException(const std::string& message) { throw std::runtime_error(message); }
474485

@@ -523,8 +534,7 @@ std::wstring LoadDriverOrThrowException() {
523534
}
524535

525536
// Convert wstring to string for logging
526-
std::string dllDirStr(dllDir.begin(), dllDir.end());
527-
LOG("Attempting to load driver from - {}", dllDirStr);
537+
LOG("Attempting to load driver from - {}", WideToUTF8(dllDir));
528538

529539
HMODULE hModule = LoadLibraryW(dllDir.c_str());
530540
if (!hModule) {
@@ -597,7 +607,6 @@ std::wstring LoadDriverOrThrowException() {
597607
SQLDisconnect_ptr && SQLFreeStmt_ptr && SQLGetDiagRec_ptr;
598608

599609
if (!success) {
600-
LOG("Failed to load required function pointers from driver - {}", dllDirStr);
601610
ThrowStdException("Failed to load required function pointers from driver");
602611
}
603612
LOG("Successfully loaded function pointers from driver");
@@ -860,7 +869,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
860869
DriverLoader::getInstance().loadDriver(); // Load the driver
861870
}
862871

863-
SQLRETURN ret;
872+
SQLRETURN ret = SQL_SUCCESS;
864873
SQLHSTMT hStmt = StatementHandle->get();
865874
for (SQLSMALLINT i = 1; i <= colCount; ++i) {
866875
SQLWCHAR columnName[256];
@@ -1137,7 +1146,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p
11371146
if (SQL_SUCCEEDED(ret)) {
11381147
// TODO: Refactor these if's across other switches to avoid code duplication
11391148
if (dataLen > 0) {
1140-
if (dataLen <= columnSize) {
1149+
if (static_cast<size_t>(dataLen) <= columnSize) {
11411150
row.append(py::bytes(reinterpret_cast<const char*>(
11421151
dataBuffer.get()), dataLen));
11431152
} else {
@@ -1551,7 +1560,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
15511560
// TODO: variable length data needs special handling, this logic wont suffice
15521561
SQLULEN columnSize = columnMeta["ColumnSize"].cast<SQLULEN>();
15531562
HandleZeroColumnSizeAtFetch(columnSize);
1554-
if (dataLen <= columnSize) {
1563+
if (static_cast<size_t>(dataLen) <= columnSize) {
15551564
row.append(py::bytes(reinterpret_cast<const char*>(
15561565
&buffers.charBuffers[col - 1][i * columnSize]),
15571566
dataLen));
@@ -1702,7 +1711,7 @@ SQLRETURN FetchMany_wrap(SqlHandlePtr StatementHandle, py::list& rows, int fetch
17021711
}
17031712

17041713
SQLULEN numRowsFetched;
1705-
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)fetchSize, 0);
1714+
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)(intptr_t)fetchSize, 0);
17061715
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROWS_FETCHED_PTR, &numRowsFetched, 0);
17071716

17081717
ret = FetchBatchData(hStmt, buffers, columnNames, rows, numCols, numRowsFetched);
@@ -1790,7 +1799,7 @@ SQLRETURN FetchAll_wrap(SqlHandlePtr StatementHandle, py::list& rows) {
17901799
}
17911800

17921801
SQLULEN numRowsFetched;
1793-
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)fetchSize, 0);
1802+
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)(intptr_t)fetchSize, 0);
17941803
SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_ROWS_FETCHED_PTR, &numRowsFetched, 0);
17951804

17961805
while (ret != SQL_NO_DATA) {

‎mssql_python/pybind/ddbc_bindings.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,5 @@ struct ErrorInfo {
177177
std::wstring ddbcErrorMsg;
178178
};
179179
ErrorInfo SQLCheckError_Wrap(SQLSMALLINT handleType, SqlHandlePtr handle, SQLRETURN retcode);
180+
181+
std::string WideToUTF8(const std::wstring& wstr);

0 commit comments

Comments
 (0)