From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 19 Sep 2023 06:57:15 +0000 (+0200)
Subject: Some delinting of common cache code
X-Git-Tag: rec-5.0.0-alpha2~52^2~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=662dda3ead35bb59fd5ab4e45b7626fe54b2a6f0;p=thirdparty%2Fpdns.git

Some delinting of common cache code
---

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index 9ec8e13c22..8373ae6f01 100644
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -167,7 +167,7 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
         container.preRemoval(*shard, *i);
         i = sidx.erase(i);
         erased++;
-        --content.d_entriesCount;
+        content.decEntriesCount();
       }
       else {
         ++i;
@@ -226,7 +226,7 @@ uint64_t pruneMutexCollectionsVector(C& container, std::vector<T>& maps, uint64_
     for (auto i = sidx.begin(); i != sidx.end() && removed < toTrimForThisShard; removed++) {
       container.preRemoval(*shard, *i);
       i = sidx.erase(i);
-      --content.d_entriesCount;
+      content.decEntriesCount();
       ++totErased;
       if (--toTrim == 0) {
         return totErased;
diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 57ac811e13..be67da138b 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -39,7 +39,7 @@ size_t NegCache::size() const
 {
   size_t count = 0;
   for (const auto& map : d_maps) {
-    count += map.d_entriesCount;
+    count += map.getEntriesCount();
   }
   return count;
 }
@@ -161,7 +161,7 @@ void NegCache::add(const NegCacheEntry& ne)
   auto content = map.lock();
   inserted = lruReplacingInsert<SequenceTag>(content->d_map, ne);
   if (inserted) {
-    ++map.d_entriesCount;
+    map.incEntriesCount();
   }
 }
 
@@ -229,7 +229,7 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
           break;
         i = m->d_map.erase(i);
         ret++;
-        --map.d_entriesCount;
+        map.decEntriesCount();
       }
     }
     return ret;
@@ -242,7 +242,7 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
   while (i != range.second) {
     i = content->d_map.erase(i);
     ret++;
-    --map.d_entriesCount;
+    map.decEntriesCount();
   }
   return ret;
 }
@@ -258,7 +258,7 @@ size_t NegCache::wipeTyped(const DNSName& qname, QType qtype)
     if (i->d_qtype == QType::ENT || i->d_qtype == qtype) {
       i = content->d_map.erase(i);
       ++ret;
-      --map.d_entriesCount;
+      map.decEntriesCount();
     }
     else {
       ++i;
@@ -275,7 +275,7 @@ void NegCache::clear()
   for (auto& map : d_maps) {
     auto m = map.lock();
     m->d_map.clear();
-    map.d_entriesCount = 0;
+    map.clearEntriesCount();
   }
 }
 
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 90d64ee822..1ac2855e31 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -141,7 +141,6 @@ private:
       uint64_t d_acquired_count{0};
       void invalidate() {}
     };
-    pdns::stat_t d_entriesCount{0};
 
     LockGuardedTryHolder<MapCombo::LockedContent> lock()
     {
@@ -154,8 +153,29 @@ private:
       return locked;
     }
 
+    [[nodiscard]] auto getEntriesCount() const
+    {
+      return d_entriesCount.load();
+    }
+
+    void incEntriesCount()
+    {
+      ++d_entriesCount;
+    }
+
+    void decEntriesCount()
+    {
+      --d_entriesCount;
+    }
+
+    void clearEntriesCount()
+    {
+      d_entriesCount = 0;
+    }
+
   private:
     LockGuarded<LockedContent> d_content;
+    pdns::stat_t d_entriesCount{0};
   };
 
   vector<MapCombo> d_maps;
diff --git a/pdns/recursordist/recpacketcache.cc b/pdns/recursordist/recpacketcache.cc
index 77dc63e1e1..4ae3cb5170 100644
--- a/pdns/recursordist/recpacketcache.cc
+++ b/pdns/recursordist/recpacketcache.cc
@@ -24,7 +24,7 @@ uint64_t RecursorPacketCache::size() const
 {
   uint64_t count = 0;
   for (const auto& map : d_maps) {
-    count += map.d_entriesCount;
+    count += map.getEntriesCount();
   }
   return count;
 }
@@ -92,7 +92,7 @@ uint64_t RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qt
       }
       if (qtype == 0xffff || iter->d_type == qtype) {
         iter = idx.erase(iter);
-        map.d_entriesCount--;
+        map.decEntriesCount();
         count++;
       }
       else {
@@ -235,20 +235,20 @@ void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash,
     return;
   }
 
-  struct Entry entry(qname, qtype, qclass, std::move(responsePacket), std::move(query), tcp, qhash, now + ttl, now, tag, valState);
+  struct Entry entry(DNSName(qname), qtype, qclass, std::move(responsePacket), std::move(query), tcp, qhash, now + ttl, now, tag, valState);
   if (pbdata) {
     entry.d_pbdata = std::move(*pbdata);
   }
 
   shard->d_map.insert(entry);
-  map.d_entriesCount++;
+  map.incEntriesCount();
 
   if (shard->d_map.size() > shard->d_shardSize) {
     auto& seq_idx = shard->d_map.get<SequencedTag>();
     seq_idx.erase(seq_idx.begin());
-    map.d_entriesCount--;
+    map.decEntriesCount();
   }
-  assert(map.d_entriesCount == shard->d_map.size()); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay): clib implementation
+  assert(map.getEntriesCount() == shard->d_map.size()); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay): clib implementation
 }
 
 void RecursorPacketCache::doPruneTo(size_t maxSize)
