Issues list compiled by AI #90

New issue

Open

opened 2026-10-18 11:08:37 +00:00 by eelke · 0 comments

eelke commented

2026-10-18 11:08:37 +00:00

Owner

Memory / ownership

• High pgsql/Pgsql_Params.cpp:42: The move assignment replaces m_paramValues without first freeing the existing new char[] buffers, so they leak.
• High pgsql/Pgsql_Params.cpp:18-27: The copy assignment appends to m_paramValues without clearing it, but overwrites m_paramLengths first. appendValues then indexes m_paramLengths[ofs + i] out of bounds when the target was not empty.
• Medium pgsql/Pgsql_Params.h:61-65: Manual new[]/delete[] ownership is fragile. The two assignment bugs above are the kind of mistake it invites. fixed by #91
• Low pgsql/Pgsql_Connection.cpp:176-178: this is passed through reinterpret_cast<void*> to the libpq notice receiver. It is safe only if the connection outlives the libpq handle. Likely a false positive, as this is a common C-API pattern.

Potential bugs

• Medium pglablib/util.cpp:325-343: The if (c == 'u') check is a separate if, not chained with the L/U branch. The else at line 340 therefore runs right after L" or U" sets IN_STRING and forces ERROR, so L"..." and U"..." literals are always rejected.
• Medium pglablib/util.cpp:427-431: A \U escape for a code point at or below 0xFFFF writes nothing, because there is no else branch. The character is silently dropped. fixed in 6206d02c20
• Medium pglab/BackupDialog.cpp:404: The unresolved TODO says the backup file path is never checked for existence or validity before use.
• Low pglablib/ConnectionConfig.cpp:357 and pglab/BackupDialog.cpp:467: The password is passed to the dump process as PGPASSWORD in its environment. This is less visible than command-line arguments, and the comment at BackupDialog.cpp:464 shows the trade-off was considered. Likely accepted, so a probable
false positive.

Error handling

• Medium core/Expected.h:146-148 and core/Expected.h:255-257: These catch (...) blocks return false without recording any error, so the cause is lost. The other two catch blocks at lines 157 and 267 do convert the exception into an error value. fixed 8f5de10f58
• Low pglablib/ASyncDBConnection.cpp:152-154: The catch-all reports "Unknown error" to the UI. This is an acceptable thread-boundary guard, but it discards the exception type. Likely a false positive.
• Low pglab/crud/CrudModel.cpp:401-405: Model code calls QMessageBox::warning with a nullptr parent. This mixes UI into the data layer and makes the code harder to test.

Code smells

~~• Medium pglablib/util.cpp:291-470: ConvertLangToSqlString is a roughly 180-line state machine with deeply nested if/else chains and repeated state = ERROR; break; blocks. It is hard to follow.~~ fixed in 6206d02c20
• Medium pglab/crud/CrudModel.cpp (about 742 lines), pglab/ConnectionListModel.cpp (about 645) and pglab/querytool/QueryTool.cpp (about 627): These large files mix SQL building, model logic and UI concerns.
• Low pglablib/util.cpp:351-352: The c == 'a' branch appears twice, so the second one is dead code. Lines 356-357 use empty ; statements for \r and \v, which silently drop those characters and read poorly.
• Low pglablib/SqlFormattingUtils.cpp:58-111: A large block of commented-out C code, a copy of PQescapeInternal, is left in the file.
• Low pglablib/ParamListModel.cpp:108: A stale FIXME: Implement me! sits above a working insertRows. Commented-out stubs with the same FIXME are at lines 31-39, 115-120 and 131-136. fixed in 5821c23439
• Low pglab/BackupDialog.cpp:468 and pglab/BackupDialog.cpp:472-475: Commented-out code for the SESSIONNAME environment variable and the Windows console flags.
• Low pglablib/ASyncDBConnection.cpp:139-141: A TODO says the main thread has to remember state from callbacks, and that status getters would be better.
• Low pglablib/catalog/PgDatabaseCatalog.cpp:71: A TODO says the tablespace name lookup is not implemented.
• Low pgsql/Pgsql_Value.cpp:80-82: An open TODO about timezone handling, plus the typo "compatability" on line 82.

Style / readability

• Low pglablib/SqlFormattingUtils.cpp:18-44: Tabs and spaces are mixed within the same function.
• Low pglablib/SqlFormattingUtils.cpp:31: The comment has the typo "seperation".
• Low pglablib/util.cpp:396 and pglablib/util.cpp:415: The comments have the typo "hax" where "hex" is meant.

Verified safe (not reported)

• pglab/CodeGenerator.cpp:19-20 and pglab/CodeGenerator.cpp:33-34: Model and ui are allocated in the constructor and deleted in the destructor. This is balanced, so it is a false positive.
• pglab/crud/CrudModel.cpp:475, 485, 510 and 537: Identifiers go through quoteIdent and values use $n placeholders. No SQL injection is visible here.
• Qt widget new calls with a parent are not reported.

Notes and limitations

• This is a quick scan. pgsql/Pgsql_Result.cpp, core/SqlLexer.cpp, pglablib/catalog/PgProc.cpp, PgClass.cpp, pglablib/ConnectionConfig.cpp and PgTriggerContainer.cpp were not read in depth. Their coverage is limited to the grep hits above, so they may hide issues that did not show up in the pattern
searches.
• The plan marked Step 1 and Step 2 complete, but the review of those files was partial. Treat the list above as a starting point, not an exhaustive audit. Nothing in the project was modified.

