rec: Copy entries retrieved from the negative cache right away

The optimization of not copying the entry until we actually decide
to use it seemed nice, but the resulting interface is too brittle.
It requires not keeping the pointer around if there is any chance
that we could yield by going to the network, which is hard to keep
track of in the recursor.

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index a48107e761c633f0f8cb057635897ed6636c6b6e..057bb1cafc071c0f0bc1959ac676ffc53494d3d3 100644 (file)
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -35,7 +35,7 @@
  * \param ne       A NegCacheEntry that is filled when there is a cache entry
  * \return         true if ne was filled out, false otherwise
  */
-bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne)
+bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne)
 {
   // Never deny the root.
   if (qname.isRoot())
@@ -49,7 +49,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, c
   while (ni != d_negcache.end() && ni->d_name == lastLabel && ni->d_auth.isRoot() && ni->d_qtype == qtnull) {
     // We have something
     if ((uint32_t)now.tv_sec < ni->d_ttd) {
-      *ne = &(*ni);
+      ne = *ni;
       moveCacheItemToBack<SequenceTag>(d_negcache, ni);
       return true;
     }
@@ -68,7 +68,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, c
  * \param ne       A NegCacheEntry that is filled when there is a cache entry
  * \return         true if ne was filled out, false otherwise
  */
-bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch)
+bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch)
 {
   const auto& idx = d_negcache.get<2>();
   auto range = idx.equal_range(qname);
@@ -82,7 +82,7 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva
 
       if ((uint32_t)now.tv_sec < ni->d_ttd) {
         // Not expired
-        *ne = &(*ni);
+        ne = *ni;
         moveCacheItemToBack<SequenceTag>(d_negcache, firstIndexIterator);
         return true;
       }

diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 55200ff4a257a4d65c78efc8413d405999bb772a..0b5ecffca90a5434cde6861dc7abd02e7b89d256 100644 (file)
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -64,8 +64,8 @@ public:
 
   void add(const NegCacheEntry& ne);
   void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState, boost::optional<uint32_t> capTTD);
-  bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch = false);
-  bool getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne);
+  bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch = false);
+  bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne);
   uint64_t count(const DNSName& qname) const;
   uint64_t count(const DNSName& qname, const QType qtype) const;
   void prune(unsigned int maxEntries);

diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc
index 5a96f2fd0ac6a536625204fe7d1f24862343749b..0c33e1cb3ac494c57c86a2b0aadac42a63044c40 100644 (file)
--- a/pdns/recursordist/test-negcache_cc.cc
+++ b/pdns/recursordist/test-negcache_cc.cc
@@ -60,13 +60,13 @@ BOOST_AUTO_TEST_CASE(test_get_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.get(qname, QType(1), now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.get(qname, QType(1), now, ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne->d_name, qname);
-  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(0).getName());
-  BOOST_CHECK_EQUAL(ne->d_auth, auth);
+  BOOST_CHECK_EQUAL(ne.d_name, qname);
+  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(0).getName());
+  BOOST_CHECK_EQUAL(ne.d_auth, auth);
 }
 
 BOOST_AUTO_TEST_CASE(test_get_entry_exact_type)
@@ -85,11 +85,10 @@ BOOST_AUTO_TEST_CASE(test_get_entry_exact_type)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.get(qname, QType(1), now, &ne, true);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.get(qname, QType(1), now, ne, true);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_get_NODATA_entry)
@@ -105,18 +104,17 @@ BOOST_AUTO_TEST_CASE(test_get_NODATA_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.get(qname, QType(1), now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.get(qname, QType(1), now, ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne->d_name, qname);
-  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(1).getName());
-  BOOST_CHECK_EQUAL(ne->d_auth, auth);
+  BOOST_CHECK_EQUAL(ne.d_name, qname);
+  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(1).getName());
+  BOOST_CHECK_EQUAL(ne.d_auth, auth);
 
-  const NegCache::NegCacheEntry* ne2 = nullptr;
-  ret = cache.get(qname, QType(16), now, &ne2);
+  NegCache::NegCacheEntry ne2;
+  ret = cache.get(qname, QType(16), now, ne2);
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne2 == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_entry)
@@ -132,13 +130,13 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.getRootNXTrust(qname, now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.getRootNXTrust(qname, now, ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne->d_name, qname);
-  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(0).getName());
-  BOOST_CHECK_EQUAL(ne->d_auth, auth);
+  BOOST_CHECK_EQUAL(ne.d_name, qname);
+  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(0).getName());
+  BOOST_CHECK_EQUAL(ne.d_auth, auth);
 }
 
 BOOST_AUTO_TEST_CASE(test_add_and_get_expired_entry)
