From b6718e373ed425fa2440ddd8f1b05c76b782dc2b Mon Sep 17 00:00:00 2001 From: practicalswift Date: Fri, 21 Sep 2018 11:03:21 +0200 Subject: [PATCH] tests: Use MakeUnique to construct objects owned by unique_ptrs --- doc/developer-notes.md | 5 +++++ src/bench/bench_bitcoin.cpp | 2 +- src/bench/coin_selection.cpp | 2 +- src/test/allocator_tests.cpp | 2 +- src/test/checkqueue_tests.cpp | 14 +++++++------- src/test/net_tests.cpp | 4 ++-- src/test/test_bitcoin.cpp | 2 +- src/test/test_bitcoin_fuzzy.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 2 +- 9 files changed, 20 insertions(+), 15 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index da8384c53..bdb8d7dba 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -439,6 +439,11 @@ General C++ - *Rationale*: This avoids memory and resource leaks, and ensures exception safety +- Use `MakeUnique()` to construct objects owned by `unique_ptr`s + + - *Rationale*: `MakeUnique` is concise and ensures exception safety in complex expressions. + `MakeUnique` is a temporary project local implementation of `std::make_unique` (C++14). + C++ data structures -------------------- diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index d7b8083e7..4fa516cb8 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -82,7 +82,7 @@ int main(int argc, char** argv) return EXIT_FAILURE; } - std::unique_ptr printer(new benchmark::ConsolePrinter()); + std::unique_ptr printer = MakeUnique(); std::string printer_arg = gArgs.GetArg("-printer", DEFAULT_BENCH_PRINTER); if ("plot" == printer_arg) { printer.reset(new benchmark::PlotlyPrinter( diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 0a6f5d85e..27c23d683 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -66,7 +66,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; - std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + std::unique_ptr wtx = MakeUnique(&testWallet, MakeTransactionRef(std::move(tx))); set.emplace_back(COutput(wtx.get(), nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0); wtxn.emplace_back(std::move(wtx)); } diff --git a/src/test/allocator_tests.cpp b/src/test/allocator_tests.cpp index 1052ada23..c72c062b8 100644 --- a/src/test/allocator_tests.cpp +++ b/src/test/allocator_tests.cpp @@ -163,7 +163,7 @@ private: BOOST_AUTO_TEST_CASE(lockedpool_tests_mock) { // Test over three virtual arenas, of which one will succeed being locked - std::unique_ptr x(new TestLockedPageAllocator(3, 1)); + std::unique_ptr x = MakeUnique(3, 1); LockedPool pool(std::move(x)); BOOST_CHECK(pool.stats().total == 0); BOOST_CHECK(pool.stats().locked == 0); diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 2732598b4..f4b416c4c 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -148,7 +148,7 @@ typedef CCheckQueue FrozenCleanup_Queue; */ static void Correct_Queue_range(std::vector range) { - auto small_queue = std::unique_ptr(new Correct_Queue {QUEUE_BATCH_SIZE}); + auto small_queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; for (auto x = 0; x < nScriptCheckThreads; ++x) { tg.create_thread([&]{small_queue->Thread();}); @@ -213,7 +213,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random) /** Test that failing checks are caught */ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) { - auto fail_queue = std::unique_ptr(new Failing_Queue {QUEUE_BATCH_SIZE}); + auto fail_queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; for (auto x = 0; x < nScriptCheckThreads; ++x) { @@ -246,7 +246,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) // future blocks, ie, the bad state is cleared. BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) { - auto fail_queue = std::unique_ptr(new Failing_Queue {QUEUE_BATCH_SIZE}); + auto fail_queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; for (auto x = 0; x < nScriptCheckThreads; ++x) { tg.create_thread([&]{fail_queue->Thread();}); @@ -274,7 +274,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) // more than once as well BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) { - auto queue = std::unique_ptr(new Unique_Queue {QUEUE_BATCH_SIZE}); + auto queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; for (auto x = 0; x < nScriptCheckThreads; ++x) { tg.create_thread([&]{queue->Thread();}); @@ -310,7 +310,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) // time could leave the data hanging across a sequence of blocks. BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) { - auto queue = std::unique_ptr(new Memory_Queue {QUEUE_BATCH_SIZE}); + auto queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; for (auto x = 0; x < nScriptCheckThreads; ++x) { tg.create_thread([&]{queue->Thread();}); @@ -341,7 +341,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) // have been destructed BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) { - auto queue = std::unique_ptr(new FrozenCleanup_Queue {QUEUE_BATCH_SIZE}); + auto queue = MakeUnique(QUEUE_BATCH_SIZE); boost::thread_group tg; bool fails = false; for (auto x = 0; x < nScriptCheckThreads; ++x) { @@ -384,7 +384,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) /** Test that CCheckQueueControl is threadsafe */ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks) { - auto queue = std::unique_ptr(new Standard_Queue{QUEUE_BATCH_SIZE}); + auto queue = MakeUnique(QUEUE_BATCH_SIZE); { boost::thread_group tg; std::atomic nThreads {0}; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index eaa8b1618..35a143957 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -179,12 +179,12 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) bool fInboundIn = false; // Test that fFeeler is false by default. - std::unique_ptr pnode1(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, fInboundIn)); + std::unique_ptr pnode1 = MakeUnique(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, fInboundIn); BOOST_CHECK(pnode1->fInbound == false); BOOST_CHECK(pnode1->fFeeler == false); fInboundIn = true; - std::unique_ptr pnode2(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, fInboundIn)); + std::unique_ptr pnode2 = MakeUnique(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, fInboundIn); BOOST_CHECK(pnode2->fInbound == true); BOOST_CHECK(pnode2->fFeeler == false); } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 0d7c9f0ab..766e34e5b 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -107,7 +107,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha nScriptCheckThreads = 3; for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - g_connman = std::unique_ptr(new CConnman(0x1337, 0x1337)); // Deterministic randomness for tests. + g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. connman = g_connman.get(); peerLogic.reset(new PeerLogicValidation(connman, scheduler, /*enable_bip61=*/true)); } diff --git a/src/test/test_bitcoin_fuzzy.cpp b/src/test/test_bitcoin_fuzzy.cpp index 11b7c66cc..88c082ff6 100644 --- a/src/test/test_bitcoin_fuzzy.cpp +++ b/src/test/test_bitcoin_fuzzy.cpp @@ -279,7 +279,7 @@ static int test_one_input(std::vector buffer) { static std::unique_ptr globalVerifyHandle; void initialize() { - globalVerifyHandle = std::unique_ptr(new ECCVerifyHandle()); + globalVerifyHandle = MakeUnique(); } // This function is used by libFuzzer diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 6d1d09d5a..21857df08 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -65,7 +65,7 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + std::unique_ptr wtx = MakeUnique(&testWallet, MakeTransactionRef(std::move(tx))); if (fIsFromMe) { wtx->fDebitCached = true;