diff --git a/pdns/recursordist/recpacketcache.hh b/pdns/recursordist/recpacketcache.hh
index 2e847687a1..5a8c23a606 100644
--- a/pdns/recursordist/recpacketcache.hh
+++ b/pdns/recursordist/recpacketcache.hh
@@ -21,7 +21,7 @@
  */
 #pragma once
 #include <string>
-#include <inttypes.h>
+#include <cinttypes>
 #include "dns.hh"
 #include "namespaces.hh"
 #include <iostream>
@@ -104,9 +104,9 @@ public:
 private:
   struct Entry
   {
-    Entry(const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& packet, std::string&& query, bool tcp,
-          uint32_t qhash, time_t ttd, time_t now, uint32_t tag, vState vstate) :
-      d_name(qname), d_packet(std::move(packet)), d_query(std::move(query)), d_ttd(ttd), d_creation(now), d_qhash(qhash), d_tag(tag), d_type(qtype), d_class(qclass), d_vstate(vstate), d_tcp(tcp)
+    Entry(DNSName&& qname, uint16_t qtype, uint16_t qclass, std::string&& packet, std::string&& query, bool tcp,
+          uint32_t qhash, time_t ttd, time_t now, uint32_t tag, vState vstate) : // NOLINT
+      d_name(std::move(qname)), d_packet(std::move(packet)), d_query(std::move(query)), d_ttd(ttd), d_creation(now), d_qhash(qhash), d_tag(tag), d_type(qtype), d_class(qclass), d_vstate(vstate), d_tcp(tcp)
     {
     }
 
@@ -157,8 +157,12 @@ private:
   struct MapCombo
   {
     MapCombo() = default;
+    ~MapCombo() = default;
     MapCombo(const MapCombo&) = delete;
+    MapCombo(MapCombo&&) = delete;
     MapCombo& operator=(const MapCombo&) = delete;
+    MapCombo& operator=(MapCombo&&) = delete;
+
     struct LockedContent
     {
       packetCache_t d_map;
@@ -169,7 +173,6 @@ private:
       uint64_t d_acquired_count{0};
       void invalidate() {}
     };
-    pdns::stat_t d_entriesCount{0};
 
     LockGuardedTryHolder<MapCombo::LockedContent> lock()
     {
@@ -182,8 +185,24 @@ private:
       return locked;
     }
 
+    [[nodiscard]] auto getEntriesCount() const
+    {
+      return d_entriesCount.load();
+    }
+
+    void incEntriesCount()
+    {
+      ++d_entriesCount;
+    }
+
+    void decEntriesCount()
+    {
+      --d_entriesCount;
+    }
+
   private:
     LockGuarded<LockedContent> d_content;
+    pdns::stat_t d_entriesCount{0};
   };
 
   vector<MapCombo> d_maps;
diff --git a/pdns/recursordist/recursor_cache.cc b/pdns/recursordist/recursor_cache.cc
index 60116e9ff1..d3e434017e 100644
--- a/pdns/recursordist/recursor_cache.cc
+++ b/pdns/recursordist/recursor_cache.cc
@@ -63,7 +63,7 @@ size_t MemRecursorCache::size() const
 {
   size_t count = 0;
   for (const auto& shard : d_maps) {
-    count += shard.d_entriesCount;
+    count += shard.getEntriesCount();
   }
   return count;
 }
@@ -548,7 +548,7 @@ void MemRecursorCache::replace(time_t now, const DNSName& qname, const QType qt,
   cache_t::iterator stored = lockedShard->d_map.find(key);
   if (stored == lockedShard->d_map.end()) {
     stored = lockedShard->d_map.insert(CacheEntry(key, auth)).first;
-    ++shard.d_entriesCount;
+    shard.incEntriesCount();
     isNew = true;
   }
 
@@ -641,7 +641,7 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
       if (i->d_qtype == qtype || qtype == 0xffff) {
         i = idx.erase(i);
         count++;
-        --shard.d_entriesCount;
+        shard.decEntriesCount();
       }
       else {
         ++i;
@@ -670,7 +670,7 @@ size_t MemRecursorCache::doWipeCache(const DNSName& name, bool sub, const QType
         if (i->d_qtype == qtype || qtype == 0xffff) {
           count++;
           i = idx.erase(i);
-          --mc.d_entriesCount;
+          mc.decEntriesCount();
         }
         else {
           ++i;
diff --git a/pdns/recursordist/recursor_cache.hh b/pdns/recursordist/recursor_cache.hh
index 379249b610..7cc2f0316c 100644
--- a/pdns/recursordist/recursor_cache.hh
+++ b/pdns/recursordist/recursor_cache.hh
@@ -249,8 +249,6 @@ private:
       }
     };
 
-    pdns::stat_t d_entriesCount{0};
-
     LockGuardedTryHolder<LockedContent> lock()
     {
       auto locked = d_content.try_lock();
@@ -262,8 +260,29 @@ private:
       return locked;
     }
 
+    [[nodiscard]] auto getEntriesCount() const
+    {
+      return d_entriesCount.load();
+    }
+
+    void incEntriesCount()
+    {
+      ++d_entriesCount;
+    }
+
+    void decEntriesCount()
+    {
+      --d_entriesCount;
+    }
+
+    void clearEntriesCount()
+    {
+      d_entriesCount = 0;
+    }
+
   private:
     LockGuarded<LockedContent> d_content;
+    pdns::stat_t d_entriesCount{0};
   };
 
   vector<MapCombo> d_maps;