From: Fred Morcos Date: Mon, 27 Nov 2023 13:37:25 +0000 (+0100) Subject: Cleanup ssqlite3 X-Git-Tag: rec-5.0.0-rc1~15^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7e5efe3c268f8981e23fb9116fc29d02e66b02e4;p=thirdparty%2Fpdns.git Cleanup ssqlite3 --- diff --git a/pdns/ssqlite3.cc b/pdns/ssqlite3.cc index 9dc4888cc5..660c561b78 100644 --- a/pdns/ssqlite3.cc +++ b/pdns/ssqlite3.cc @@ -27,6 +27,7 @@ #include "ssqlite3.hh" #include #include +#include #include "pdns/logger.hh" #include "misc.hh" #include "utility.hh" @@ -49,21 +50,26 @@ static int pdns_sqlite3_clear_bindings(sqlite3_stmt* pStmt) } #endif -static string SSQLite3ErrorString(sqlite3* db) +static string SSQLite3ErrorString(sqlite3* database) { - return string(sqlite3_errmsg(db) + string(" (") + std::to_string(sqlite3_extended_errcode(db)) + string(")")); + return string(sqlite3_errmsg(database) + string(" (") + std::to_string(sqlite3_extended_errcode(database)) + string(")")); } class SSQLite3Statement : public SSqlStatement { public: - SSQLite3Statement(SSQLite3* db, bool dolog, const string& query) : - d_query(query), - d_db(db), + SSQLite3Statement(SSQLite3* database, bool dolog, string query) : + d_query(std::move(query)), + d_db(database), d_dolog(dolog) { } + SSQLite3Statement(const SSQLite3Statement&) = delete; + SSQLite3Statement(SSQLite3Statement&&) = delete; + SSQLite3Statement& operator=(const SSQLite3Statement&) = delete; + SSQLite3Statement& operator=(SSQLite3Statement&&) = delete; + int name2idx(const string& name) { string zName = string(":") + name; @@ -72,7 +78,7 @@ public: // XXX: support @ and $? } - SSqlStatement* bind(const string& name, bool value) + SSqlStatement* bind(const string& name, bool value) override { int idx = name2idx(name); if (idx > 0) { @@ -80,7 +86,8 @@ public: }; return this; } - SSqlStatement* bind(const string& name, int value) + + SSqlStatement* bind(const string& name, int value) override { int idx = name2idx(name); if (idx > 0) { @@ -88,7 +95,8 @@ public: }; return this; } - SSqlStatement* bind(const string& name, uint32_t value) + + SSqlStatement* bind(const string& name, uint32_t value) override { int idx = name2idx(name); if (idx > 0) { @@ -96,7 +104,8 @@ public: }; return this; } - SSqlStatement* bind(const string& name, long value) + + SSqlStatement* bind(const string& name, long value) override { int idx = name2idx(name); if (idx > 0) { @@ -104,15 +113,17 @@ public: }; return this; } - SSqlStatement* bind(const string& name, unsigned long value) + + SSqlStatement* bind(const string& name, unsigned long value) override { int idx = name2idx(name); if (idx > 0) { - sqlite3_bind_int64(d_stmt, idx, value); + sqlite3_bind_int64(d_stmt, idx, static_cast(value)); }; return this; } - SSqlStatement* bind(const string& name, long long value) + + SSqlStatement* bind(const string& name, long long value) override { int idx = name2idx(name); if (idx > 0) { @@ -120,23 +131,26 @@ public: }; return this; }; - SSqlStatement* bind(const string& name, unsigned long long value) + + SSqlStatement* bind(const string& name, unsigned long long value) override { int idx = name2idx(name); if (idx > 0) { - sqlite3_bind_int64(d_stmt, idx, value); + sqlite3_bind_int64(d_stmt, idx, static_cast(value)); }; return this; } - SSqlStatement* bind(const string& name, const std::string& value) + + SSqlStatement* bind(const string& name, const std::string& value) override { int idx = name2idx(name); if (idx > 0) { - sqlite3_bind_text(d_stmt, idx, value.c_str(), value.size(), SQLITE_TRANSIENT); + sqlite3_bind_text(d_stmt, idx, value.c_str(), static_cast(value.size()), SQLITE_TRANSIENT); }; return this; } - SSqlStatement* bindNull(const string& name) + + SSqlStatement* bindNull(const string& name) override { int idx = name2idx(name); if (idx > 0) { @@ -145,37 +159,42 @@ public: return this; } - SSqlStatement* execute() + SSqlStatement* execute() override { prepareStatement(); if (d_dolog) { - g_log << Logger::Warning << "Query " << ((long)(void*)this) << ": " << d_query << endl; + g_log << Logger::Warning << "Query " << this << ": " << d_query << endl; d_dtime.set(); } - int attempts = d_db->inTransaction(); // try only once - while (attempts < 2 && (d_rc = sqlite3_step(d_stmt)) == SQLITE_BUSY) + + int attempts = d_db->inTransaction() ? 1 : 0; // try only once + while (attempts < 2 && (d_rc = sqlite3_step(d_stmt)) == SQLITE_BUSY) { attempts++; + } if (d_rc != SQLITE_ROW && d_rc != SQLITE_DONE) { // failed. releaseStatement(); - if (d_rc == SQLITE_CANTOPEN) + if (d_rc == SQLITE_CANTOPEN) { throw SSqlException(string("CANTOPEN error in sqlite3, often caused by unwritable sqlite3 db *directory*: ") + SSQLite3ErrorString(d_db->db())); + } throw SSqlException(string("Error while retrieving SQLite query results: ") + SSQLite3ErrorString(d_db->db())); } - if (d_dolog) - g_log << Logger::Warning << "Query " << ((long)(void*)this) << ": " << d_dtime.udiffNoReset() << " us to execute" << endl; + if (d_dolog) { + g_log << Logger::Warning << "Query " << this << ": " << d_dtime.udiffNoReset() << " us to execute" << endl; + } return this; } - bool hasNextRow() + + bool hasNextRow() override { if (d_dolog && d_rc != SQLITE_ROW) { - g_log << Logger::Warning << "Query " << ((long)(void*)this) << ": " << d_dtime.udiffNoReset() << " us total to last row" << endl; + g_log << Logger::Warning << "Query " << this << ": " << d_dtime.udiffNoReset() << " us total to last row" << endl; } return d_rc == SQLITE_ROW; } - SSqlStatement* nextRow(row_t& row) + SSqlStatement* nextRow(row_t& row) override { row.clear(); int numCols = sqlite3_column_count(d_stmt); @@ -186,6 +205,7 @@ public: row.emplace_back(""); } else { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast): SQLite API returns unsigned char strings and std::strings are signed char. const char* pData = (const char*)sqlite3_column_text(d_stmt, i); row.emplace_back(pData, sqlite3_column_bytes(d_stmt, i)); } @@ -194,7 +214,7 @@ public: return this; } - SSqlStatement* getResult(result_t& result) + SSqlStatement* getResult(result_t& result) override { result.clear(); while (hasNextRow()) { @@ -205,7 +225,7 @@ public: return this; } - SSqlStatement* reset() + SSqlStatement* reset() override { sqlite3_reset(d_stmt); #if SQLITE_VERSION_NUMBER >= 3003009 @@ -216,13 +236,13 @@ public: return this; } - ~SSQLite3Statement() + ~SSQLite3Statement() override { // deallocate if necessary releaseStatement(); } - const string& getQuery() { return d_query; }; + const string& getQuery() override { return d_query; }; private: string d_query; @@ -235,10 +255,11 @@ private: void prepareStatement() { - const char* pTail; + const char* pTail = nullptr; - if (d_prepared) + if (d_prepared) { return; + } #if SQLITE_VERSION_NUMBER >= 3003009 if (sqlite3_prepare_v2(d_db->db(), d_query.c_str(), -1, &d_stmt, &pTail) != SQLITE_OK) #else @@ -248,15 +269,17 @@ private: releaseStatement(); throw SSqlException(string("Unable to compile SQLite statement : '") + d_query + "': " + SSQLite3ErrorString(d_db->db())); } - if (pTail && strlen(pTail) > 0) + if ((pTail != nullptr) && strlen(pTail) > 0) { g_log << Logger::Warning << "Sqlite3 command partially processed. Unprocessed part: " << pTail << endl; + } d_prepared = true; } void releaseStatement() { - if (d_stmt) + if (d_stmt != nullptr) { sqlite3_finalize(d_stmt); + } d_stmt = nullptr; d_prepared = false; } @@ -266,22 +289,26 @@ private: SSQLite3::SSQLite3(const std::string& database, const std::string& journalmode, bool creat) { if (access(database.c_str(), F_OK) == -1) { - if (!creat) + if (!creat) { throw sPerrorException("SQLite database '" + database + "' does not exist yet"); + } } else { - if (creat) + if (creat) { throw sPerrorException("SQLite database '" + database + "' already exists"); + } } - if (sqlite3_open(database.c_str(), &m_pDB) != SQLITE_OK) + if (sqlite3_open(database.c_str(), &m_pDB) != SQLITE_OK) { throw sPerrorException("Could not connect to the SQLite database '" + database + "'"); - m_dolog = 0; + } + m_dolog = false; m_in_transaction = false; - sqlite3_busy_handler(m_pDB, busyHandler, 0); + sqlite3_busy_handler(m_pDB, busyHandler, nullptr); - if (journalmode.length()) + if (journalmode.length() != 0) { execute("PRAGMA journal_mode=" + journalmode); + } } void SSQLite3::setLog(bool state) @@ -292,16 +319,17 @@ void SSQLite3::setLog(bool state) // Destructor. SSQLite3::~SSQLite3() { - int ret; - for (int n = 0;; ++n) { - if ((ret = sqlite3_close(m_pDB)) != SQLITE_OK) { - if (n || ret != SQLITE_BUSY) { // if we have SQLITE_BUSY, and a working m_Pstmt, try finalize + for (int tries = 0;; ++tries) { + int ret = sqlite3_close(m_pDB); + if (ret != SQLITE_OK) { + if (tries != 0 || ret != SQLITE_BUSY) { // if we have SQLITE_BUSY, and a working m_Pstmt, try finalize cerr << "Unable to close down sqlite connection: " << ret << endl; abort(); } } - else + else { break; + } } } @@ -312,33 +340,31 @@ std::unique_ptr SSQLite3::prepare(const string& query, int nparam void SSQLite3::execute(const string& query) { - char* errmsg; + char* errmsg = nullptr; std::string errstr1; - int rc = sqlite3_exec(m_pDB, query.c_str(), nullptr, nullptr, &errmsg); - if (rc != SQLITE_OK) { + int execRet = sqlite3_exec(m_pDB, query.c_str(), nullptr, nullptr, &errmsg); + if (execRet != SQLITE_OK) { errstr1 = errmsg; sqlite3_free(errmsg); } - if (rc == SQLITE_BUSY) { + if (execRet == SQLITE_BUSY) { if (m_in_transaction) { throw SSqlException("Failed to execute query: " + errstr1); } - else { - rc = sqlite3_exec(m_pDB, query.c_str(), NULL, NULL, &errmsg); - std::string errstr2; - if (rc != SQLITE_OK) { - errstr2 = errmsg; - sqlite3_free(errmsg); - throw SSqlException("Failed to execute query: " + errstr2); - } + execRet = sqlite3_exec(m_pDB, query.c_str(), nullptr, nullptr, &errmsg); + std::string errstr2; + if (execRet != SQLITE_OK) { + errstr2 = errmsg; + sqlite3_free(errmsg); + throw SSqlException("Failed to execute query: " + errstr2); } } - else if (rc != SQLITE_OK) { + else if (execRet != SQLITE_OK) { throw SSqlException("Failed to execute query: " + errstr1); } } -int SSQLite3::busyHandler(void*, int) +int SSQLite3::busyHandler([[maybe_unused]] void* userData, [[maybe_unused]] int invocationsSoFar) { Utility::usleep(1000); return 1; @@ -365,5 +391,5 @@ void SSQLite3::commit() // Constructs a SSqlException object. SSqlException SSQLite3::sPerrorException(const std::string& reason) { - return SSqlException(reason); + return {reason}; } diff --git a/pdns/ssqlite3.hh b/pdns/ssqlite3.hh index 6c929d9d43..29a1e10c9e 100644 --- a/pdns/ssqlite3.hh +++ b/pdns/ssqlite3.hh @@ -39,7 +39,7 @@ public: SSQLite3(const std::string& database, const std::string& journalmode, bool creat = false); //! Destructor. - ~SSQLite3(); + ~SSQLite3() override; std::unique_ptr prepare(const string& query, int nparams) override; void execute(const string& query) override; @@ -51,7 +51,7 @@ public: sqlite3* db() { return this->m_pDB; }; - bool inTransaction() { return m_in_transaction; }; + [[nodiscard]] bool inTransaction() const { return m_in_transaction; }; //! Used to create an backend specific exception message. SSqlException sPerrorException(const std::string& reason) override;