From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 9 Jan 2023 12:42:44 +0000 (+0100)
Subject: Simplify code by using more methods already existing and correct entry count when... 
X-Git-Tag: dnsdist-1.8.0-rc1~93^2~7
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9597df1a8fd31909adcb8b6cd3ae8ff33a7462c8;p=thirdparty%2Fpdns.git

Simplify code by using more methods already existing and correct entry count when deleting from the negcache.
---

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index af33d7dc3f..195a1dc64a 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -44,25 +44,6 @@ size_t NegCache::size() const
   return count;
 }
 
-void NegCache::purge(const DNSName& qname, QType qtype)
-{
-  auto& mc = getMap(qname);
-  auto content = mc.lock();
-  auto& idx = content->d_map.get<NegCacheEntry>();
-
-  auto range = idx.equal_range(qname);
-  auto ni = range.first;
-
-  while (ni != range.second) {
-    // We have an entry
-    if ((ni->d_qtype == QType::ENT) || ni->d_qtype == qtype) {
-        ni = idx.erase(ni);
-    }
-    else
-        ++ni;
-  }
-}
-
 /*!
  * Set ne to the NegCacheEntry for the last label in qname and return true if there
  * was one.
@@ -261,6 +242,26 @@ size_t NegCache::wipe(const DNSName& name, bool subtree)
   return ret;
 }
 
+size_t NegCache::wipe(const DNSName& qname, QType qtype)
+{
+  size_t ret = 0;
+  auto& map = getMap(qname);
+  auto content = map.lock();
+  auto range = content->d_map.equal_range(std::tie(qname));
+  auto i = range.first;
+  while (i != range.second) {
+    if (i->d_qtype == QType::ENT || i->d_qtype == qtype) {
+      i = content->d_map.erase(i);
+      ++ret;
+      --map.d_entriesCount;
+    }
+    else {
+      ++i;
+    }
+  }
+  return ret;
+}
+
 /*!
  * Clear the negative cache
  */
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index d3d61f7cbf..97e9be32a9 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -96,9 +96,8 @@ public:
   void clear();
   size_t doDump(int fd, size_t maxCacheEntries);
   size_t wipe(const DNSName& name, bool subtree = false);
+  size_t wipe(const DNSName& name, QType qtype);
   size_t size() const;
-  void purge(const DNSName& qname, QType qtype);
-
 
 private:
   struct CompositeKey
diff --git a/pdns/recursordist/recursor_cache.cc b/pdns/recursordist/recursor_cache.cc
index 984cc81a0d..2bf5d556e9 100644
--- a/pdns/recursordist/recursor_cache.cc
+++ b/pdns/recursordist/recursor_cache.cc
@@ -316,20 +316,6 @@ bool MemRecursorCache::entryMatches(MemRecursorCache::OrderedTagIterator_t& entr
   return match;
 }
 
-void MemRecursorCache::purge(const DNSName& qname, QType qt)
-{
-   if(qt == QType::CNAME)
-   {
-        auto& mc = getMap(qname);
-        auto map = mc.lock();
-        auto key = std::make_tuple(qname, qt, boost::none, Netmask());
-        auto entry = map->d_map.find(key);
-        if (entry != map->d_map.end()) {
-            map->d_map.erase(entry);
-        }
-   }  
-}
-
 // Fake a cache miss if more than refreshTTLPerc of the original TTL has passed
 time_t MemRecursorCache::fakeTTD(MemRecursorCache::OrderedTagIterator_t& entry, const DNSName& qname, QType qtype, time_t ret, time_t now, uint32_t origTTL, bool refresh)
 {
diff --git a/pdns/recursordist/recursor_cache.hh b/pdns/recursordist/recursor_cache.hh
index ae9981f53c..91102d99ae 100644
--- a/pdns/recursordist/recursor_cache.hh
+++ b/pdns/recursordist/recursor_cache.hh
@@ -72,7 +72,6 @@ public:
   void replace(time_t, const DNSName& qname, const QType qt, const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, const std::vector<std::shared_ptr<DNSRecord>>& authorityRecs, bool auth, const DNSName& authZone, boost::optional<Netmask> ednsmask = boost::none, const OptTag& routingTag = boost::none, vState state = vState::Indeterminate, boost::optional<ComboAddress> from = boost::none, bool refresh = false);
 
   void doPrune(size_t keep);
-  void purge(const DNSName& qname, QType qt);
   uint64_t doDump(int fd, size_t maxCacheEntries);
 
   size_t doWipeCache(const DNSName& name, bool sub, QType qtype = 0xffff);
diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 5e23111624..2429bb3c2c 100644
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -4637,9 +4637,9 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
           rememberParentSetIfNeeded(i->first.name, i->second.records, depth);
         }
         g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, authorityRecs, i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh);
-        // delete negcache
-        g_negCache->purge(i->first.name, i->first.type);
-        
+        // delete negcache entry
+        g_negCache->wipe(i->first.name, i->first.type);
+
         if (g_aggressiveNSECCache && needWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->second.signatures.empty() && !d_routingTag && !ednsmask) {
           /* we have an answer synthesized from a wildcard and aggressive NSEC is enabled, we need to store the
              wildcard in its non-expanded form in the cache to be able to synthesize wildcard answers later */
@@ -4805,8 +4805,8 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
       */
       if (newtarget.empty() && putInNegCache) {
         g_negCache->add(ne);
-        if(qtype == QType::CNAME){
-            g_recCache->purge(qname, qtype);
+        if (qtype == QType::CNAME) {
+          g_recCache->doWipeCache(qname, false, qtype);
         }
         if (s_rootNXTrust && ne.d_auth.isRoot() && auth.isRoot() && lwr.d_aabit) {
           ne.d_name = ne.d_name.getLastLabel();