- Change KDbSqlResult* KDbConnection::executeSQL to QSharedPointer<KDbSqlResult> KDbConnection::prepareSql (fixes risks of memory leaks and unfinished execution when resulting object is ignored)
- Change bool KDbConnection::executeVoidSQL to bool KDbConnection::executeSql (also in Kexi)
- Change KDbConnection::insertRecord to return QSharedPointer<KDbSqlResult> (fixes risks of memory leaks and unfinished execution when resulting object is ignored)
- Change KDbConnection::recentSQLString() to recentSqlString()
- Change KDbSqlRecord* KDbSqlResult::fetchRecord() to QSharedPointer<KDbSqlRecord> fetchRecord()
- Change KDbSqlResult* KDbPreparedStatementInterface::execute() to QSharedPointer<KDbSqlResult> KDbPreparedStatementInterface::execute()
- Change KDb::lastInsertedAutoIncValue(KDbSqlResult *result, ...) to KDb::lastInsertedAutoIncValue(QSharedPointer<KDbSqlResult>, ...)
- Update drivers accordingly
- KDb & Kexi: Replace KDbConnection::executeVoidSQL uses with KDbConnection::executeSql and Replace KDbConnection::executeSQL uses with KDbConnection::prepareSql where needed (fix memory leak and unfinished execution)
Details
Run all autotests of KDb, build and try Kexi with sqlite/pgsql/mysql projects.
Diff Detail
- Repository
- R15 KDb
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
This is not a review, it's a few impressions :-)
src/KDb.h | ||
---|---|---|
128–130 | Shared pointer make sense when it's passed by value to increase ref count. | |
src/KDbConnection.cpp | ||
1074–1075 | Pointer is passed when argument can be optional i.e. nullptr, by reference when it can't be optional. Respecting this rule code base became cleaner. | |
src/drivers/mysql/MysqlPreparedStatement.cpp | ||
140 | sqlite bind in mysql driver - it looks quite strange :) It's not only here. |
src/KDb.h | ||
---|---|---|
128–130 | Thanks Anthony, definitely that was too mechanical change. | |
src/KDbConnection.cpp | ||
1074–1075 | What you say works but we're using the Qt API design guidelines. Like Q*Event* is used everywhere in QWidget::*Event(QEvent *) even while it's required. In other words, do you know a place in, say, QtSql and further in Qt or even KF5 that uses nonconst-references for the mentioned reason? Another topic is missing assertions or checks for such code. | |
src/drivers/mysql/MysqlPreparedStatement.cpp | ||
140 | Thanks, currently it's ifdefed out, KDB_USE_MYSQL_STMT is off. Code kept for reference here and changes made mechanically. |
src/KDb.h | ||
---|---|---|
128–130 | Done, the param is now optional. Feel free to review the other code :) |
src/KDbConnection.h | ||
---|---|---|
953 | why not returning a qsharedpointer here? |
src/KDbConnection.h | ||
---|---|---|
953 | I used to do that initially but since this is not a user API, for performance I decided to return raw pointer here. Ownership is returned here to the caller so it's a clear situation. |