Sqlite: Improve ownership

Using the compile option instead of a function pointer makes the code
less brittle.

Change-Id: Id7f3b42c044d47f13e099f50c5bb33b72c05cde1
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

src/libs/sqlite/sqlitebasestatement.cpp

@@ -27,17 +27,11 @@ extern "C" int sqlite3_carray_bind(
 namespace Sqlite {
 
 BaseStatement::BaseStatement(Utils::SmallStringView sqlStatement, Database &database)
-    : m_compiledStatement(nullptr, deleteCompiledStatement)
+    : m_database(database)
-    , m_database(database)
 {
     prepare(sqlStatement);
 }
 
-void BaseStatement::deleteCompiledStatement(sqlite3_stmt *compiledStatement)
-{
-    sqlite3_finalize(compiledStatement);
-}
-
 class UnlockNotification
 {
 public:
@@ -485,4 +479,9 @@ ValueView BaseStatement::fetchValueView(int column) const
     return ValueView::create(NullValue{});
 }
 
+void BaseStatement::Deleter::operator()(sqlite3_stmt *statement)
+{
+    sqlite3_finalize(statement);
+}
+
 } // namespace Sqlite

src/libs/sqlite/sqlitebasestatement.h

@@ -50,8 +50,6 @@ public:
     BaseStatement(const BaseStatement &) = delete;
     BaseStatement &operator=(const BaseStatement &) = delete;
 
-    static void deleteCompiledStatement(sqlite3_stmt *m_compiledStatement);
-
     bool next() const;
     void step() const;
     void reset() const noexcept;
@@ -123,7 +121,13 @@ protected:
     ~BaseStatement() = default;
 
 private:
-    std::unique_ptr<sqlite3_stmt, void (*)(sqlite3_stmt *)> m_compiledStatement;
+    struct Deleter
+    {
+        SQLITE_EXPORT void operator()(sqlite3_stmt *statement);
+    };
+
+private:
+    std::unique_ptr<sqlite3_stmt, Deleter> m_compiledStatement;
     Database &m_database;
 };
 

src/libs/sqlite/sqlitedatabasebackend.cpp

