From: Remi Gacogne Date: Mon, 8 Jan 2018 15:44:50 +0000 (+0100) Subject: rec: Don't mix time() and gettimeofday() in our unit tests X-Git-Tag: dnsdist-1.3.0~156^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=606accb0e70e07b74f8213931a8dfc7b49173dd1;p=pdns rec: Don't mix time() and gettimeofday() in our unit tests 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. --- diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc index 28531923e..573ee1e08 100644 --- a/pdns/recursordist/negcache.cc +++ b/pdns/recursordist/negcache.cc @@ -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(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(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(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(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(ne.d_ttd - now.tv_sec), sig.d_content->getZoneRepresentation().c_str()); } } return ret; diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index c1bc99927..10f90e62f 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -75,18 +75,18 @@ void primeHints(void) arr.d_content=std::make_shared(ComboAddress(rootIps4[c-'a'])); vector aset; aset.push_back(arr); - t_RC->replace(time(0), DNSName(templ), QType(QType::A), aset, vector>(), vector>(), true); // auth, nuke it all + t_RC->replace(time(nullptr), DNSName(templ), QType(QType::A), aset, vector>(), vector>(), true); // auth, nuke it all if (rootIps6[c-'a'] != NULL) { aaaarr.d_content=std::make_shared(ComboAddress(rootIps6[c-'a'])); vector aaaaset; aaaaset.push_back(aaaarr); - t_RC->replace(time(0), DNSName(templ), QType(QType::AAAA), aaaaset, vector>(), vector>(), true); + t_RC->replace(time(nullptr), DNSName(templ), QType(QType::AAAA), aaaaset, vector>(), vector>(), true); } nsset.push_back(nsrr); } - t_RC->replace(time(0), g_rootdnsname, QType(QType::NS), nsset, vector>(), vector>(), false); // and stuff in the cache + t_RC->replace(time(nullptr), g_rootdnsname, QType(QType::NS), nsset, vector>(), vector>(), 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 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 */