From fa021e9a5b7e930a3db0febb416942dea3a90a8f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 20 Jun 2020 08:44:42 -0400 Subject: [PATCH 1/2] wallet: Remove confusing double return value ret+success Also, remove redundant comments --- src/wallet/bdb.cpp | 9 --------- src/wallet/bdb.h | 25 ++++++------------------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 125bf004e44..a3a56aa51c8 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -809,10 +809,8 @@ bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) if (!pdb) return false; - // Key SafeDbt datKey(key.data(), key.size()); - // Read SafeDbt datValue; int ret = pdb->get(activeTxn, datKey, datValue, 0); if (ret == 0 && datValue.get_data() != nullptr) { @@ -829,13 +827,10 @@ bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrit if (fReadOnly) assert(!"Write called on database in read-only mode"); - // Key SafeDbt datKey(key.data(), key.size()); - // Value SafeDbt datValue(value.data(), value.size()); - // Write int ret = pdb->put(activeTxn, datKey, datValue, (overwrite ? 0 : DB_NOOVERWRITE)); return (ret == 0); } @@ -847,10 +842,8 @@ bool BerkeleyBatch::EraseKey(CDataStream& key) if (fReadOnly) assert(!"Erase called on database in read-only mode"); - // Key SafeDbt datKey(key.data(), key.size()); - // Erase int ret = pdb->del(activeTxn, datKey, 0); return (ret == 0 || ret == DB_NOTFOUND); } @@ -860,10 +853,8 @@ bool BerkeleyBatch::HasKey(CDataStream& key) if (!pdb) return false; - // Key SafeDbt datKey(key.data(), key.size()); - // Exists int ret = pdb->exists(activeTxn, datKey, 0); return ret == 0; } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index c121bb42282..a2eaf950012 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -223,64 +223,51 @@ public: template bool Read(const K& key, T& value) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool success = false; - bool ret = ReadKey(ssKey, ssValue); - if (ret) { - // Unserialize value - try { - ssValue >> value; - success = true; - } catch (const std::exception&) { - // In this case success remains 'false' - } + if (!ReadKey(ssKey, ssValue)) return false; + try { + ssValue >> value; + return true; + } catch (const std::exception&) { + return false; } - return ret && success; } template bool Write(const K& key, const T& value, bool fOverwrite = true) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Value CDataStream ssValue(SER_DISK, CLIENT_VERSION); ssValue.reserve(10000); ssValue << value; - // Write return WriteKey(ssKey, ssValue, fOverwrite); } template bool Erase(const K& key) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Erase return EraseKey(ssKey); } template bool Exists(const K& key) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Exists return HasKey(ssKey); } From fa8a341b88cabfd7f8d702db7cb9972b0804bf2a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 20 Jun 2020 08:55:07 -0400 Subject: [PATCH 2/2] wallet: Replace CDataStream& with CDataStream&& where appropriate The keys and values are only to be used once because their memory is set to zero. Make that explicit by moving the bytes into the lower level methods. --- src/wallet/bdb.cpp | 8 ++++---- src/wallet/bdb.h | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index a3a56aa51c8..267b147f38a 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -804,7 +804,7 @@ std::string BerkeleyDatabaseVersion() return DbEnv::version(nullptr, nullptr, nullptr); } -bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) +bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) { if (!pdb) return false; @@ -820,7 +820,7 @@ bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) return false; } -bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite) +bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite) { if (!pdb) return true; @@ -835,7 +835,7 @@ bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrit return (ret == 0); } -bool BerkeleyBatch::EraseKey(CDataStream& key) +bool BerkeleyBatch::EraseKey(CDataStream&& key) { if (!pdb) return false; @@ -848,7 +848,7 @@ bool BerkeleyBatch::EraseKey(CDataStream& key) return (ret == 0 || ret == DB_NOTFOUND); } -bool BerkeleyBatch::HasKey(CDataStream& key) +bool BerkeleyBatch::HasKey(CDataStream&& key) { if (!pdb) return false; diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index a2eaf950012..91116afd019 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -189,10 +189,10 @@ class BerkeleyBatch }; private: - bool ReadKey(CDataStream& key, CDataStream& value); - bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true); - bool EraseKey(CDataStream& key); - bool HasKey(CDataStream& key); + bool ReadKey(CDataStream&& key, CDataStream& value); + bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true); + bool EraseKey(CDataStream&& key); + bool HasKey(CDataStream&& key); protected: Db* pdb; @@ -228,7 +228,7 @@ public: ssKey << key; CDataStream ssValue(SER_DISK, CLIENT_VERSION); - if (!ReadKey(ssKey, ssValue)) return false; + if (!ReadKey(std::move(ssKey), ssValue)) return false; try { ssValue >> value; return true; @@ -248,7 +248,7 @@ public: ssValue.reserve(10000); ssValue << value; - return WriteKey(ssKey, ssValue, fOverwrite); + return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite); } template @@ -258,7 +258,7 @@ public: ssKey.reserve(1000); ssKey << key; - return EraseKey(ssKey); + return EraseKey(std::move(ssKey)); } template @@ -268,7 +268,7 @@ public: ssKey.reserve(1000); ssKey << key; - return HasKey(ssKey); + return HasKey(std::move(ssKey)); } Dbc* GetCursor();