From: Otto Moerbeek Date: Tue, 19 Mar 2024 14:21:42 +0000 (+0100) Subject: Tidy X-Git-Tag: rec-5.1.0-alpha1~77^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=02b99062b154ce5f2ea70d08a2938bfc6f951570;p=thirdparty%2Fpdns.git Tidy --- diff --git a/pdns/recursordist/nod.cc b/pdns/recursordist/nod.cc index 811811ca6a..f049af939f 100644 --- a/pdns/recursordist/nod.cc +++ b/pdns/recursordist/nod.cc @@ -28,7 +28,7 @@ #include #include #include "threadname.hh" -#include +#include #include "logger.hh" #include "logging.hh" #include "misc.hh" @@ -40,10 +40,10 @@ namespace filesystem = boost::filesystem; std::mutex PersistentSBF::d_cachedir_mutex; -void PersistentSBF::remove_tmp_files(const filesystem::path& p, std::lock_guard& /* lock */) +void PersistentSBF::remove_tmp_files(const filesystem::path& path, std::lock_guard& /* lock */) { Regex file_regex(d_prefix + ".*\\." + bf_suffix + "\\..{8}$"); - for (filesystem::directory_iterator i(p); i != filesystem::directory_iterator(); ++i) { + for (filesystem::directory_iterator i(path); i != filesystem::directory_iterator(); ++i) { if (filesystem::is_regular_file(i->path()) && file_regex.match(i->path().filename().string())) { filesystem::remove(*i); } @@ -59,15 +59,15 @@ bool PersistentSBF::init(bool ignore_pid) { auto log = g_slog->withName("nod"); std::lock_guard lock(d_cachedir_mutex); - if (d_cachedir.length()) { - filesystem::path p(d_cachedir); + if (d_cachedir.length() != 0) { + filesystem::path path(d_cachedir); try { - if (filesystem::exists(p) && filesystem::is_directory(p)) { - remove_tmp_files(p, lock); + if (filesystem::exists(path) && filesystem::is_directory(path)) { + remove_tmp_files(path, lock); filesystem::path newest_file; std::time_t newest_time = time(nullptr); Regex file_regex(d_prefix + ".*\\." + bf_suffix + "$"); - for (filesystem::directory_iterator i(p); i != filesystem::directory_iterator(); ++i) { + for (filesystem::directory_iterator i(path); i != filesystem::directory_iterator(); ++i) { if (filesystem::is_regular_file(i->path()) && file_regex.match(i->path().filename().string())) { if (ignore_pid || (i->path().filename().string().find(std::to_string(getpid())) == std::string::npos)) { // look for the newest file matching the regex @@ -79,7 +79,7 @@ bool PersistentSBF::init(bool ignore_pid) } } if (filesystem::exists(newest_file)) { - std::string filename = newest_file.string(); + const std::string& filename = newest_file.string(); std::ifstream infile; try { infile.open(filename, std::ios::in | std::ios::binary); @@ -130,13 +130,13 @@ void PersistentSBF::setCacheDir(const std::string& cachedir) bool PersistentSBF::snapshotCurrent(std::thread::id tid) { auto log = g_slog->withName("nod"); - if (d_cachedir.length()) { - filesystem::path p(d_cachedir); - filesystem::path f(d_cachedir); - std::stringstream ss; - ss << d_prefix << "_" << tid; - f /= ss.str() + "_" + std::to_string(getpid()) + "." + bf_suffix; - if (filesystem::exists(p) && filesystem::is_directory(p)) { + if (d_cachedir.length() != 0) { + filesystem::path path(d_cachedir); + filesystem::path file(d_cachedir); + std::stringstream strStream; + strStream << d_prefix << "_" << tid; + file /= strStream.str() + "_" + std::to_string(getpid()) + "." + bf_suffix; + if (filesystem::exists(path) && filesystem::is_directory(path)) { try { std::ofstream ofile; std::stringstream iss; @@ -145,24 +145,24 @@ bool PersistentSBF::snapshotCurrent(std::thread::id tid) d_sbf.lock()->dump(iss); } // Now write it out to the file - std::string ftmp = f.string() + ".XXXXXXXX"; - int fd = mkstemp(&ftmp.at(0)); - if (fd == -1) { + std::string ftmp = file.string() + ".XXXXXXXX"; + int fileDesc = mkstemp(&ftmp.at(0)); + if (fileDesc == -1) { throw std::runtime_error("Cannot create temp file: " + stringerror()); } std::string str = iss.str(); - ssize_t len = write(fd, str.data(), str.length()); + ssize_t len = write(fileDesc, str.data(), str.length()); if (len != static_cast(str.length())) { - close(fd); + close(fileDesc); filesystem::remove(ftmp.c_str()); throw std::runtime_error("Failed to write to file:" + ftmp); } - if (close(fd) != 0) { + if (close(fileDesc) != 0) { filesystem::remove(ftmp); throw std::runtime_error("Failed to write to file:" + ftmp); } try { - filesystem::rename(ftmp, f); + filesystem::rename(ftmp, file); } catch (const std::runtime_error& e) { SLOG(g_log << Logger::Warning << "NODDB snapshot: Cannot rename file: " << e.what() << endl, @@ -178,8 +178,8 @@ bool PersistentSBF::snapshotCurrent(std::thread::id tid) } } else { - SLOG(g_log << Logger::Warning << "NODDB snapshot: Cannot write file: " << f.string() << endl, - log->info(Logr::Warning, "NODDB snapshot: Cannot write file", "file", Logging::Loggable(f.string()))); + SLOG(g_log << Logger::Warning << "NODDB snapshot: Cannot write file: " << file.string() << endl, + log->info(Logr::Warning, "NODDB snapshot: Cannot write file", "file", Logging::Loggable(file.string()))); } } return false; @@ -191,10 +191,8 @@ void NODDB::housekeepingThread(std::thread::id tid) { setThreadName("rec/nod-hk"); for (;;) { - sleep(d_snapshot_interval); - { - snapshotCurrent(tid); - } + std::this_thread::sleep_for(std::chrono::seconds(d_snapshot_interval)); + snapshotCurrent(tid); } } @@ -222,7 +220,7 @@ bool NODDB::isNewDomainWithParent(const std::string& domain, std::string& observ bool NODDB::isNewDomainWithParent(const DNSName& dname, std::string& observed) { bool ret = isNewDomain(dname); - if (ret == true) { + if (ret) { DNSName mdname = dname; while (mdname.chopOff()) { if (!isNewDomain(mdname)) { @@ -261,9 +259,7 @@ void UniqueResponseDB::housekeepingThread(std::thread::id tid) { setThreadName("rec/udr-hk"); for (;;) { - sleep(d_snapshot_interval); - { - snapshotCurrent(tid); - } + std::this_thread::sleep_for(std::chrono::seconds(d_snapshot_interval)); + snapshotCurrent(tid); } } diff --git a/pdns/recursordist/nod.hh b/pdns/recursordist/nod.hh index a27b1d77fd..bc5c8c26be 100644 --- a/pdns/recursordist/nod.hh +++ b/pdns/recursordist/nod.hh @@ -77,8 +77,7 @@ private: class NODDB { public: - NODDB() : - d_psbf{} {} + NODDB() = default; NODDB(uint32_t num_cells) : d_psbf{num_cells} {} // Set ignore_pid to true if you don't mind loading files @@ -97,7 +96,7 @@ public: void setSnapshotInterval(unsigned int secs) { d_snapshot_interval = secs; } void setCacheDir(const std::string& cachedir) { d_psbf.setCacheDir(cachedir); } bool snapshotCurrent(std::thread::id tid) { return d_psbf.snapshotCurrent(tid); } - static void startHousekeepingThread(std::shared_ptr noddbp, std::thread::id tid) + static void startHousekeepingThread(const std::shared_ptr& noddbp, std::thread::id tid) { noddbp->housekeepingThread(tid); } @@ -111,8 +110,7 @@ private: class UniqueResponseDB { public: - UniqueResponseDB() : - d_psbf{} {} + UniqueResponseDB() = default; UniqueResponseDB(uint32_t num_cells) : d_psbf{num_cells} {} bool init(bool ignore_pid = false) @@ -125,7 +123,7 @@ public: void setSnapshotInterval(unsigned int secs) { d_snapshot_interval = secs; } void setCacheDir(const std::string& cachedir) { d_psbf.setCacheDir(cachedir); } bool snapshotCurrent(std::thread::id tid) { return d_psbf.snapshotCurrent(tid); } - static void startHousekeepingThread(std::shared_ptr udrdbp, std::thread::id tid) + static void startHousekeepingThread(const std::shared_ptr& udrdbp, std::thread::id tid) { udrdbp->housekeepingThread(tid); } diff --git a/pdns/recursordist/stable-bloom.hh b/pdns/recursordist/stable-bloom.hh index bf7c837692..1c1b7b1e1b 100644 --- a/pdns/recursordist/stable-bloom.hh +++ b/pdns/recursordist/stable-bloom.hh @@ -29,6 +29,7 @@ #include #include "misc.hh" #include "noinitvector.hh" +#include "views.hh" #include "ext/probds/murmur3.h" namespace bf @@ -40,119 +41,139 @@ namespace bf class stableBF { public: - stableBF(float fp_rate, uint32_t num_cells, uint8_t p) : + stableBF(float fp_rate, uint32_t num_cells, uint8_t pArg) : d_k(optimalK(fp_rate)), d_num_cells(num_cells), - d_p(p), + d_p(pArg), d_cells(num_cells), d_gen(std::random_device()()), - d_dis(0, num_cells) {} - stableBF(uint8_t k, uint32_t num_cells, uint8_t p, const std::string& bitstr) : - d_k(k), + d_dis(0, static_cast(num_cells)) {} + stableBF(uint8_t kArg, uint32_t num_cells, uint8_t pArg, const std::string& bitstr) : + d_k(kArg), d_num_cells(num_cells), - d_p(p), + d_p(pArg), d_cells(bitstr), d_gen(std::random_device()()), - d_dis(0, num_cells) {} + d_dis(0, static_cast(num_cells)) {} + void add(const std::string& data) { decrement(); auto hashes = hash(data); - for (auto& i : hashes) { - d_cells.set(i % d_num_cells); + for (auto& hash : hashes) { + d_cells.set(hash % d_num_cells); } } - bool test(const std::string& data) const + + [[nodiscard]] bool test(const std::string& data) const { auto hashes = hash(data); - for (auto& i : hashes) { - if (d_cells.test(i % d_num_cells) == false) + for (auto& hash : hashes) { // NOLINT(readability-use-anyofallof) not more clear IMO + if (!d_cells.test(hash % d_num_cells)) { return false; + } } return true; } + bool testAndAdd(const std::string& data) { auto hashes = hash(data); bool retval = true; - for (auto& i : hashes) { - if (d_cells.test(i % d_num_cells) == false) { + for (auto& hash : hashes) { + if (!d_cells.test(hash % d_num_cells)) { retval = false; break; } } decrement(); - for (auto& i : hashes) { - d_cells.set(i % d_num_cells); + for (auto& hash : hashes) { + d_cells.set(hash % d_num_cells); } return retval; } - void dump(std::ostream& os) + + void dump(std::ostream& ostr) { - os.write((char*)&d_k, sizeof(d_k)); + ostr.write(charPtr(&d_k), sizeof(d_k)); uint32_t nint = htonl(d_num_cells); - os.write((char*)&nint, sizeof(nint)); - os.write((char*)&d_p, sizeof(d_p)); + ostr.write(charPtr(&nint), sizeof(nint)); + ostr.write(charPtr(&d_p), sizeof(d_p)); std::string temp_str; boost::to_string(d_cells, temp_str); - uint32_t bitstr_length = htonl((uint32_t)temp_str.length()); - os.write((char*)&bitstr_length, sizeof(bitstr_length)); - os.write((char*)temp_str.c_str(), temp_str.length()); - if (os.fail()) { + uint32_t bitstr_length = htonl(static_cast(temp_str.length())); + ostr.write(charPtr(&bitstr_length), sizeof(bitstr_length)); + ostr.write(charPtr(temp_str.c_str()), static_cast(temp_str.length())); + if (ostr.fail()) { throw std::runtime_error("SBF: Failed to dump"); } } - void restore(std::istream& is) + + void restore(std::istream& istr) { - uint8_t k, p; - uint32_t num_cells, bitstr_len; - is.read((char*)&k, sizeof(k)); - if (is.fail()) { + uint8_t kValue{}; + istr.read(charPtr(&kValue), sizeof(kValue)); + if (istr.fail()) { throw std::runtime_error("SBF: read failed (file too short?)"); } - is.read((char*)&num_cells, sizeof(num_cells)); - if (is.fail()) { + uint32_t num_cells{}; + istr.read(charPtr(&num_cells), sizeof(num_cells)); + if (istr.fail()) { throw std::runtime_error("SBF: read failed (file too short?)"); } num_cells = ntohl(num_cells); - is.read((char*)&p, sizeof(p)); - if (is.fail()) { + uint8_t pValue{}; + istr.read(charPtr(&pValue), sizeof(pValue)); + if (istr.fail()) { throw std::runtime_error("SBF: read failed (file too short?)"); } - is.read((char*)&bitstr_len, sizeof(bitstr_len)); - if (is.fail()) { + uint32_t bitstr_len{}; + istr.read(charPtr(&bitstr_len), sizeof(bitstr_len)); + if (istr.fail()) { throw std::runtime_error("SBF: read failed (file too short?)"); } bitstr_len = ntohl(bitstr_len); if (bitstr_len > 2 * 64 * 1024 * 1024U) { // twice the current size throw std::runtime_error("SBF: read failed (bitstr_len too big)"); } - auto bitcstr = std::make_unique(bitstr_len); - is.read(bitcstr.get(), bitstr_len); - if (is.fail()) { + auto bitcstr = NoInitVector(bitstr_len); + istr.read(bitcstr.data(), bitstr_len); + if (istr.fail()) { throw std::runtime_error("SBF: read failed (file too short?)"); } - std::string bitstr(bitcstr.get(), bitstr_len); - stableBF tempbf(k, num_cells, p, bitstr); + const std::string bitstr(bitcstr.data(), bitstr_len); + stableBF tempbf(kValue, num_cells, pValue, bitstr); swap(tempbf); } private: - unsigned int optimalK(float fp_rate) + static const char* charPtr(const void* ptr) + { + return static_cast(ptr); + } + + static char* charPtr(void* ptr) + { + return static_cast(ptr); + } + + static unsigned int optimalK(float fp_rate) { - return std::ceil(std::log2(1 / fp_rate)); + return std::ceil(std::log2(1.0 / fp_rate)); } + void decrement() { // Choose a random cell then decrement the next p-1 // The stable bloom algorithm described in the paper says // to choose p independent positions, but that is much slower // and this shouldn't change the properties of the SBF - size_t r = d_dis(d_gen); + size_t randomValue = d_dis(d_gen); for (uint64_t i = 0; i < d_p; ++i) { - d_cells.reset((r + i) % d_num_cells); + d_cells.reset((randomValue + i) % d_num_cells); } } + void swap(stableBF& rhs) { std::swap(d_k, rhs.d_k); @@ -160,29 +181,32 @@ private: std::swap(d_p, rhs.d_p); d_cells.swap(rhs.d_cells); } + // This is a double hash implementation returning an array of // k hashes - std::vector hash(const std::string& data) const + [[nodiscard]] std::vector hash(const std::string& data) const { - uint32_t h1, h2; + uint32_t hash1{}; + uint32_t hash2{}; // MurmurHash3 assumes the data is uint32_t aligned, so fixup if needed // It does handle string lengths that are not a multiple of sizeof(uint32_t) correctly - if (reinterpret_cast(data.data()) % sizeof(uint32_t) != 0) { - NoInitVector x((data.length() / sizeof(uint32_t)) + 1); - memcpy(x.data(), data.data(), data.length()); - MurmurHash3_x86_32(x.data(), data.length(), 1, &h1); - MurmurHash3_x86_32(x.data(), data.length(), 2, &h2); + if (reinterpret_cast(data.data()) % sizeof(uint32_t) != 0) { // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + NoInitVector vec((data.length() / sizeof(uint32_t)) + 1); + memcpy(vec.data(), data.data(), data.length()); + MurmurHash3_x86_32(vec.data(), static_cast(data.length()), 1, &hash1); + MurmurHash3_x86_32(vec.data(), static_cast(data.length()), 2, &hash2); } else { - MurmurHash3_x86_32(data.data(), data.length(), 1, &h1); - MurmurHash3_x86_32(data.data(), data.length(), 2, &h2); + MurmurHash3_x86_32(data.data(), static_cast(data.length()), 1, &hash1); + MurmurHash3_x86_32(data.data(), static_cast(data.length()), 2, &hash2); } std::vector ret_hashes(d_k); for (size_t i = 0; i < d_k; ++i) { - ret_hashes[i] = h1 + i * h2; + ret_hashes[i] = hash1 + i * hash2; } return ret_hashes; } + uint8_t d_k; uint32_t d_num_cells; uint8_t d_p;