Minor cleanups to geoip and lmdb

diff --git a/ext/lmdb-safe/lmdb-typed.hh b/ext/lmdb-safe/lmdb-typed.hh
index 790fead2910b8308aab63d97eea964ec70864036..ae32d78fd67b26a37bdcd969a6c2f4b2253d74d5 100644 (file)
--- 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 ada8093b41a7d8c66f4ab78e166e270b9a0ca561..497d80c860cf0c9f6afe8a6b1f19ca730e15039f 100644 (file)
--- 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<DNSName, GeoIPService> services;
   map<DNSName, vector<GeoIPDNSResourceRecord>> records;
   vector<string> mapping_lookup_formats;
@@ -77,16 +77,15 @@ const static std::array<string, 12> 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<string>& 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<string>());
     dom.ttl = domain["ttl"].as<int>();
 
-    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<string>());
       vector<GeoIPDNSResourceRecord> 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<string>();
+            auto attr = iter->first.as<string>();
             if (attr == "content") {
-              string content = iter->second.as<string>();
+              auto content = iter->second.as<string>();
               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<string>();
+          auto content = rec->second.as<string>();
           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<string>()};
       NetmaskTree<vector<string>> 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<string> value;
           if (net->second.IsSequence()) {
             value = net->second.as<vector<string>>();
@@ -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<string>()};
-            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<string>()};
+            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<string> mapping_lookup_formats = formats.as<vector<string>>();
-        if (!validateMappingLookupFormats(mapping_lookup_formats))
+        auto mapping_lookup_formats = formats.as<vector<string>>();
+        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<GeoIPDNSResourceRecord> 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<uint16_t, GeoIPDNSResourceRecord*> 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<float>(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<int>((static_cast<float>(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<int>((static_cast<float>(resourceRecord.weight) / weights[rr_type]) * 1000.0);
+          sums[rr_type] += static_cast<float>(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<GeoIPDomain>& domains)
 {
   vector<std::filesystem::path> 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<GeoIP
       const auto& zoneRoot = YAML::LoadFile(p.string());
       // expect zone key
       const auto& zone = zoneRoot["zone"];
-      if (loadDomain(zone, domains.size(), dom))
+      if (loadDomain(zone, domains.size(), dom)) {
         domains.push_back(dom);
+      }
     }
     catch (std::exception& ex) {
       g_log << Logger::Warning << "Cannot load zone from " << p << ": " << ex.what() << endl;
@@ -371,8 +382,9 @@ void GeoIPBackend::initialize()
     }
   }
 
-  if (s_geoip_files.empty())
+  if (s_geoip_files.empty()) {
     g_log << Logger::Warning << "No GeoIP database files loaded!" << endl;
+  }
 
   if (!getArg("zones-file").empty()) {
     try {
@@ -387,8 +399,9 @@ void GeoIPBackend::initialize()
   // if none defined at the domain level.
   if (YAML::Node formats = config["mapping_lookup_formats"]) {
     d_global_mapping_lookup_formats = formats.as<vector<string>>();
-    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<map<std::string, std::string>>();
@@ -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<string>(), 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 c7e988a2f8ec1eaa7a0a3f24a66cea46ae4e443d..851927628d1f34f4f9c62984f07741a2b53e1782 100644 (file)
--- 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<GeoIPDomain>& domains);
   vector<DNSResourceRecord> d_result;
   vector<GeoIPInterface> d_files;

diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index 9b600f1370643e681e42b61f322f33e9eaed5767..60a05cac1d099e35ea0bdda84e8bc833cd867589 100644 (file)
--- 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();