@@ -90,14 +90,16 @@ void DatabaseBackend::open(Utils::SmallStringView databaseFilePath,
 {
     checkCanOpenDatabase(databaseFilePath);
 
+    sqlite3 *handle = m_databaseHandle.get();
     int resultCode = sqlite3_open_v2(std::string(databaseFilePath).c_str(),
-                                     &m_databaseHandle,
+                                     &handle,
                                      createOpenFlags(openMode, journalMode),
                                      nullptr);
+    m_databaseHandle.reset(handle);
 
     checkDatabaseCouldBeOpened(resultCode);
 
-    resultCode = sqlite3_carray_init(m_databaseHandle, nullptr, nullptr);
+    resultCode = sqlite3_carray_init(m_databaseHandle.get(), nullptr, nullptr);
 
     checkCarrayCannotBeIntialized(resultCode);
 }
@@ -105,7 +107,7 @@ void DatabaseBackend::open(Utils::SmallStringView databaseFilePath,
 sqlite3 *DatabaseBackend::sqliteDatabaseHandle() const
 {
     checkDatabaseHandleIsNotNull();
-    return m_databaseHandle;
+    return m_databaseHandle.get();
 }
 
 void DatabaseBackend::setPragmaValue(Utils::SmallStringView pragmaKey, Utils::SmallStringView newPragmaValue)
@@ -216,12 +218,11 @@ void DatabaseBackend::close()
 {
     checkForOpenDatabaseWhichCanBeClosed();
 
-    int resultCode = sqlite3_close(m_databaseHandle);
+    int resultCode = sqlite3_close(m_databaseHandle.get());
 
     checkDatabaseClosing(resultCode);
 
-    m_databaseHandle = nullptr;
+    m_databaseHandle.release();
-
 }
 
 bool DatabaseBackend::databaseIsOpen() const
@@ -231,12 +232,7 @@ bool DatabaseBackend::databaseIsOpen() const
 
 void DatabaseBackend::closeWithoutException()
 {
-    if (m_databaseHandle) {
+    m_databaseHandle.reset();
-        int resultCode = sqlite3_close_v2(m_databaseHandle);
-        m_databaseHandle = nullptr;
-        if (resultCode != SQLITE_OK)
-            qWarning() << "SqliteDatabaseBackend::closeWithoutException: Unexpected error at closing the database!";
-    }
 }
 
 namespace {
@@ -414,12 +410,12 @@ int DatabaseBackend::createOpenFlags(OpenMode openMode, JournalMode journalMode)
 
 void DatabaseBackend::setBusyTimeout(std::chrono::milliseconds timeout)
 {
-    sqlite3_busy_timeout(m_databaseHandle, int(timeout.count()));
+    sqlite3_busy_timeout(m_databaseHandle.get(), int(timeout.count()));
 }
 
 void DatabaseBackend::walCheckpointFull()
 {
-    int resultCode = sqlite3_wal_checkpoint_v2(m_databaseHandle,
+    int resultCode = sqlite3_wal_checkpoint_v2(m_databaseHandle.get(),
                                                nullptr,
                                                SQLITE_CHECKPOINT_TRUNCATE,
                                                nullptr,
@@ -433,12 +429,12 @@ void DatabaseBackend::setUpdateHook(
     void *object,
     void (*callback)(void *object, int, char const *database, char const *, long long rowId))
 {
-    sqlite3_update_hook(m_databaseHandle, callback, object);
+    sqlite3_update_hook(m_databaseHandle.get(), callback, object);
 }
 
 void DatabaseBackend::resetUpdateHook()
 {
-    sqlite3_update_hook(m_databaseHandle, nullptr, nullptr);
+    sqlite3_update_hook(m_databaseHandle.get(), nullptr, nullptr);
 }
 
 void DatabaseBackend::setBusyHandler(DatabaseBackend::BusyHandler &&busyHandler)
@@ -491,4 +487,14 @@ Type DatabaseBackend::toValue(Utils::SmallStringView sqlStatement) const
     }
 }
 
+void DatabaseBackend::Deleter::operator()(sqlite3 *database)
+{
+    if (database) {
+        int resultCode = sqlite3_close_v2(database);
+        if (resultCode != SQLITE_OK)
+            qWarning() << "SqliteDatabaseBackend::closeWithoutException: Unexpected error at "
+                          "closing the database!";
+    }
+}
+
 } // namespace Sqlite

src/libs/sqlite/sqlitedatabasebackend.h

@@ -108,9 +108,15 @@ protected:
     static Utils::SmallStringView journalModeToPragma(JournalMode journalMode);
     static JournalMode pragmaToJournalMode(Utils::SmallStringView pragma);
 
+private:
+    struct Deleter
+    {
+        SQLITE_EXPORT void operator()(sqlite3 *database);
+    };
+
 private:
     Database &m_database;
-    sqlite3 *m_databaseHandle;
+    std::unique_ptr<sqlite3, Deleter> m_databaseHandle;
     BusyHandler m_busyHandler;
     ProgressHandler m_progressHandler;
 };

src/libs/sqlite/sqlitesessions.cpp

@@ -173,4 +173,9 @@ SessionChangeSets Sessions::changeSets() const
     return selectChangeSets.values<SessionChangeSet>(1024);
 }
 
+void Sessions::Deleter::operator()(sqlite3_session *session)
+{
+    sqlite3session_delete(session);
+}
+
 } // namespace Sqlite

src/libs/sqlite/sqlitesessions.h

@@ -8,10 +8,6 @@
 #include "sqlitesessionchangeset.h"
 #include "sqlitewritestatement.h"
 
-extern "C" {
-void sqlite3session_delete(sqlite3_session *pSession);
-};
-
 namespace Sqlite {
 
 namespace Internal {
@@ -44,7 +40,6 @@ public:
                             {"INSERT INTO ", sessionsTableName, "(changeset) VALUES(?)"}),
                         database}
         , databaseName(databaseName)
-        , session{nullptr, sqlite3session_delete}
     {}
     ~Sessions();
 
@@ -64,12 +59,17 @@ public:
 private:
     void attachTables(const Utils::SmallStringVector &tables);
 
+    struct Deleter
+    {
+        SQLITE_EXPORT void operator()(sqlite3_session *statement);
+    };
+
 public:
     Database &database;
     WriteStatement<1> insertSession;
     Utils::SmallString databaseName;
     Utils::SmallStringVector tableNames;
-    std::unique_ptr<sqlite3_session, decltype(&sqlite3session_delete)> session;
+    std::unique_ptr<sqlite3_session, Deleter> session;
 };
 
 } // namespace Sqlite