Merge branch 'main' of git@github.com:mixxxdj/mixxx.git into track-metadata-sync

6 files changed, 73 insertions, 59 deletions

diff --git a/src/library/trackset/crate/cratestorage.cpp b/src/library/trackset/crate/cratestorage.cpp
index ee50e37fdf..a5a3da13d1 100644
--- a/src/library/trackset/crate/cratestorage.cpp
+++ b/src/library/trackset/crate/cratestorage.cpp
@@ -52,12 +52,11 @@ const QString kCrateSummaryViewQuery =
                         CRATE_TABLE,
                         CRATETABLE_ID);
 
-class CrateQueryBinder {
+class CrateQueryBinder final {
   public:
     explicit CrateQueryBinder(FwdSqlQuery& query)
             : m_query(query) {
     }
-    virtual ~CrateQueryBinder() = default;
 
     void bindId(const QString& placeholder, const Crate& crate) const {
         m_query.bindValue(placeholder, crate.getId());
diff --git a/src/mixer/playermanager.cpp b/src/mixer/playermanager.cpp
index 4a2675fdde..bf981f6664 100644
--- a/src/mixer/playermanager.cpp
+++ b/src/mixer/playermanager.cpp
@@ -31,18 +31,28 @@ const mixxx::Logger kLogger("PlayerManager");
 // Utilize half of the available cores for adhoc analysis of tracks
 const int kNumberOfAnalyzerThreads = math_max(1, QThread::idealThreadCount() / 2);
 
-const QRegularExpression kDeckRegex(QStringLiteral("\\[Channel\\d+\\]"));
-const QRegularExpression kSamplerRegex(QStringLiteral("\\[Sampler\\d+\\]"));
-const QRegularExpression kPreviewDeckRegex(QStringLiteral("\\[PreviewDeck\\d+\\]"));
+const QRegularExpression kDeckRegex(QStringLiteral("^\\[Channel(\\d+)\\]$"));
+const QRegularExpression kSamplerRegex(QStringLiteral("^\\[Sampler(\\d+)\\]$"));
+const QRegularExpression kPreviewDeckRegex(QStringLiteral("^\\[PreviewDeck(\\d+)\\]$"));
 
 bool extractIntFromRegex(const QRegularExpression& regex, const QString& group, int* number) {
-    QRegularExpressionMatch match = regex.match(group);
+    const QRegularExpressionMatch match = regex.match(group);
+    DEBUG_ASSERT(match.isValid());
     if (!match.hasMatch()) {
         return false;
     }
+    // The regex is expected to contain a single capture group with the number
+    constexpr int capturedNumberIndex = 1;
+    DEBUG_ASSERT(match.lastCapturedIndex() <= capturedNumberIndex);
+    if (match.lastCapturedIndex() < capturedNumberIndex) {
+        qWarning() << "No number found in group" << group;
+        return false;
+    }
     if (number) {
+        const QString capturedNumber = match.captured(capturedNumberIndex);
+        DEBUG_ASSERT(!capturedNumber.isNull());
         bool okay = false;
-        const int numberFromMatch = match.captured(1).toInt(&okay);
+        const int numberFromMatch = capturedNumber.toInt(&okay);
         VERIFY_OR_DEBUG_ASSERT(okay) {
             return false;
         }
diff --git a/src/util/db/fwdsqlquery.h b/src/util/db/fwdsqlquery.h
index 5a08a04ba3..1ae1d864fd 100644
--- a/src/util/db/fwdsqlquery.h
+++ b/src/util/db/fwdsqlquery.h
@@ -14,22 +14,26 @@
 class SqlQueryFinisher;
 class FwdSqlQuerySelectResult;
 
-// A forward-only QSqlQuery that is prepared immediately
-// during initialization. It offers a limited set of functions
-// from QSqlQuery.
-//
-// Setting QSqlQuery to forward-only causes memory savings since
-// QSqlCachedResult (what QtSQLite uses) won't allocate a giant
-// in-memory table that we won't use at all when invoking only
-// QSqlQuery::next() to iterate over the results.
-//
-// Prefer to use this derived class instead of QSqlQuery to avoid
-// performance bottlenecks and for implicit logging failed query
-// executions.
-//
-// Please note that forward-only queries don't provide information
-// about the size of the result set!
-class FwdSqlQuery : protected QSqlQuery {
+/// A forward-only QSqlQuery that is prepared immediately
+/// during initialization. It offers a limited set of functions
+/// from QSqlQuery.
+///
+/// Setting QSqlQuery to forward-only causes memory savings since
+/// QSqlCachedResult (what QtSQLite uses) won't allocate a giant
+/// in-memory table that we won't use at all when invoking only
+/// QSqlQuery::next() to iterate over the results.
+///
+/// Prefer to use this derived class instead of QSqlQuery to avoid
+/// performance bottlenecks and for implicit logging failed query
+/// executions.
+///
+/// Please note that forward-only queries don't provide information
+/// about the size of the result set!
+///
+/// Since the destructor of the base class QSqlQuery is non-virtual
+/// FwdSqlQuery must not contain any members with a non-trivial
+/// destructor to prevent memory leaks!
+class FwdSqlQuery final : protected QSqlQuery {
     friend class SqlQueryFinisher;
     friend class FwdSqlQuerySelectResult;
 
@@ -40,6 +44,11 @@ class FwdSqlQuery : protected QSqlQuery {
     FwdSqlQuery(
             const QSqlDatabase& database,
             const QString& statement);
+    // The copy constructor of QSqlQuery is marked as deprecated in Qt6
+    FwdSqlQuery(const FwdSqlQuery&) = delete;
+    FwdSqlQuery(FwdSqlQuery&&) = default;
+
+    FwdSqlQuery& operator=(FwdSqlQuery&&) = default;
 
     bool isPrepared() const {
         return m_prepared;
diff --git a/src/util/db/fwdsqlqueryselectresult.cpp b/src/util/db/fwdsqlqueryselectresult.cpp
index 3543756825..0b0c52c270 100644
--- a/src/util/db/fwdsqlqueryselectresult.cpp
+++ b/src/util/db/fwdsqlqueryselectresult.cpp
@@ -1,8 +1,7 @@
 #include "util/db/fwdsqlqueryselectresult.h"
 
-
 FwdSqlQuerySelectResult::FwdSqlQuerySelectResult()
-    : m_queryFinisher(m_query) {
+        : m_queryFinisher(&m_query) {
     DEBUG_ASSERT(!m_query.isActive());
 }
 
@@ -12,11 +11,11 @@ FwdSqlQuerySelectResult::FwdSqlQuerySelectResult()
 // be less efficient than actually moving the object's contents, but
 // meets all requirements.
 FwdSqlQuerySelectResult::FwdSqlQuerySelectResult(FwdSqlQuery&& query)
-    : m_query(std::move(query)),
-      // Pass a reference to the member to m_queryFinisher, because
-      // the contents of the r-value reference parameter might have
-      // been moved!
-      m_queryFinisher(m_query) {
+        : m_query(std::move(query)),
+          // Pass a reference to the member to m_queryFinisher, because
+          // the contents of the r-value reference parameter might have
+          // been moved!
+          m_queryFinisher(&m_query) {
     DEBUG_ASSERT(m_query.isActive());
     DEBUG_ASSERT(m_query.isSelect());
     // Verify that the query has just been executed and that
@@ -37,6 +36,6 @@ FwdSqlQuery FwdSqlQuerySelectResult::release() {
     FwdSqlQuery query(std::move(m_query));
     // Ensure that the wrapped query is inaccessible after moving it!
     m_query = FwdSqlQuery();
-    m_queryFinisher.release();
+    DEBUG_ASSERT(!m_query.isActive());
     return query;
 }
diff --git a/src/util/db/sqlqueryfinisher.cpp b/src/util/db/sqlqueryfinisher.cpp
index 2b9a40b5d1..7a46a36e09 100644
--- a/src/util/db/sqlqueryfinisher.cpp
+++ b/src/util/db/sqlqueryfinisher.cpp
@@ -1,19 +1,10 @@
 #include "util/db/sqlqueryfinisher.h"
 
-#include "util/assert.h"
-
-
-bool SqlQueryFinisher::finish() {
-    if (m_query.isActive()) {
-        m_query.finish();
-        release();
-        return true;
-    } else {
+bool SqlQueryFinisher::tryFinish() {
+    if (!m_pQuery || !m_pQuery->isActive()) {
         return false;
     }
-}
-
-void SqlQueryFinisher::release() {
-    m_query = QSqlQuery();
-    DEBUG_ASSERT(!m_query.isActive());
+    m_pQuery->finish();
+    m_pQuery = nullptr;
+    return true;
 }
diff --git a/src/util/db/sqlqueryfinisher.h b/src/util/db/sqlqueryfinisher.h
index a6fc06902c..8cc19087b8 100644
--- a/src/util/db/sqlqueryfinisher.h
+++ b/src/util/db/sqlqueryfinisher.h
@@ -2,32 +2,38 @@
 
 #include <QSqlQuery>
 
+#include "util/db/fwdsqlquery.h"
 
-// Ensures that reusable queries are properly finished when
-// leaving the corresponding execution scope. This will free
-// resources of prepared statements until the next execution.
+/// Ensures that reusable queries are properly finished when
+/// leaving the corresponding execution scope. This will free
+/// resources of prepared statements until the next execution.
 class SqlQueryFinisher final {
-public:
-  explicit SqlQueryFinisher(const QSqlQuery& query)
-          : m_query(query) { // implicitly shared (not copied)
-  }
+  public:
+    explicit SqlQueryFinisher(QSqlQuery* pQuery)
+            : m_pQuery(pQuery) {
+    }
+    explicit SqlQueryFinisher(FwdSqlQuery* pQuery)
+            : m_pQuery(pQuery) {
+    }
     SqlQueryFinisher(SqlQueryFinisher&& other)
-        : m_query(std::move(other.m_query)) { // implicitly shared (not moved)
-        other.release();
+            : m_pQuery(other.m_pQuery) {
+        other.m_pQuery = nullptr;
     }
     ~SqlQueryFinisher() {
-        finish();
+        tryFinish();
     }
 
-    bool finish();
+    bool tryFinish();
 
-    void release();
+    void release() {
+        m_pQuery = nullptr;
+    }
 
-private:
+  private:
     // Disable copy construction and copy/move assignment
     SqlQueryFinisher(const SqlQueryFinisher&) = delete;
     SqlQueryFinisher& operator=(const SqlQueryFinisher&) = delete;
     SqlQueryFinisher& operator=(SqlQueryFinisher&&) = delete;
 
-    QSqlQuery m_query; // implicitly shared
+    QSqlQuery* m_pQuery;
 };