From: Fred Morcos Date: Thu, 28 Sep 2023 20:37:56 +0000 (+0200) Subject: Minor cleanups to geoip and lmdb X-Git-Tag: rec-5.0.0-alpha2~10^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0706da10180221cdba56c0f30c7513389b9f8670;p=thirdparty%2Fpdns.git Minor cleanups to geoip and lmdb --- diff --git a/ext/lmdb-safe/lmdb-typed.hh b/ext/lmdb-safe/lmdb-typed.hh index 790fead291..ae32d78fd6 100644 --- a/ext/lmdb-safe/lmdb-typed.hh +++ b/ext/lmdb-safe/lmdb-typed.hh @@ -162,8 +162,6 @@ struct LMDBIndexOps auto scombined = makeCombinedKey(keyConv(d_parent->getMember(t)), id); MDBInVal combined(scombined); - MDBOutVal currentvalue; - // if the entry existed already, this will just update the timestamp/txid in the LS header. This is intentional, so objects and their indexes always get synced together. txn->put(d_idx, combined, empty, flags); } diff --git a/modules/geoipbackend/geoipbackend.cc b/modules/geoipbackend/geoipbackend.cc index ada8093b41..497d80c860 100644 --- a/modules/geoipbackend/geoipbackend.cc +++ b/modules/geoipbackend/geoipbackend.cc @@ -38,8 +38,8 @@ ReadWriteLock GeoIPBackend::s_state_lock; struct GeoIPDNSResourceRecord : DNSResourceRecord { - int weight; - bool has_weight; + int weight{}; + bool has_weight{}; }; struct GeoIPService @@ -53,7 +53,7 @@ struct GeoIPDomain { int id; DNSName domain; - int ttl; + int ttl{}; map services; map> records; vector mapping_lookup_formats; @@ -77,16 +77,15 @@ const static std::array GeoIP_MONTHS = {"jan", "feb", "mar", "apr", GeoIPBackend::GeoIPBackend(const string& suffix) { - WriteLock wl(&s_state_lock); - d_dnssec = false; + WriteLock writeLock(&s_state_lock); setArgPrefix("geoip" + suffix); - if (getArg("dnssec-keydir").empty() == false) { - DIR* d = opendir(getArg("dnssec-keydir").c_str()); - if (d == nullptr) { + if (!getArg("dnssec-keydir").empty()) { + DIR* dir = opendir(getArg("dnssec-keydir").c_str()); + if (dir == nullptr) { throw PDNSException("dnssec-keydir " + getArg("dnssec-keydir") + " does not exist"); } d_dnssec = true; - closedir(d); + closedir(dir); } if (s_rc == 0) { // first instance gets to open everything initialize(); @@ -104,14 +103,17 @@ static string queryGeoIP(const Netmask& addr, GeoIPInterface::GeoIPQueryAttribut // infinite recursion. static bool validateMappingLookupFormats(const vector& formats) { - string::size_type cur, last; + string::size_type cur = 0; + string::size_type last = 0; + for (const auto& lookupFormat : formats) { last = 0; while ((cur = lookupFormat.find("%", last)) != string::npos) { - if (!lookupFormat.compare(cur, 3, "%mp")) { + if (lookupFormat.compare(cur, 3, "%mp") == 0) { return false; } - else if (!lookupFormat.compare(cur, 2, "%%")) { // Ensure escaped % is also accepted + + if (lookupFormat.compare(cur, 2, "%%") == 0) { // Ensure escaped % is also accepted last = cur + 2; continue; } @@ -127,12 +129,12 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo dom.domain = DNSName(domain["domain"].as()); dom.ttl = domain["ttl"].as(); - for (YAML::const_iterator recs = domain["records"].begin(); recs != domain["records"].end(); recs++) { + for (auto recs = domain["records"].begin(); recs != domain["records"].end(); recs++) { DNSName qname = DNSName(recs->first.as()); vector rrs; - for (YAML::const_iterator item = recs->second.begin(); item != recs->second.end(); item++) { - YAML::const_iterator rec = item->begin(); + for (auto item = recs->second.begin(); item != recs->second.end(); item++) { + auto rec = item->begin(); GeoIPDNSResourceRecord rr; rr.domain_id = dom.id; rr.ttl = dom.ttl; @@ -151,9 +153,9 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo } else if (rec->second.IsMap()) { for (YAML::const_iterator iter = rec->second.begin(); iter != rec->second.end(); iter++) { - string attr = iter->first.as(); + auto attr = iter->first.as(); if (attr == "content") { - string content = iter->second.as(); + auto content = iter->second.as(); rr.content = std::move(content); } else if (attr == "weight") { @@ -174,24 +176,25 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo } } else { - string content = rec->second.as(); + auto content = rec->second.as(); rr.content = std::move(content); rr.weight = 100; } - rr.auth = 1; + rr.auth = true; rrs.push_back(rr); } std::swap(dom.records[qname], rrs); } - for (YAML::const_iterator service = domain["services"].begin(); service != domain["services"].end(); service++) { - unsigned int netmask4 = 0, netmask6 = 0; + for (auto service = domain["services"].begin(); service != domain["services"].end(); service++) { + unsigned int netmask4 = 0; + unsigned int netmask6 = 0; DNSName srvName{service->first.as()}; NetmaskTree> nmt; // if it's an another map, we need to iterate it again, otherwise we just add two root entries. if (service->second.IsMap()) { - for (YAML::const_iterator net = service->second.begin(); net != service->second.end(); net++) { + for (auto net = service->second.begin(); net != service->second.end(); net++) { vector value; if (net->second.IsSequence()) { value = net->second.as>(); @@ -204,12 +207,14 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo nmt.insert(Netmask("::/0")).second.swap(value); } else { - Netmask nm{net->first.as()}; - nmt.insert(nm).second.swap(value); - if (nm.isIPv6() == true && netmask6 < nm.getBits()) - netmask6 = nm.getBits(); - if (nm.isIPv6() == false && netmask4 < nm.getBits()) - netmask4 = nm.getBits(); + Netmask netmask{net->first.as()}; + nmt.insert(netmask).second.swap(value); + if (netmask.isIPv6() && netmask6 < netmask.getBits()) { + netmask6 = netmask.getBits(); + } + if (!netmask.isIPv6() && netmask4 < netmask.getBits()) { + netmask4 = netmask.getBits(); + } } } } @@ -228,9 +233,10 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo // Allow per domain override of mapping_lookup_formats and custom_mapping. // If not defined, the global values will be used. if (YAML::Node formats = domain["mapping_lookup_formats"]) { - vector mapping_lookup_formats = formats.as>(); - if (!validateMappingLookupFormats(mapping_lookup_formats)) + auto mapping_lookup_formats = formats.as>(); + if (!validateMappingLookupFormats(mapping_lookup_formats)) { throw PDNSException(string("%mp is not allowed in mapping lookup formats of domain ") + dom.domain.toLogString()); + } dom.mapping_lookup_formats = mapping_lookup_formats; } @@ -254,8 +260,8 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo // ensure we have parent in records DNSName name = item.first; while (name.chopOff() && name.isPartOf(dom.domain)) { - if (dom.records.find(name) == dom.records.end() && !dom.services.count(name)) { // don't ENT out a service! GeoIPDNSResourceRecord rr; + if (dom.records.find(name) == dom.records.end() && (dom.services.count(name) == 0U)) { // don't ENT out a service! vector rrs; rr.domain_id = dom.id; rr.ttl = dom.ttl; @@ -300,25 +306,27 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo map lasts; bool has_weight = false; // first we look for used weight - for (const auto& rr : item.second) { - weights[rr.qtype.getCode()] += rr.weight; - if (rr.has_weight) + for (const auto& resourceRecord : item.second) { + weights[resourceRecord.qtype.getCode()] += static_cast(resourceRecord.weight); + if (resourceRecord.has_weight) { has_weight = true; + } } if (has_weight) { // put them back as probabilities and values.. - for (auto& rr : item.second) { - uint16_t rr_type = rr.qtype.getCode(); - rr.weight = static_cast((static_cast(rr.weight) / weights[rr_type]) * 1000.0); - sums[rr_type] += rr.weight; - rr.has_weight = has_weight; - lasts[rr_type] = &rr; + for (auto& resourceRecord : item.second) { + uint16_t rr_type = resourceRecord.qtype.getCode(); + resourceRecord.weight = static_cast((static_cast(resourceRecord.weight) / weights[rr_type]) * 1000.0); + sums[rr_type] += static_cast(resourceRecord.weight); + resourceRecord.has_weight = has_weight; + lasts[rr_type] = &resourceRecord; } // remove rounding gap for (auto& x : lasts) { float sum = sums[x.first]; - if (sum < 1000) + if (sum < 1000) { x.second->weight += (1000 - sum); + } } } } @@ -337,9 +345,11 @@ bool GeoIPBackend::loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDo void GeoIPBackend::loadDomainsFromDirectory(const std::string& dir, vector& domains) { vector paths; - for (const std::filesystem::path& p : std::filesystem::directory_iterator(std::filesystem::path(dir))) - if (std::filesystem::is_regular_file(p) && p.has_extension() && (p.extension() == ".yaml" || p.extension() == ".yml")) + for (const std::filesystem::path& p : std::filesystem::directory_iterator(std::filesystem::path(dir))) { + if (std::filesystem::is_regular_file(p) && p.has_extension() && (p.extension() == ".yaml" || p.extension() == ".yml")) { paths.push_back(p); + } + } std::sort(paths.begin(), paths.end()); for (const auto& p : paths) { try { @@ -347,8 +357,9 @@ void GeoIPBackend::loadDomainsFromDirectory(const std::string& dir, vector>(); - if (!validateMappingLookupFormats(d_global_mapping_lookup_formats)) + if (!validateMappingLookupFormats(d_global_mapping_lookup_formats)) { throw PDNSException(string("%mp is not allowed in mapping lookup")); + } } if (YAML::Node mapping = config["custom_mapping"]) { d_global_custom_mapping = mapping.as>(); @@ -397,12 +410,14 @@ void GeoIPBackend::initialize() for (YAML::const_iterator _domain = config["domains"].begin(); _domain != config["domains"].end(); _domain++) { GeoIPDomain dom; auto id = tmp_domains.size(); - if (loadDomain(*_domain, id, dom)) + if (loadDomain(*_domain, id, dom)) { tmp_domains.push_back(std::move(dom)); + } } - if (YAML::Node domain_dir = config["zones_dir"]) + if (YAML::Node domain_dir = config["zones_dir"]) { loadDomainsFromDirectory(domain_dir.as(), tmp_domains); + } s_domains.clear(); std::swap(s_domains, tmp_domains); @@ -414,7 +429,7 @@ void GeoIPBackend::initialize() GeoIPBackend::~GeoIPBackend() { try { - WriteLock wl(&s_state_lock); + WriteLock writeLock(&s_state_lock); s_rc--; if (s_rc == 0) { // last instance gets to cleanup s_geoip_files.clear(); diff --git a/modules/geoipbackend/geoipbackend.hh b/modules/geoipbackend/geoipbackend.hh index c7e988a2f8..851927628d 100644 --- a/modules/geoipbackend/geoipbackend.hh +++ b/modules/geoipbackend/geoipbackend.hh @@ -79,10 +79,10 @@ private: void initialize(); string format2str(string format, const Netmask& addr, GeoIPNetmask& gl, const GeoIPDomain& dom); - bool d_dnssec; + bool d_dnssec{}; bool hasDNSSECkey(const DNSName& name); bool lookup_static(const GeoIPDomain& dom, const DNSName& search, const QType& qtype, const DNSName& qdomain, const Netmask& addr, GeoIPNetmask& gl); - bool loadDomain(const YAML::Node& domain, unsigned int id, GeoIPDomain& dom); + bool loadDomain(const YAML::Node& domain, unsigned int /* id */, GeoIPDomain& dom); void loadDomainsFromDirectory(const std::string& dir, vector& domains); vector d_result; vector d_files; diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index 9b600f1370..60a05cac1d 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -464,7 +464,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) int index = 0; - for (const std::string& dbname : {"domains", "keydata", "tsig", "metadata"}) { + for (const std::string dbname : {"domains", "keydata", "tsig", "metadata"}) { std::cerr << "migrating " << dbname << std::endl; std::string tdbname = dbname + "_v5"; @@ -504,7 +504,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) index = 0; - for (const std::string& dbname : {"domains", "keydata", "tsig", "metadata"}) { + for (const std::string dbname : {"domains", "keydata", "tsig", "metadata"}) { std::string fdbname = dbname + "_0"; std::cerr << "migrating " << dbname << std::endl; std::string tdbname = dbname + "_v5_0"; @@ -553,7 +553,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) std::string header(LMDBLS::LS_MIN_HEADER_SIZE, '\0'); - for (const std::string& keyname : {"schemaversion", "shards"}) { + for (const std::string keyname : {"schemaversion", "shards"}) { cerr << "migrating pdns." << keyname << endl; key.mv_data = (char*)keyname.c_str(); @@ -591,7 +591,7 @@ bool LMDBBackend::upgradeToSchemav5(std::string& filename) } } - for (const std::string& keyname : {"uuid"}) { + for (const std::string keyname : {"uuid"}) { cerr << "migrating pdns." << keyname << endl; key.mv_data = (char*)keyname.c_str();