@@ -155,13 +153,12 @@ BOOST_AUTO_TEST_CASE(test_add_and_get_expired_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
 
   now.tv_sec += 1000;
-  bool ret = cache.get(qname, QType(1), now, &ne);
+  bool ret = cache.get(qname, QType(1), now, ne);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_expired_entry)
@@ -178,13 +175,12 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_expired_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
 
   now.tv_sec += 1000;
-  bool ret = cache.getRootNXTrust(qname, now, &ne);
+  bool ret = cache.getRootNXTrust(qname, now, ne);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_add_updated_entry)
@@ -203,12 +199,12 @@ BOOST_AUTO_TEST_CASE(test_add_updated_entry)
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.get(qname, QType(1), now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.get(qname, QType(1), now, ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne->d_name, qname);
-  BOOST_CHECK_EQUAL(ne->d_auth, auth2);
+  BOOST_CHECK_EQUAL(ne.d_name, qname);
+  BOOST_CHECK_EQUAL(ne.d_auth, auth2);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust)
@@ -225,12 +221,12 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust)
   cache.add(genNegCacheEntry(qname, auth, now));
   cache.add(genNegCacheEntry(qname2, auth2, now));
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.getRootNXTrust(qname, now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.getRootNXTrust(qname, now, ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne->d_name, qname2);
-  BOOST_CHECK_EQUAL(ne->d_auth, auth2);
+  BOOST_CHECK_EQUAL(ne.d_name, qname2);
+  BOOST_CHECK_EQUAL(ne.d_auth, auth2);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_full_domain_only)
@@ -247,11 +243,10 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_full_domain_only)
   cache.add(genNegCacheEntry(qname, auth, now));
   cache.add(genNegCacheEntry(qname2, auth2, now, 1)); // Add the denial for COM|A
 
-  const NegCache::NegCacheEntry* ne = nullptr;
-  bool ret = cache.getRootNXTrust(qname, now, &ne);
+  NegCache::NegCacheEntry ne;
+  bool ret = cache.getRootNXTrust(qname, now, ne);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_prune)
@@ -301,11 +296,11 @@ BOOST_AUTO_TEST_CASE(test_prune_valid_entries)
   cache.prune(1);
   BOOST_CHECK_EQUAL(cache.size(), 1U);
 
-  const NegCache::NegCacheEntry* got = nullptr;
-  bool ret = cache.get(power2, QType(1), now, &got);
+  NegCache::NegCacheEntry got;
+  bool ret = cache.get(power2, QType(1), now, got);
   BOOST_REQUIRE(ret);
-  BOOST_CHECK_EQUAL(got->d_name, power2);
-  BOOST_CHECK_EQUAL(got->d_auth, auth);
+  BOOST_CHECK_EQUAL(got.d_name, power2);
+  BOOST_CHECK_EQUAL(got.d_auth, auth);
 
   /* insert power1 back */
   ne = genNegCacheEntry(power1, auth, now);
@@ -323,11 +318,11 @@ BOOST_AUTO_TEST_CASE(test_prune_valid_entries)
   cache.prune(1);
 
   BOOST_CHECK_EQUAL(cache.size(), 1U);
-  got = nullptr;
-  ret = cache.get(power2, QType(1), now, &got);
+  got = NegCache::NegCacheEntry();
+  ret = cache.get(power2, QType(1), now, got);
   BOOST_REQUIRE(ret);
-  BOOST_CHECK_EQUAL(got->d_name, power2);
-  BOOST_CHECK_EQUAL(got->d_auth, auth);
+  BOOST_CHECK_EQUAL(got.d_name, power2);
+  BOOST_CHECK_EQUAL(got.d_auth, auth);
 }
 
 BOOST_AUTO_TEST_CASE(test_wipe_single)
@@ -354,20 +349,18 @@ BOOST_AUTO_TEST_CASE(test_wipe_single)
   cache.wipe(auth);
   BOOST_CHECK_EQUAL(cache.size(), 400U);
 
-  const NegCache::NegCacheEntry* ne2 = nullptr;
-  bool ret = cache.get(auth, QType(1), now, &ne2);
+  NegCache::NegCacheEntry ne2;
+  bool ret = cache.get(auth, QType(1), now, ne2);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne2 == nullptr);
 
   cache.wipe(DNSName("1.powerdns.com"));
   BOOST_CHECK_EQUAL(cache.size(), 399U);
 
-  const NegCache::NegCacheEntry* ne3 = nullptr;
-  ret = cache.get(auth, QType(1), now, &ne3);
+  NegCache::NegCacheEntry ne3;
+  ret = cache.get(auth, QType(1), now, ne3);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK(ne3 == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_wipe_subtree)

