Merge bitcoin/bitcoin#24486: wallet: refactor: dedup sqlite blob binding

8ea6167099 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 8ea6167099
  achow101:
    ACK 8ea6167099
  klementtan:
    ACK 8ea6167099

Tree-SHA512: 1de0d214f836bc405a01e98a3a2d71f2deaddc7d23c31cad80219d1614bec92619c06d9a4a091dd563d3e95ffb879649c29745d8f89669b2a5330552c212af3f
This commit is contained in:
fanquake 2022-03-10 12:40:03 +00:00
commit 4f5d3ce5a0
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1

View file

@ -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()