From 74f7b1273c41892058fb2ff99aab878ccd22082a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 20 Apr 2016 09:05:12 +0200 Subject: dbwrapper: Remove throw keywords in function signatures Using throw() specifications in function signatures is not only not required in C++, it is considered deprecated for [various reasons](https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature). It is not implemented by any of the common C++ compilers. The usage is also inconsistent with the rest of the source code. --- src/dbwrapper.cpp | 4 ++-- src/dbwrapper.h | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 1907e2fa78..16f85a3e65 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -15,7 +15,7 @@ #include #include -void HandleError(const leveldb::Status& status) throw(dbwrapper_error) +void HandleError(const leveldb::Status& status) { if (status.ok()) return; @@ -102,7 +102,7 @@ CDBWrapper::~CDBWrapper() options.env = NULL; } -bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync) throw(dbwrapper_error) +bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync) { leveldb::Status status = pdb->Write(fSync ? syncoptions : writeoptions, &batch.batch); HandleError(status); diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 5e7313f7eb..96fb42429f 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -23,7 +23,7 @@ public: dbwrapper_error(const std::string& msg) : std::runtime_error(msg) {} }; -void HandleError(const leveldb::Status& status) throw(dbwrapper_error); +void HandleError(const leveldb::Status& status); /** Batch of changes queued to be written to a CDBWrapper */ class CDBBatch @@ -180,7 +180,7 @@ public: ~CDBWrapper(); template - bool Read(const K& key, V& value) const throw(dbwrapper_error) + bool Read(const K& key, V& value) const { CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(ssKey.GetSerializeSize(key)); @@ -206,7 +206,7 @@ public: } template - bool Write(const K& key, const V& value, bool fSync = false) throw(dbwrapper_error) + bool Write(const K& key, const V& value, bool fSync = false) { CDBBatch batch(&obfuscate_key); batch.Write(key, value); @@ -214,7 +214,7 @@ public: } template - bool Exists(const K& key) const throw(dbwrapper_error) + bool Exists(const K& key) const { CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(ssKey.GetSerializeSize(key)); @@ -233,14 +233,14 @@ public: } template - bool Erase(const K& key, bool fSync = false) throw(dbwrapper_error) + bool Erase(const K& key, bool fSync = false) { CDBBatch batch(&obfuscate_key); batch.Erase(key); return WriteBatch(batch, fSync); } - bool WriteBatch(CDBBatch& batch, bool fSync = false) throw(dbwrapper_error); + bool WriteBatch(CDBBatch& batch, bool fSync = false); // not available for LevelDB; provide for compatibility with BDB bool Flush() @@ -248,7 +248,7 @@ public: return true; } - bool Sync() throw(dbwrapper_error) + bool Sync() { CDBBatch batch(&obfuscate_key); return WriteBatch(batch, true); -- cgit v1.2.3 From 878bf480a3875181712a53a1156754faa19e579b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 20 Apr 2016 09:08:45 +0200 Subject: dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex It is an unnecessary method as it is used only two times and only internally, and the whole implementation is HexStr(obfuscate_key). --- src/dbwrapper.cpp | 9 ++------- src/dbwrapper.h | 6 ------ 2 files changed, 2 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 16f85a3e65..9eae7c7c80 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -84,10 +84,10 @@ CDBWrapper::CDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, b Write(OBFUSCATE_KEY_KEY, new_key); obfuscate_key = new_key; - LogPrintf("Wrote new obfuscate key for %s: %s\n", path.string(), GetObfuscateKeyHex()); + LogPrintf("Wrote new obfuscate key for %s: %s\n", path.string(), HexStr(obfuscate_key)); } - LogPrintf("Using obfuscation key for %s: %s\n", path.string(), GetObfuscateKeyHex()); + LogPrintf("Using obfuscation key for %s: %s\n", path.string(), HexStr(obfuscate_key)); } CDBWrapper::~CDBWrapper() @@ -141,11 +141,6 @@ const std::vector& CDBWrapper::GetObfuscateKey() const return obfuscate_key; } -std::string CDBWrapper::GetObfuscateKeyHex() const -{ - return HexStr(obfuscate_key); -} - CDBIterator::~CDBIterator() { delete piter; } bool CDBIterator::Valid() { return piter->Valid(); } void CDBIterator::SeekToFirst() { piter->SeekToFirst(); } diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 96fb42429f..153c0fd1bf 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -268,12 +268,6 @@ public: * Accessor for obfuscate_key. */ const std::vector& GetObfuscateKey() const; - - /** - * Return the obfuscate_key as a hex-formatted string. - */ - std::string GetObfuscateKeyHex() const; - }; #endif // BITCOIN_DBWRAPPER_H -- cgit v1.2.3 From b69836d6ff2bd7dc9568ad4af8235662bb4f1826 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 20 Apr 2016 11:46:01 +0200 Subject: dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator Pass parent wrapper directly instead of obfuscation key. This makes it possible for other databases which re-use this code to use other properties from the database. Add a namespace dbwrapper_private for private functions to be used only in dbwrapper.h/cpp and dbwrapper_tests. --- src/dbwrapper.cpp | 14 +++++++++----- src/dbwrapper.h | 46 +++++++++++++++++++++++++++----------------- src/test/dbwrapper_tests.cpp | 8 ++++---- src/txdb.cpp | 6 +++--- 4 files changed, 44 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 9eae7c7c80..42f57676ab 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -136,12 +136,16 @@ bool CDBWrapper::IsEmpty() return !(it->Valid()); } -const std::vector& CDBWrapper::GetObfuscateKey() const -{ - return obfuscate_key; -} - CDBIterator::~CDBIterator() { delete piter; } bool CDBIterator::Valid() { return piter->Valid(); } void CDBIterator::SeekToFirst() { piter->SeekToFirst(); } void CDBIterator::Next() { piter->Next(); } + +namespace dbwrapper_private { + +const std::vector& GetObfuscateKey(const CDBWrapper &w) +{ + return w.obfuscate_key; +} + +}; diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 153c0fd1bf..9eca2edf60 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -25,20 +25,34 @@ public: void HandleError(const leveldb::Status& status); +class CDBWrapper; + +/** These should be considered an implementation detail of the specific database. + */ +namespace dbwrapper_private { + +/** Work around circular dependency, as well as for testing in dbwrapper_tests. + * Database obfuscation should be considered an implementation detail of the + * specific database. + */ +const std::vector& GetObfuscateKey(const CDBWrapper &w); + +}; + /** Batch of changes queued to be written to a CDBWrapper */ class CDBBatch { friend class CDBWrapper; private: + const CDBWrapper &parent; leveldb::WriteBatch batch; - const std::vector *obfuscate_key; public: /** - * @param[in] obfuscate_key If passed, XOR data with this key. + * @param[in] parent CDBWrapper that this batch is to be submitted to */ - CDBBatch(const std::vector *obfuscate_key) : obfuscate_key(obfuscate_key) { }; + CDBBatch(const CDBWrapper &parent) : parent(parent) { }; template void Write(const K& key, const V& value) @@ -51,7 +65,7 @@ public: CDataStream ssValue(SER_DISK, CLIENT_VERSION); ssValue.reserve(ssValue.GetSerializeSize(value)); ssValue << value; - ssValue.Xor(*obfuscate_key); + ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent)); leveldb::Slice slValue(&ssValue[0], ssValue.size()); batch.Put(slKey, slValue); @@ -72,17 +86,17 @@ public: class CDBIterator { private: + const CDBWrapper &parent; leveldb::Iterator *piter; - const std::vector *obfuscate_key; public: /** + * @param[in] parent Parent CDBWrapper instance. * @param[in] piterIn The original leveldb iterator. - * @param[in] obfuscate_key If passed, XOR data with this key. */ - CDBIterator(leveldb::Iterator *piterIn, const std::vector* obfuscate_key) : - piter(piterIn), obfuscate_key(obfuscate_key) { }; + CDBIterator(const CDBWrapper &parent, leveldb::Iterator *piterIn) : + parent(parent), piter(piterIn) { }; ~CDBIterator(); bool Valid(); @@ -118,7 +132,7 @@ public: leveldb::Slice slValue = piter->value(); try { CDataStream ssValue(slValue.data(), slValue.data() + slValue.size(), SER_DISK, CLIENT_VERSION); - ssValue.Xor(*obfuscate_key); + ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent)); ssValue >> value; } catch (const std::exception&) { return false; @@ -134,6 +148,7 @@ public: class CDBWrapper { + friend const std::vector& dbwrapper_private::GetObfuscateKey(const CDBWrapper &w); private: //! custom environment this database is using (may be NULL in case of default environment) leveldb::Env* penv; @@ -208,7 +223,7 @@ public: template bool Write(const K& key, const V& value, bool fSync = false) { - CDBBatch batch(&obfuscate_key); + CDBBatch batch(*this); batch.Write(key, value); return WriteBatch(batch, fSync); } @@ -235,7 +250,7 @@ public: template bool Erase(const K& key, bool fSync = false) { - CDBBatch batch(&obfuscate_key); + CDBBatch batch(*this); batch.Erase(key); return WriteBatch(batch, fSync); } @@ -250,24 +265,19 @@ public: bool Sync() { - CDBBatch batch(&obfuscate_key); + CDBBatch batch(*this); return WriteBatch(batch, true); } CDBIterator *NewIterator() { - return new CDBIterator(pdb->NewIterator(iteroptions), &obfuscate_key); + return new CDBIterator(*this, pdb->NewIterator(iteroptions)); } /** * Return true if the database managed by this class contains no entries. */ bool IsEmpty(); - - /** - * Accessor for obfuscate_key. - */ - const std::vector& GetObfuscateKey() const; }; #endif // BITCOIN_DBWRAPPER_H diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index e399315870..081d57831d 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -39,7 +39,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) uint256 res; // Ensure that we're doing real obfuscation when obfuscate=true - BOOST_CHECK(obfuscate != is_null_key(dbw.GetObfuscateKey())); + BOOST_CHECK(obfuscate != is_null_key(dbwrapper_private::GetObfuscateKey(dbw))); BOOST_CHECK(dbw.Write(key, in)); BOOST_CHECK(dbw.Read(key, res)); @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) uint256 in3 = GetRandHash(); uint256 res; - CDBBatch batch(&dbw.GetObfuscateKey()); + CDBBatch batch(dbw); batch.Write(key, in); batch.Write(key2, in2); @@ -156,7 +156,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) BOOST_CHECK_EQUAL(res2.ToString(), in.ToString()); BOOST_CHECK(!odbw.IsEmpty()); // There should be existing data - BOOST_CHECK(is_null_key(odbw.GetObfuscateKey())); // The key should be an empty string + BOOST_CHECK(is_null_key(dbwrapper_private::GetObfuscateKey(odbw))); // The key should be an empty string uint256 in2 = GetRandHash(); uint256 res3; @@ -193,7 +193,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) // Check that the key/val we wrote with unobfuscated wrapper doesn't exist uint256 res2; BOOST_CHECK(!odbw.Read(key, res2)); - BOOST_CHECK(!is_null_key(odbw.GetObfuscateKey())); + BOOST_CHECK(!is_null_key(dbwrapper_private::GetObfuscateKey(odbw))); uint256 in2 = GetRandHash(); uint256 res3; diff --git a/src/txdb.cpp b/src/txdb.cpp index 19ca178654..5fbaeb608a 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -49,7 +49,7 @@ uint256 CCoinsViewDB::GetBestBlock() const { } bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { - CDBBatch batch(&db.GetObfuscateKey()); + CDBBatch batch(db); size_t count = 0; size_t changed = 0; for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { @@ -139,7 +139,7 @@ void CCoinsViewDBCursor::Next() } bool CBlockTreeDB::WriteBatchSync(const std::vector >& fileInfo, int nLastFile, const std::vector& blockinfo) { - CDBBatch batch(&GetObfuscateKey()); + CDBBatch batch(*this); for (std::vector >::const_iterator it=fileInfo.begin(); it != fileInfo.end(); it++) { batch.Write(make_pair(DB_BLOCK_FILES, it->first), *it->second); } @@ -155,7 +155,7 @@ bool CBlockTreeDB::ReadTxIndex(const uint256 &txid, CDiskTxPos &pos) { } bool CBlockTreeDB::WriteTxIndex(const std::vector >&vect) { - CDBBatch batch(&GetObfuscateKey()); + CDBBatch batch(*this); for (std::vector >::const_iterator it=vect.begin(); it!=vect.end(); it++) batch.Write(make_pair(DB_TXINDEX, it->first), it->second); return WriteBatch(batch); -- cgit v1.2.3 From 869cf1234a915808fda6fd663dead5580fbd046e Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 20 Apr 2016 11:48:57 +0200 Subject: dbwrapper: Move `HandleError` to `dbwrapper_private` HandleError is implementation-specific. --- src/dbwrapper.cpp | 34 +++++++++++++++++----------------- src/dbwrapper.h | 10 ++++++---- 2 files changed, 23 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 42f57676ab..09c68fbe55 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -15,20 +15,6 @@ #include #include -void HandleError(const leveldb::Status& status) -{ - if (status.ok()) - return; - LogPrintf("%s\n", status.ToString()); - if (status.IsCorruption()) - throw dbwrapper_error("Database corrupted"); - if (status.IsIOError()) - throw dbwrapper_error("Database I/O error"); - if (status.IsNotFound()) - throw dbwrapper_error("Database entry missing"); - throw dbwrapper_error("Unknown database error"); -} - static leveldb::Options GetOptions(size_t nCacheSize) { leveldb::Options options; @@ -61,13 +47,13 @@ CDBWrapper::CDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, b if (fWipe) { LogPrintf("Wiping LevelDB in %s\n", path.string()); leveldb::Status result = leveldb::DestroyDB(path.string(), options); - HandleError(result); + dbwrapper_private::HandleError(result); } TryCreateDirectory(path); LogPrintf("Opening LevelDB in %s\n", path.string()); } leveldb::Status status = leveldb::DB::Open(options, path.string(), &pdb); - HandleError(status); + dbwrapper_private::HandleError(status); LogPrintf("Opened LevelDB successfully\n"); // The base-case obfuscation key, which is a noop. @@ -105,7 +91,7 @@ CDBWrapper::~CDBWrapper() bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync) { leveldb::Status status = pdb->Write(fSync ? syncoptions : writeoptions, &batch.batch); - HandleError(status); + dbwrapper_private::HandleError(status); return true; } @@ -143,6 +129,20 @@ void CDBIterator::Next() { piter->Next(); } namespace dbwrapper_private { +void HandleError(const leveldb::Status& status) +{ + if (status.ok()) + return; + LogPrintf("%s\n", status.ToString()); + if (status.IsCorruption()) + throw dbwrapper_error("Database corrupted"); + if (status.IsIOError()) + throw dbwrapper_error("Database I/O error"); + if (status.IsNotFound()) + throw dbwrapper_error("Database entry missing"); + throw dbwrapper_error("Unknown database error"); +} + const std::vector& GetObfuscateKey(const CDBWrapper &w) { return w.obfuscate_key; diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 9eca2edf60..a0779d3ab9 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -23,14 +23,16 @@ public: dbwrapper_error(const std::string& msg) : std::runtime_error(msg) {} }; -void HandleError(const leveldb::Status& status); - class CDBWrapper; /** These should be considered an implementation detail of the specific database. */ namespace dbwrapper_private { +/** Handle database error by throwing dbwrapper_error exception. + */ +void HandleError(const leveldb::Status& status); + /** Work around circular dependency, as well as for testing in dbwrapper_tests. * Database obfuscation should be considered an implementation detail of the * specific database. @@ -208,7 +210,7 @@ public: if (status.IsNotFound()) return false; LogPrintf("LevelDB read failure: %s\n", status.ToString()); - HandleError(status); + dbwrapper_private::HandleError(status); } try { CDataStream ssValue(strValue.data(), strValue.data() + strValue.size(), SER_DISK, CLIENT_VERSION); @@ -242,7 +244,7 @@ public: if (status.IsNotFound()) return false; LogPrintf("LevelDB read failure: %s\n", status.ToString()); - HandleError(status); + dbwrapper_private::HandleError(status); } return true; } -- cgit v1.2.3