diff --git a/pdns/recursordist/test-syncres_cc8.cc b/pdns/recursordist/test-syncres_cc8.cc
index 056fb1ef88a12694441111b2ae057c654c10d0ec..a56eb3666955f04fe8f2d5b2598e6b1adda12d2d 100644 (file)
--- a/pdns/recursordist/test-syncres_cc8.cc
+++ b/pdns/recursordist/test-syncres_cc8.cc
@@ -676,15 +676,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity)
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 
   /* check that the entry has not been negatively cached for longer than the RRSIG validity */
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_ttd, fixedNow + 1);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Secure);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_ttd, fixedNow + 1);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1U);
 
   /* again, to test the cache */
   ret.clear();
@@ -747,15 +747,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_bogus_validity)
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 
   /* check that the entry has been negatively cached but not longer than s_maxbogusttl */
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_ttd, fixedNow + SyncRes::s_maxbogusttl);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_ttd, fixedNow + SyncRes::s_maxbogusttl);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 
   /* again, to test the cache */
   ret.clear();

diff --git a/pdns/recursordist/test-syncres_cc9.cc b/pdns/recursordist/test-syncres_cc9.cc
index 59c1ad5ceb777b7af72b1950283ff23a8d8994eb..241e6c5fd04f35bd3223255b66a21b1742f53011 100644 (file)
--- a/pdns/recursordist/test-syncres_cc9.cc
+++ b/pdns/recursordist/test-syncres_cc9.cc
@@ -387,14 +387,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure)
   BOOST_REQUIRE_EQUAL(ret.size(), 4U);
   BOOST_CHECK_EQUAL(queriesCount, 1U);
   /* check that the entry has been negatively cached */
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1U);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -405,12 +405,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure)
   BOOST_REQUIRE_EQUAL(ret.size(), 4U);
   BOOST_CHECK_EQUAL(queriesCount, 4U);
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Secure);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1U);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure_ds)
@@ -525,14 +525,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure)
   BOOST_REQUIRE_EQUAL(ret.size(), 1U);
   BOOST_CHECK_EQUAL(queriesCount, 1U);
   /* check that the entry has not been negatively cached */
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -542,12 +542,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure)
   BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 1U);
   BOOST_CHECK_EQUAL(queriesCount, 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Insecure);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Insecure);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus)
@@ -612,15 +612,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus)
     }
   }
   BOOST_CHECK_EQUAL(queriesCount, 1U);
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxnegttl);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.d_ttd, now + SyncRes::s_maxnegttl);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -633,13 +633,13 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus)
     BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
   }
   BOOST_CHECK_EQUAL(queriesCount, 4U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.d_ttd, now + SyncRes::s_maxbogusttl);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 
   ret.clear();
   /* third one _does_ not require validation, we just check that
@@ -653,13 +653,13 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus)
     BOOST_CHECK_EQUAL(record.d_ttl, SyncRes::s_maxbogusttl);
   }
   BOOST_CHECK_EQUAL(queriesCount, 4U);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
-  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1U);
-  BOOST_CHECK_EQUAL(ne->d_ttd, now + SyncRes::s_maxbogusttl);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0U);
-  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0U);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
+  BOOST_CHECK_EQUAL(ne.d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1U);
+  BOOST_CHECK_EQUAL(ne.d_ttd, now + SyncRes::s_maxbogusttl);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0U);
+  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0U);
 }
 
 BOOST_AUTO_TEST_CASE(test_lowercase_outgoing)

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 3041e9ad50847f411c47a7d8f69fd660f08afc5a..d66cbb882502e267b295509697a238708ee653fd 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1486,32 +1486,32 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
   uint32_t sttl=0;
   //  cout<<"Lookup for '"<<qname<<"|"<<qtype.getName()<<"' -> "<<getLastLabel(qname)<<endl;
   vState cachedState;
-  const NegCache::NegCacheEntry* ne = nullptr;
+  NegCache::NegCacheEntry ne;
 
   if(s_rootNXTrust &&
-      t_sstorage.negcache.getRootNXTrust(qname, d_now, &ne) &&
-      ne->d_auth.isRoot() &&
+      t_sstorage.negcache.getRootNXTrust(qname, d_now, ne) &&
+      ne.d_auth.isRoot() &&
       !(wasForwardedOrAuthZone && !authname.isRoot())) { // when forwarding, the root may only neg-cache if it was forwarded to.
-    sttl = ne->d_ttd - d_now.tv_sec;
-    LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne->d_auth<<"' & '"<<ne->d_name<<"' for another "<<sttl<<" seconds"<<endl);
+    sttl = ne.d_ttd - d_now.tv_sec;
+    LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne.d_auth<<"' & '"<<ne.d_name<<"' for another "<<sttl<<" seconds"<<endl);
     res = RCode::NXDomain;
     giveNegative = true;
-    cachedState = ne->d_validationState;
-  } else if (t_sstorage.negcache.get(qname, qtype, d_now, &ne)) {
+    cachedState = ne.d_validationState;
+  } else if (t_sstorage.negcache.get(qname, qtype, d_now, ne)) {
     /* If we are looking for a DS, discard NXD if auth == qname
        and ask for a specific denial instead */
