]> granicus.if.org Git - pdns/commitdiff
rec: Don't mix time() and gettimeofday() in our unit tests
authorRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 8 Jan 2018 15:44:50 +0000 (16:44 +0100)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Mon, 8 Jan 2018 15:44:50 +0000 (16:44 +0100)
It turns out that, at least on Linux, doing

```
struct timeval now;
gettimeofday(&now, nullptr);
now.tv_sec - time(nullptr);
```

might be 1, without the time actually going backward. So let's just
be consistent in our calls and we should far less false positive
during our tests.

pdns/recursordist/negcache.cc
pdns/recursordist/test-syncres_cc.cc

index 28531923eea4184cf00285ca0c8de1c9e8611d3b..573ee1e0816ebee10f8a72a237d2fa028838670f 100644 (file)
@@ -24,6 +24,7 @@
 #include "negcache.hh"
 #include "misc.hh"
 #include "cachecleaner.hh"
+#include "utility.hh"
 
 /*!
  * Set ne to the NegCacheEntry for the last label in qname and return true if there
@@ -182,16 +183,18 @@ void NegCache::prune(unsigned int maxEntries) {
  */
 uint64_t NegCache::dumpToFile(FILE* fp) {
   uint64_t ret(0);
-  time_t now = time(0);
+  struct timeval now;
+  Utility::gettimeofday(&now, nullptr);
+
   negcache_sequence_t& sidx = d_negcache.get<1>();
   for(const NegCacheEntry& ne : sidx) {
     ret++;
-    fprintf(fp, "%s %d IN %s VIA %s ; (%s)\n", ne.d_name.toString().c_str(), (unsigned int) (ne.d_ttd - now), ne.d_qtype.getName().c_str(), ne.d_auth.toString().c_str(), vStates[ne.d_validationState]);
+    fprintf(fp, "%s %" PRId64 " IN %s VIA %s ; (%s)\n", ne.d_name.toString().c_str(), static_cast<int64_t>(ne.d_ttd - now.tv_sec), ne.d_qtype.getName().c_str(), ne.d_auth.toString().c_str(), vStates[ne.d_validationState]);
     for (const auto& rec : ne.DNSSECRecords.records) {
-      fprintf(fp, "%s %" PRId64 " IN %s %s ; (%s)\n", ne.d_name.toString().c_str(), static_cast<int64_t>(ne.d_ttd - now), DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.d_content->getZoneRepresentation().c_str(), vStates[ne.d_validationState]);
+      fprintf(fp, "%s %" PRId64 " IN %s %s ; (%s)\n", ne.d_name.toString().c_str(), static_cast<int64_t>(ne.d_ttd - now.tv_sec), DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.d_content->getZoneRepresentation().c_str(), vStates[ne.d_validationState]);
     }
     for (const auto& sig : ne.DNSSECRecords.signatures) {
-      fprintf(fp, "%s %" PRId64 " IN RRSIG %s ;\n", ne.d_name.toString().c_str(), static_cast<int64_t>(ne.d_ttd - now), sig.d_content->getZoneRepresentation().c_str());
+      fprintf(fp, "%s %" PRId64 " IN RRSIG %s ;\n", ne.d_name.toString().c_str(), static_cast<int64_t>(ne.d_ttd - now.tv_sec), sig.d_content->getZoneRepresentation().c_str());
     }
   }
   return ret;
index c1bc99927d993075635b346f820517597dec722c..10f90e62fd04562cb6635046f34d7c87d9c4a06d 100644 (file)
@@ -75,18 +75,18 @@ void primeHints(void)
     arr.d_content=std::make_shared<ARecordContent>(ComboAddress(rootIps4[c-'a']));
     vector<DNSRecord> aset;
     aset.push_back(arr);
-    t_RC->replace(time(0), DNSName(templ), QType(QType::A), aset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), true); // auth, nuke it all
+    t_RC->replace(time(nullptr), DNSName(templ), QType(QType::A), aset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), true); // auth, nuke it all
     if (rootIps6[c-'a'] != NULL) {
       aaaarr.d_content=std::make_shared<AAAARecordContent>(ComboAddress(rootIps6[c-'a']));
 
       vector<DNSRecord> aaaaset;
       aaaaset.push_back(aaaarr);
-      t_RC->replace(time(0), DNSName(templ), QType(QType::AAAA), aaaaset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), true);
+      t_RC->replace(time(nullptr), DNSName(templ), QType(QType::AAAA), aaaaset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), true);
     }
 
     nsset.push_back(nsrr);
   }
-  t_RC->replace(time(0), g_rootdnsname, QType(QType::NS), nsset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), false); // and stuff in the cache
+  t_RC->replace(time(nullptr), g_rootdnsname, QType(QType::NS), nsset, vector<std::shared_ptr<RRSIGRecordContent>>(), vector<std::shared_ptr<DNSRecord>>(), false); // and stuff in the cache
 }
 
 LuaConfigItems::LuaConfigItems()
