diff options
author | fanquake <fanquake@gmail.com> | 2022-03-10 12:40:03 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-03-10 12:40:20 +0000 |
commit | 4f5d3ce5a059218ec03cf0b0d12e3e4026dc9939 (patch) | |
tree | 12b9ae8c0df929aa8a5aa744bc6c29b90e65d352 /src/wallet | |
parent | 76d44e832f5b9a86c5d37b583825f6ddb6096e7e (diff) | |
parent | 8ea6167099ddfe9a42464c3c8ae2acd895329d4f (diff) | |
download | bitcoin-4f5d3ce5a059218ec03cf0b0d12e3e4026dc9939.tar.xz |
Merge bitcoin/bitcoin#24486: wallet: refactor: dedup sqlite blob binding
8ea6167099ddfe9a42464c3c8ae2acd895329d4f wallet: refactor: dedup sqlite blob binding (Sebastian Falbesoner)
Pull request description:
This refactoring PR deduplicates repeated SQLite blob binding to a statement with a newly introduced helper function `BindBlobToStatement`, abstracting away the calls to `sqlite3_bind_blob(...)`.
This should be more readable and less error-prone, e.g. in case that the error handling has to be adapted. As a slight drawback, the function where the binding happens is not printed anymore (`__func__`), i.e. one could argue this is not strictly a refactoring, but IMHO the advantages of deduplication outweigh this; binding errors are purely internal logic errors (wrong use of the sqlite API) rather than something that is dependend on external data like DB content.
ACKs for top commit:
laanwj:
Code review ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f
achow101:
ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f
klementtan:
ACK 8ea6167099ddfe9a42464c3c8ae2acd895329d4f
Tree-SHA512: 1de0d214f836bc405a01e98a3a2d71f2deaddc7d23c31cad80219d1614bec92619c06d9a4a091dd563d3e95ffb879649c29745d8f89669b2a5330552c212af3f
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/sqlite.cpp | 67 |
1 files changed, 26 insertions, 41 deletions
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 2b2181e70b..55949e6e7a 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -37,6 +37,22 @@ static void ErrorLogCallback(void* arg, int code, const char* msg) LogPrintf("SQLite Error. Code: %d. Message: %s\n", code, msg); } +static bool BindBlobToStatement(sqlite3_stmt* stmt, + int index, + Span<const std::byte> blob, + const std::string& description) +{ + int res = sqlite3_bind_blob(stmt, index, blob.data(), blob.size(), SQLITE_STATIC); + if (res != SQLITE_OK) { + LogPrintf("Unable to bind %s to statement: %s\n", description, sqlite3_errstr(res)); + sqlite3_clear_bindings(stmt); + sqlite3_reset(stmt); + return false; + } + + return true; +} + static std::optional<int> ReadPragmaInteger(sqlite3* db, const std::string& key, const std::string& description, bilingual_str& error) { std::string stmt_text = strprintf("PRAGMA %s", key); @@ -377,14 +393,8 @@ bool SQLiteBatch::ReadKey(CDataStream&& key, CDataStream& value) assert(m_read_stmt); // Bind: leftmost parameter in statement is index 1 - int res = sqlite3_bind_blob(m_read_stmt, 1, key.data(), key.size(), SQLITE_STATIC); - if (res != SQLITE_OK) { - LogPrintf("%s: Unable to bind statement: %s\n", __func__, sqlite3_errstr(res)); - sqlite3_clear_bindings(m_read_stmt); - sqlite3_reset(m_read_stmt); - return false; - } - res = sqlite3_step(m_read_stmt); + if (!BindBlobToStatement(m_read_stmt, 1, key, "key")) return false; + int res = sqlite3_step(m_read_stmt); if (res != SQLITE_ROW) { if (res != SQLITE_DONE) { // SQLITE_DONE means "not found", don't log an error in that case. @@ -418,23 +428,11 @@ bool SQLiteBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrit // Bind: leftmost parameter in statement is index 1 // Insert index 1 is key, 2 is value - int res = sqlite3_bind_blob(stmt, 1, key.data(), key.size(), SQLITE_STATIC); - if (res != SQLITE_OK) { - LogPrintf("%s: Unable to bind key to statement: %s\n", __func__, sqlite3_errstr(res)); - sqlite3_clear_bindings(stmt); - sqlite3_reset(stmt); - return false; - } - res = sqlite3_bind_blob(stmt, 2, value.data(), value.size(), SQLITE_STATIC); - if (res != SQLITE_OK) { - LogPrintf("%s: Unable to bind value to statement: %s\n", __func__, sqlite3_errstr(res)); - sqlite3_clear_bindings(stmt); - sqlite3_reset(stmt); - return false; - } + if (!BindBlobToStatement(stmt, 1, key, "key")) return false; + if (!BindBlobToStatement(stmt, 2, value, "value")) return false; // Execute - res = sqlite3_step(stmt); + int res = sqlite3_step(stmt); sqlite3_clear_bindings(stmt); sqlite3_reset(stmt); if (res != SQLITE_DONE) { @@ -449,16 +447,10 @@ bool SQLiteBatch::EraseKey(CDataStream&& key) assert(m_delete_stmt); // Bind: leftmost parameter in statement is index 1 - int res = sqlite3_bind_blob(m_delete_stmt, 1, key.data(), key.size(), SQLITE_STATIC); - if (res != SQLITE_OK) { - LogPrintf("%s: Unable to bind statement: %s\n", __func__, sqlite3_errstr(res)); - sqlite3_clear_bindings(m_delete_stmt); - sqlite3_reset(m_delete_stmt); - return false; - } + if (!BindBlobToStatement(m_delete_stmt, 1, key, "key")) return false; // Execute - res = sqlite3_step(m_delete_stmt); + int res = sqlite3_step(m_delete_stmt); sqlite3_clear_bindings(m_delete_stmt); sqlite3_reset(m_delete_stmt); if (res != SQLITE_DONE) { @@ -473,18 +465,11 @@ bool SQLiteBatch::HasKey(CDataStream&& key) assert(m_read_stmt); // Bind: leftmost parameter in statement is index 1 - bool ret = false; - int res = sqlite3_bind_blob(m_read_stmt, 1, key.data(), key.size(), SQLITE_STATIC); - if (res == SQLITE_OK) { - res = sqlite3_step(m_read_stmt); - if (res == SQLITE_ROW) { - ret = true; - } - } - + if (!BindBlobToStatement(m_read_stmt, 1, key, "key")) return false; + int res = sqlite3_step(m_read_stmt); sqlite3_clear_bindings(m_read_stmt); sqlite3_reset(m_read_stmt); - return ret; + return res == SQLITE_ROW; } bool SQLiteBatch::StartCursor() |