-    if (qtype != QType::DS || ne->d_qtype.getCode() || ne->d_auth != qname ||
-        t_sstorage.negcache.get(qname, qtype, d_now, &ne, true))
+    if (qtype != QType::DS || ne.d_qtype.getCode() || ne.d_auth != qname ||
+        t_sstorage.negcache.get(qname, qtype, d_now, ne, true))
     {
       res = RCode::NXDomain;
-      sttl = ne->d_ttd - d_now.tv_sec;
+      sttl = ne.d_ttd - d_now.tv_sec;
       giveNegative = true;
-      cachedState = ne->d_validationState;
-      if (ne->d_qtype.getCode()) {
-        LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne->d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+      cachedState = ne.d_validationState;
+      if (ne.d_qtype.getCode()) {
+        LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
         res = RCode::NoError;
       } else {
-        LOG(prefix<<qname<<": Entire name '"<<qname<<"' is negatively cached via '"<<ne->d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+        LOG(prefix<<qname<<": Entire name '"<<qname<<"' is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
       }
     }
   } else if (s_hardenNXD != HardenNXD::No && !qname.isRoot() && !wasForwardedOrAuthZone) {
@@ -1520,19 +1520,19 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
     negCacheName.prependRawLabel(labels.back());
     labels.pop_back();
     while(!labels.empty()) {
-      if (t_sstorage.negcache.get(negCacheName, QType(0), d_now, &ne, true)) {
-        if (ne->d_validationState == Indeterminate && validationEnabled()) {
+      if (t_sstorage.negcache.get(negCacheName, QType(0), d_now, ne, true)) {
+        if (ne.d_validationState == Indeterminate && validationEnabled()) {
           // LOG(prefix << negCacheName <<  " negatively cached and Indeterminate, trying to validate NXDOMAIN" << endl);
           // ...
           // And get the updated ne struct
-          //t_sstorage.negcache.get(negCacheName, QType(0), d_now, &ne, true);
+          //t_sstorage.negcache.get(negCacheName, QType(0), d_now, ne, true);
         }
-        if ((s_hardenNXD == HardenNXD::Yes && ne->d_validationState != Bogus) || ne->d_validationState == Secure) {
+        if ((s_hardenNXD == HardenNXD::Yes && ne.d_validationState != Bogus) || ne.d_validationState == Secure) {
           res = RCode::NXDomain;
-          sttl = ne->d_ttd - d_now.tv_sec;
+          sttl = ne.d_ttd - d_now.tv_sec;
           giveNegative = true;
-          cachedState = ne->d_validationState;
-          LOG(prefix<<qname<<": Name '"<<negCacheName<<"' and below, is negatively cached via '"<<ne->d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+          cachedState = ne.d_validationState;
+          LOG(prefix<<qname<<": Name '"<<negCacheName<<"' and below, is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
           break;
         }
       }
@@ -1545,17 +1545,9 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
 
     state = cachedState;
 
-    /* let's do a full copy now, since:
-       - we know we are going to use the records ;
-       - we might have to go to the network in computeNegCacheValidationStatus(),
-       and our pointer might get invalidated during that time.
-    */
-    NegCache::NegCacheEntry negativeEntry = *ne;
-    ne = nullptr;
-
     if (!wasAuthZone && shouldValidate() && state == Indeterminate) {
       LOG(prefix<<qname<<": got Indeterminate state for records retrieved from the negative cache, validating.."<<endl);
-      computeNegCacheValidationStatus(negativeEntry, qname, qtype, res, state, depth);
+      computeNegCacheValidationStatus(ne, qname, qtype, res, state, depth);
 
       if (state != cachedState && state == Bogus) {
         sttl = std::min(sttl, s_maxbogusttl);
@@ -1563,11 +1555,11 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
     }
 
     // Transplant SOA to the returned packet
-    addTTLModifiedRecords(negativeEntry.authoritySOA.records, sttl, ret);
+    addTTLModifiedRecords(ne.authoritySOA.records, sttl, ret);
     if(d_doDNSSEC) {
-      addTTLModifiedRecords(negativeEntry.authoritySOA.signatures, sttl, ret);
-      addTTLModifiedRecords(negativeEntry.DNSSECRecords.records, sttl, ret);
-      addTTLModifiedRecords(negativeEntry.DNSSECRecords.signatures, sttl, ret);
+      addTTLModifiedRecords(ne.authoritySOA.signatures, sttl, ret);
+      addTTLModifiedRecords(ne.DNSSECRecords.records, sttl, ret);
+      addTTLModifiedRecords(ne.DNSSECRecords.signatures, sttl, ret);
     }
 
     LOG(prefix<<qname<<": updating validation state with negative cache content for "<<qname<<" to "<<vStates[state]<<endl);