@@ -746,9 +746,10 @@ BOOST_AUTO_TEST_CASE(test_all_nss_down) {
   BOOST_CHECK_EQUAL(ret.size(), 0);
   BOOST_CHECK_EQUAL(downServers.size(), 4);
 
+  time_t now = sr->getNow().tv_sec;
   for (const auto& server : downServers) {
     BOOST_CHECK_EQUAL(SyncRes::getServerFailsCount(server), 1);
-    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK(SyncRes::isThrottled(now, server, target, QType::A));
   }
 }
 
@@ -793,9 +794,10 @@ BOOST_AUTO_TEST_CASE(test_all_nss_network_error) {
   BOOST_CHECK_EQUAL(ret.size(), 0);
   BOOST_CHECK_EQUAL(downServers.size(), 4);
 
+  time_t now = sr->getNow().tv_sec;
   for (const auto& server : downServers) {
     BOOST_CHECK_EQUAL(SyncRes::getServerFailsCount(server), 1);
-    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK(SyncRes::isThrottled(now, server, target, QType::A));
   }
 }
 
@@ -903,9 +905,10 @@ BOOST_AUTO_TEST_CASE(test_os_limit_errors) {
   BOOST_CHECK_EQUAL(downServers.size(), 3);
 
   /* Error is reported as "OS limit error" (-2) so the servers should _NOT_ be marked down */
+  time_t now = sr->getNow().tv_sec;
   for (const auto& server : downServers) {
     BOOST_CHECK_EQUAL(SyncRes::getServerFailsCount(server), 0);
-    BOOST_CHECK(!SyncRes::isThrottled(time(nullptr), server, target, QType::A));
+    BOOST_CHECK(!SyncRes::isThrottled(now, server, target, QType::A));
   }
 }
 
@@ -1498,7 +1501,8 @@ BOOST_AUTO_TEST_CASE(test_throttled_server) {
     });
 
   /* mark ns as down */
-  SyncRes::doThrottle(time(nullptr), ns, SyncRes::s_serverdownthrottletime, 10000);
+  time_t now = sr->getNow().tv_sec;
+  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, 10000);
 
   vector<DNSRecord> ret;
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
@@ -1518,14 +1522,15 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_count) {
 
   const size_t blocks = 10;
   /* mark ns as down for 'blocks' queries */
-  SyncRes::doThrottle(time(nullptr), ns, SyncRes::s_serverdownthrottletime, blocks);
+  time_t now = sr->getNow().tv_sec;
+  SyncRes::doThrottle(now, ns, SyncRes::s_serverdownthrottletime, blocks);
 
   for (size_t idx = 0; idx < blocks; idx++) {
-    BOOST_CHECK(SyncRes::isThrottled(time(nullptr), ns));
+    BOOST_CHECK(SyncRes::isThrottled(now, ns));
   }
 
   /* we have been throttled 'blocks' times, we should not be throttled anymore */
-  BOOST_CHECK(!SyncRes::isThrottled(time(nullptr), ns));
+  BOOST_CHECK(!SyncRes::isThrottled(now, ns));
 }
 
 BOOST_AUTO_TEST_CASE(test_throttled_server_time) {
@@ -1538,14 +1543,13 @@ BOOST_AUTO_TEST_CASE(test_throttled_server_time) {
 
   const size_t seconds = 1;
   /* mark ns as down for 'seconds' seconds */
-  SyncRes::doThrottle(time(nullptr), ns, seconds, 10000);
-
-  BOOST_CHECK(SyncRes::isThrottled(time(nullptr), ns));
+  time_t now = sr->getNow().tv_sec;
+  SyncRes::doThrottle(now, ns, seconds, 10000);
 
-  sleep(seconds + 1);
+  BOOST_CHECK(SyncRes::isThrottled(now, ns));
 
   /* we should not be throttled anymore */
-  BOOST_CHECK(!SyncRes::isThrottled(time(nullptr), ns));
+  BOOST_CHECK(!SyncRes::isThrottled(now + 2, ns));
 }
 
 BOOST_AUTO_TEST_CASE(test_dont_query_server) {
@@ -1875,8 +1879,7 @@ BOOST_AUTO_TEST_CASE(test_ns_speed) {
       return 0;
     });
 
-  struct timeval now;
-  gettimeofday(&now, 0);
+  struct timeval now = sr->getNow();
 
   /* make pdns-public-ns2.powerdns.com. the fastest NS, with its IPv6 address faster than the IPV4 one,
      then pdns-public-ns1.powerdns.com. on IPv4 */