From 9ffd169aa7d5a8e4aa4d86cca0353f0a8c5f84d6 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Mon, 20 Jul 2020 18:11:43 -0400 Subject: [PATCH] Various unitary fixes (#622) * some comments * No intermediate arrays (of arrays) of strings for query results... ... instead, let any query pass its own lambda to collect row data directly however it needs to, for a bit of efficiency. Also the precautions of a new GuardedCall --- src/ProjectFileIO.cpp | 133 ++++++++++++++++++++---------------------- src/ProjectFileIO.h | 16 ++--- 2 files changed, 70 insertions(+), 79 deletions(-) diff --git a/src/ProjectFileIO.cpp b/src/ProjectFileIO.cpp index 5724a49d3..589a23e7b 100644 --- a/src/ProjectFileIO.cpp +++ b/src/ProjectFileIO.cpp @@ -107,12 +107,15 @@ static const char *ProjectFileSchema = // 'samples' are fixed size blocks of int16, int32 or float32 numbers. // The blocks may be partially empty. // The quantity of valid data in the blocks is - // provided in the project XML. + // provided in the project blob. // // sampleformat specifies the format of the samples stored. // // blockID is a 64 bit number. // + // Rows are immutable -- never updated after addition, but may be + // deleted. + // // summin to summary64K are summaries at 3 distance scales. "CREATE TABLE IF NOT EXISTS .sampleblocks" "(" @@ -635,52 +638,45 @@ bool ProjectFileIO::TransactionRollback(const wxString &name) return rc == SQLITE_OK; } -/* static */ -int ProjectFileIO::ExecCallback(void *data, int cols, char **vals, char **names) +static int ExecCallback(void *data, int cols, char **vals, char **names) { - ExecParm *parms = static_cast(data); - return parms->func(parms->result, cols, vals, names); + auto &cb = *static_cast(data); + // Be careful not to throw anything across sqlite3's stack frames. + return GuardedCall( + [&]{ return cb(cols, vals, names); }, + MakeSimpleGuard( 1 ) + ); } -int ProjectFileIO::Exec(const char *query, ExecCB callback, ExecResult &result) +int ProjectFileIO::Exec(const char *query, const ExecCB &callback) { char *errmsg = nullptr; - ExecParm ep = {callback, result}; - int rc = sqlite3_exec(DB(), query, ExecCallback, &ep, &errmsg); + const void *ptr = &callback; + int rc = sqlite3_exec(DB(), query, ExecCallback, + const_cast(ptr), &errmsg); - if (errmsg) + if (rc != SQLITE_ABORT && errmsg) { SetDBError( XO("Failed to execute a project file command:\n\n%s").Format(query) ); mLibraryError = Verbatim(errmsg); + } + if (errmsg) + { sqlite3_free(errmsg); } return rc; } -bool ProjectFileIO::Query(const char *sql, ExecResult &result) +bool ProjectFileIO::Query(const char *sql, const ExecCB &callback) { - result.clear(); - - auto getresult = [](ExecResult &result, int cols, char **vals, char **names) - { - std::vector row; - - for (int i = 0; i < cols; ++i) - { - row.push_back(vals[i]); - } - - result.push_back(row); - - return SQLITE_OK; - }; - - int rc = Exec(sql, getresult, result); - if (rc != SQLITE_OK) + int rc = Exec(sql, callback); + // SQLITE_ABORT is a non-error return only meaning the callback + // stopped the iteration of rows early + if ( !(rc == SQLITE_OK || rc == SQLITE_ABORT) ) { return false; } @@ -690,21 +686,16 @@ bool ProjectFileIO::Query(const char *sql, ExecResult &result) bool ProjectFileIO::GetValue(const char *sql, wxString &result) { + // Retrieve the first column in the first row, if any result.clear(); + auto cb = [&result](int cols, char **vals, char **){ + if (cols > 0) + result = vals[0]; + // Stop after one row + return 1; + }; - ExecResult holder; - if (!Query(sql, holder)) - { - return false; - } - - // Return the first column in the first row, if any - if (holder.size() && holder[0].size()) - { - result.assign(holder[0][0]); - } - - return true; + return Query(sql, cb); } bool ProjectFileIO::GetBlob(const char *sql, wxMemoryBuffer &buffer) @@ -929,19 +920,17 @@ sqlite3 *ProjectFileIO::CopyTo(const FilePath &destpath, // Collect ALL blockids else { - ExecResult holder; - if (!Query("SELECT blockid FROM sampleblocks;", holder)) + auto cb = [&blockids](int cols, char **vals, char **){ + SampleBlockID blockid; + wxString{ vals[0] }.ToLongLong(&blockid); + blockids.insert(blockid); + return 0; + }; + + if (!Query("SELECT blockid FROM sampleblocks;", cb)) { return nullptr; } - - for (auto block : holder) - { - SampleBlockID blockid; - block[0].ToLongLong(&blockid); - - blockids.insert(blockid); - } } // Create the project doc @@ -1152,26 +1141,32 @@ bool ProjectFileIO::ShouldVacuum(const std::shared_ptr &tracks) } // Get the number of blocks and total length from the project file. - ExecResult holder; - if (!Query("SELECT Count(*), Sum(Length(summary256)) + Sum(Length(summary64k)) + Sum(Length(samples)) FROM sampleblocks;", holder)) - { - // Shouldn't vacuum since we don't have the full picture - return false; - } - - // Verify we got the results we asked for - if (holder.size() != 1 || holder[0].size() != 2) - { - // Shouldn't vacuum since we don't have the full picture - return false; - } - - // Convert unsigned long long blockcount = 0; - holder[0][0].ToULongLong(&blockcount); - unsigned long long total = 0; - holder[0][1].ToULongLong(&total); + + auto cb = + [&blockcount, &total](int cols, char **vals, char **){ + if ( cols != 2 ) + // Should have two exactly! + return 1; + if ( total > 0 ) { + // Should not have multiple rows! + total = 0; + return 1; + } + // Convert + wxString{ vals[0] }.ToULongLong( &blockcount ); + wxString{ vals[1] }.ToULongLong( &total ); + return 0; + }; + if (!Query("SELECT Count(*), " + "Sum(Length(summary256)) + Sum(Length(summary64k)) + Sum(Length(samples)) " + "FROM sampleblocks;", cb) + || total == 0) + { + // Shouldn't vacuum since we don't have the full picture + return false; + } // Remember if we had unused blocks in the project file mHadUnused = (blockcount > active.size()); diff --git a/src/ProjectFileIO.h b/src/ProjectFileIO.h index b839e0d2e..767ff49bc 100644 --- a/src/ProjectFileIO.h +++ b/src/ProjectFileIO.h @@ -120,6 +120,10 @@ public: bool TransactionCommit(const wxString &name); bool TransactionRollback(const wxString &name); + // Type of function that is given the fields of one row and returns + // 0 for success or non-zero to stop the query + using ExecCB = std::function; + private: void WriteXMLHeader(XMLWriter &xmlFile) const; void WriteXML(XMLWriter &xmlFile, bool recording = false, const std::shared_ptr &tracks = nullptr) /* not override */; @@ -130,15 +134,7 @@ private: void UpdatePrefs() override; - using ExecResult = std::vector>; - using ExecCB = std::function; - struct ExecParm - { - ExecCB func; - ExecResult &result; - }; - static int ExecCallback(void *data, int cols, char **vals, char **names); - int Exec(const char *query, ExecCB callback, ExecResult &result); + int Exec(const char *query, const ExecCB &callback); // The opening of the database may be delayed until demanded. // Returns a non-null pointer to an open database, or throws an exception @@ -165,7 +161,7 @@ private: bool CloseDB(); bool DeleteDB(); - bool Query(const char *sql, ExecResult &result); + bool Query(const char *sql, const ExecCB &callback); bool GetValue(const char *sql, wxString &value); bool GetBlob(const char *sql, wxMemoryBuffer &buffer);