This is a quick, shallow scan as requested. Findings are grouped by category and ordered by severity. Each reference was re-checked against the current file contents. Some items are likely false positives and are labelled as such. # Memory / ownership ~~• High pgsql/Pgsql_Params.cpp:42: The move assignment replaces m_paramValues without first freeing the existing new char[] buffers, so they leak. • High pgsql/Pgsql_Params.cpp:18-27: The copy assignment appends to m_paramValues without clearing it, but overwrites m_paramLengths first. appendValues then indexes m_paramLengths[ofs + i] out of bounds when the target was not empty. • Medium pgsql/Pgsql_Params.h:61-65: Manual new[]/delete[] ownership is fragile. The two assignment bugs above are the kind of mistake it invites.~~ fixed by #91 • Low pgsql/Pgsql_Connection.cpp:176-178: this is passed through reinterpret_cast<void*> to the libpq notice receiver. It is safe only if the connection outlives the libpq handle. Likely a false positive, as this is a common C-API pattern. # Potential bugs ~~• Medium pglablib/util.cpp:325-343: The if (c == 'u') check is a separate if, not chained with the L/U branch. The else at line 340 therefore runs right after L" or U" sets IN_STRING and forces ERROR, so L"..." and U"..." literals are always rejected. • Medium pglablib/util.cpp:427-431: A \U escape for a code point at or below 0xFFFF writes nothing, because there is no else branch. The character is silently dropped.~~ fixed in 6206d02c206a188efed3b5191d7f18efdbadfdc8 • Medium pglab/BackupDialog.cpp:404: The unresolved TODO says the backup file path is never checked for existence or validity before use. • Low pglablib/ConnectionConfig.cpp:357 and pglab/BackupDialog.cpp:467: The password is passed to the dump process as PGPASSWORD in its environment. This is less visible than command-line arguments, and the comment at BackupDialog.cpp:464 shows the trade-off was considered. Likely accepted, so a probable false positive. # Error handling ~~• Medium core/Expected.h:146-148 and core/Expected.h:255-257: These catch (...) blocks return false without recording any error, so the cause is lost. The other two catch blocks at lines 157 and 267 do convert the exception into an error value.~~ fixed 8f5de10f582881786a0d50065d478992bf8202d5 • Low pglablib/ASyncDBConnection.cpp:152-154: The catch-all reports "Unknown error" to the UI. This is an acceptable thread-boundary guard, but it discards the exception type. Likely a false positive. • Low pglab/crud/CrudModel.cpp:401-405: Model code calls QMessageBox::warning with a nullptr parent. This mixes UI into the data layer and makes the code harder to test. # Code smells ~~• Medium pglablib/util.cpp:291-470: ConvertLangToSqlString is a roughly 180-line state machine with deeply nested if/else chains and repeated state = ERROR; break; blocks. It is hard to follow.~~ fixed in 6206d02c206a188efed3b5191d7f18efdbadfdc8 • Medium pglab/crud/CrudModel.cpp (about 742 lines), pglab/ConnectionListModel.cpp (about 645) and pglab/querytool/QueryTool.cpp (about 627): These large files mix SQL building, model logic and UI concerns. ~~• Low pglablib/util.cpp:351-352: The c == 'a' branch appears twice, so the second one is dead code. Lines 356-357 use empty ; statements for \r and \v, which silently drop those characters and read poorly. • Low pglablib/SqlFormattingUtils.cpp:58-111: A large block of commented-out C code, a copy of PQescapeInternal, is left in the file. • Low pglablib/ParamListModel.cpp:108: A stale FIXME: Implement me! sits above a working insertRows. Commented-out stubs with the same FIXME are at lines 31-39, 115-120 and 131-136.~~ fixed in 5821c23439 • Low pglab/BackupDialog.cpp:468 and pglab/BackupDialog.cpp:472-475: Commented-out code for the SESSIONNAME environment variable and the Windows console flags. • Low pglablib/ASyncDBConnection.cpp:139-141: A TODO says the main thread has to remember state from callbacks, and that status getters would be better. • Low pglablib/catalog/PgDatabaseCatalog.cpp:71: A TODO says the tablespace name lookup is not implemented. • Low pgsql/Pgsql_Value.cpp:80-82: An open TODO about timezone handling, plus the typo "compatability" on line 82. # Style / readability • Low pglablib/SqlFormattingUtils.cpp:18-44: Tabs and spaces are mixed within the same function. • Low pglablib/SqlFormattingUtils.cpp:31: The comment has the typo "seperation". • Low pglablib/util.cpp:396 and pglablib/util.cpp:415: The comments have the typo "hax" where "hex" is meant. # Verified safe (not reported) • pglab/CodeGenerator.cpp:19-20 and pglab/CodeGenerator.cpp:33-34: Model and ui are allocated in the constructor and deleted in the destructor. This is balanced, so it is a false positive. • pglab/crud/CrudModel.cpp:475, 485, 510 and 537: Identifiers go through quoteIdent and values use $n placeholders. No SQL injection is visible here. • Qt widget new calls with a parent are not reported. # Notes and limitations • This is a quick scan. pgsql/Pgsql_Result.cpp, core/SqlLexer.cpp, pglablib/catalog/PgProc.cpp, PgClass.cpp, pglablib/ConnectionConfig.cpp and PgTriggerContainer.cpp were not read in depth. Their coverage is limited to the grep hits above, so they may hide issues that did not show up in the pattern searches. • The plan marked Step 1 and Step 2 complete, but the review of those files was partial. Treat the list above as a starting point, not an exhaustive audit. Nothing in the project was modified.