From 2e56702ecedd83c4b7cb8de9de5c437c8c08e645 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 5 Oct 2017 13:35:20 -0400 Subject: [PATCH] banman: pass the banfile path in There's no need to hard-code the path here. Passing it in means that there are no ordering concerns wrt establishing the datadir. --- src/addrdb.cpp | 7 +++---- src/addrdb.h | 4 ++-- src/init.cpp | 2 +- src/net.cpp | 9 +++------ src/net.h | 3 ++- src/test/denialofservice_tests.cpp | 6 +++--- src/test/test_bitcoin.cpp | 2 +- 7 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 1590bce074d..c6083f5554b 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -105,19 +105,18 @@ bool DeserializeFileDB(const fs::path& path, Data& data) } -CBanDB::CBanDB() +CBanDB::CBanDB(fs::path ban_list_path) : m_ban_list_path(std::move(ban_list_path)) { - pathBanlist = GetDataDir() / "banlist.dat"; } bool CBanDB::Write(const banmap_t& banSet) { - return SerializeFileDB("banlist", pathBanlist, banSet); + return SerializeFileDB("banlist", m_ban_list_path, banSet); } bool CBanDB::Read(banmap_t& banSet) { - return DeserializeFileDB(pathBanlist, banSet); + return DeserializeFileDB(m_ban_list_path, banSet); } CAddrDB::CAddrDB() diff --git a/src/addrdb.h b/src/addrdb.h index 90eca44bdbb..88da31a6fc3 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -92,9 +92,9 @@ public: class CBanDB { private: - fs::path pathBanlist; + const fs::path m_ban_list_path; public: - CBanDB(); + explicit CBanDB(fs::path ban_list_path); bool Write(const banmap_t& banSet); bool Read(banmap_t& banSet); }; diff --git a/src/init.cpp b/src/init.cpp index 86b39c34eae..ded157d49e2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1296,7 +1296,7 @@ bool AppInitMain(InitInterfaces& interfaces) // need to reindex later. assert(!g_banman); - g_banman = MakeUnique(&uiInterface); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", &uiInterface); assert(!g_connman); g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); diff --git a/src/net.cpp b/src/net.cpp index 5479130320d..fd74c58292a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -466,10 +466,9 @@ void BanMan::DumpBanlist() int64_t nStart = GetTimeMillis(); - CBanDB bandb; banmap_t banmap; GetBanned(banmap); - if (bandb.Write(banmap)) { + if (m_ban_db.Write(banmap)) { SetBannedSetDirty(false); } @@ -2431,16 +2430,14 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) return true; } -BanMan::BanMan(CClientUIInterface* client_interface) : clientInterface(client_interface) +BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface) : clientInterface(client_interface), m_ban_db(std::move(ban_file)) { if (clientInterface) clientInterface->InitMessage(_("Loading banlist...")); - // Load addresses from banlist.dat int64_t nStart = GetTimeMillis(); setBannedIsDirty = false; - CBanDB bandb; banmap_t banmap; - if (bandb.Read(banmap)) { + if (m_ban_db.Read(banmap)) { SetBanned(banmap); // thread save setter SetBannedSetDirty(false); // no need to write down, just read data SweepBanned(); // sweep out unused entries diff --git a/src/net.h b/src/net.h index 020e600b2cc..5e6fc14b389 100644 --- a/src/net.h +++ b/src/net.h @@ -133,7 +133,7 @@ public: // between nodes running old code and nodes running // new code. ~BanMan(); - BanMan(CClientUIInterface* client_interface); + BanMan(fs::path ban_file, CClientUIInterface* client_interface); void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false); void ClearBanned(); // needed for unit testing @@ -156,6 +156,7 @@ private: CCriticalSection cs_setBanned; bool setBannedIsDirty; CClientUIInterface* clientInterface = nullptr; + CBanDB m_ban_db; }; class NetEventsInterface; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 9776ea223d4..4a88613a892 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(DoS_banning) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -271,7 +271,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_AUTO_TEST_CASE(DoS_banscore) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); @@ -318,7 +318,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) BOOST_AUTO_TEST_CASE(DoS_bantime) { - auto banman = MakeUnique(nullptr); + auto banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); auto connman = MakeUnique(0x1337, 0x1337); auto peerLogic = MakeUnique(connman.get(), banman.get(), scheduler, false); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 6d1f70f9a19..7bce74b46b0 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -94,7 +94,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha for (int i=0; i < nScriptCheckThreads-1; i++) threadGroup.create_thread(&ThreadScriptCheck); - g_banman = MakeUnique(nullptr); + g_banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr); g_connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. }