From 6b68a4e39633422a3dbae5d3dac7358aa79ff76c Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 21 Feb 2017 16:30:35 +0100 Subject: [PATCH] rec: Clean up time_t / uint32_t mix for the packet cache's TTD --- pdns/cachecleaner.hh | 2 +- pdns/recpacketcache.cc | 11 ++++++----- pdns/recpacketcache.hh | 10 +++++----- pdns/recursor_cache.cc | 32 ++++++++++++++++---------------- pdns/recursor_cache.hh | 8 ++++---- 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh index fdeeb2c19..0adbe370d 100644 --- a/pdns/cachecleaner.hh +++ b/pdns/cachecleaner.hh @@ -27,7 +27,7 @@ // on a miss, move it to the beginning template void pruneCollection(T& collection, unsigned int maxCached, unsigned int scanFraction=1000) { - uint32_t now=(uint32_t)time(0); + time_t now=time(0); unsigned int toTrim=0; unsigned int cacheSize=collection.size(); diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc index dd71567a4..3396e0dd0 100644 --- a/pdns/recpacketcache.cc +++ b/pdns/recpacketcache.cc @@ -2,6 +2,7 @@ #include "config.h" #endif #include +#include #include "recpacketcache.hh" #include "cachecleaner.hh" @@ -59,8 +60,8 @@ uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket) const char* p = origPacket.c_str() + 12; for(; p < end && *p; ++p) { // XXX if you embed a 0 in your qname we'll stop lowercasing there - const char l = dns_tolower(*p); // label lengths can safely be lower cased - ret=burtle((const unsigned char*)&l, 1, ret); + const unsigned char l = dns_tolower(*p); // label lengths can safely be lower cased + ret=burtle(&l, 1, ret); } // XXX the embedded 0 in the qname will break the subnet stripping struct dnsheader* dh = (struct dnsheader*)origPacket.c_str(); @@ -115,8 +116,8 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& // the possibility is VERY real that we get hits that are not right - birthday paradox if(!qrMatch(queryPacket, iter->d_name, iter->d_type, iter->d_class)) continue; - if((uint32_t)now < iter->d_ttd) { // it is right, it is fresh! - *age = now - iter->d_creation; + if(now < iter->d_ttd) { // it is right, it is fresh! + *age = static_cast(now - iter->d_creation); *responsePacket = iter->d_packet; responsePacket->replace(0, 2, queryPacket.c_str(), 2); @@ -234,7 +235,7 @@ uint64_t RecursorPacketCache::doDump(int fd) for(auto i=sidx.cbegin(); i != sidx.cend(); ++i) { count++; try { - fprintf(fp, "%s %d %s ; tag %d\n", i->d_name.toString().c_str(), (int32_t)(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_type).c_str(), i->d_tag); + fprintf(fp, "%s %" PRId64 " %s ; tag %d\n", i->d_name.toString().c_str(), static_cast(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_type).c_str(), i->d_tag); } catch(...) { fprintf(fp, "; error printing '%s'\n", i->d_name.empty() ? "EMPTY" : i->d_name.toString().c_str()); diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh index 8c198feb6..a68dd2fe8 100644 --- a/pdns/recpacketcache.hh +++ b/pdns/recpacketcache.hh @@ -52,8 +52,8 @@ public: RecursorPacketCache(); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage); - void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttd); - void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttd, const RecProtoBufMessage* protobufMessage); + void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl); + void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl, const RecProtoBufMessage* protobufMessage); void doPruneTo(unsigned int maxSize=250000); uint64_t doDump(int fd); int doWipePacketCache(const DNSName& name, uint16_t qtype=0xffff, bool subtree=false); @@ -68,8 +68,8 @@ private: struct NameTag {}; struct Entry { - mutable uint32_t d_ttd; - mutable uint32_t d_creation; // so we can 'age' our packets + mutable time_t d_ttd; + mutable time_t d_creation; // so we can 'age' our packets DNSName d_name; uint16_t d_type; uint16_t d_class; @@ -81,7 +81,7 @@ private: uint32_t d_tag; inline bool operator<(const struct Entry& rhs) const; - uint32_t getTTD() const + time_t getTTD() const { return d_ttd; } diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 08b6f61ed..9dcfe0c68 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -1,6 +1,9 @@ #ifdef HAVE_CONFIG_H #include "config.h" #endif + +#include + #include "recursor_cache.hh" #include "misc.hh" #include @@ -31,9 +34,9 @@ unsigned int MemRecursorCache::bytes() } // returns -1 for no hits -int MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures) +int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures) { - unsigned int ttd=0; + time_t ttd=0; // cerr<<"looking up "<< qname<<"|"+qt.getName()<<"\n"; if(!d_cachecachevalid || d_cachedqname!= qname) { @@ -70,7 +73,7 @@ int MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vec dr.d_type = i->d_qtype; dr.d_class = 1; dr.d_content = *k; - dr.d_ttl = i->d_ttd; + dr.d_ttl = static_cast(i->d_ttd); dr.d_place = DNSResourceRecord::ANSWER; res->push_back(dr); } @@ -89,7 +92,7 @@ int MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vec } // cerr<<"time left : "<size() : 0) <<"\n"; - return (int)ttd-now; + return static_cast(ttd-now); } return -1; } @@ -136,7 +139,7 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt isNew = true; } - uint32_t maxTTD=UINT_MAX; + time_t maxTTD=std::numeric_limits::max(); CacheEntry ce=*stored; // this is a COPY ce.d_qtype=qt.getCode(); ce.d_signatures=signatures; @@ -173,7 +176,9 @@ void MemRecursorCache::replace(time_t now, const DNSName &qname, const QType& qt for(auto i=content.cbegin(); i != content.cend(); ++i) { - ce.d_ttd=min(maxTTD, i->d_ttl); // XXX this does weird things if TTLs differ in the set + /* Yes, we have altered the d_ttl value by adding time(nullptr) to it + prior to calling this function, so the TTL actually holds a TTD. */ + ce.d_ttd=min(maxTTD, static_cast(i->d_ttl)); // XXX this does weird things if TTLs differ in the set // cerr<<"To store: "<d_content->getZoneRepresentation()<<" with ttl/ttd "<d_ttl<<", capped at: "<d_content); // there was code here that did things with TTL and auth. Unsure if it was good. XXX @@ -216,27 +221,22 @@ int MemRecursorCache::doWipeCache(const DNSName& name, bool sub, uint16_t qtype) return count; } -bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtype, int32_t newTTL) +bool MemRecursorCache::doAgeCache(time_t now, const DNSName& name, uint16_t qtype, uint32_t newTTL) { cache_t::iterator iter = d_cache.find(tie(name, qtype)); - uint32_t maxTTD=std::numeric_limits::min(); if(iter == d_cache.end()) { return false; } CacheEntry ce = *iter; - - - maxTTD=ce.d_ttd; - int32_t maxTTL = maxTTD - now; - - if(maxTTL < 0) + if(ce.d_ttd < now) return false; // would be dead anyhow + uint32_t maxTTL = static_cast(ce.d_ttd - now); if(maxTTL > newTTL) { d_cachecachevalid=false; - uint32_t newTTD = now + newTTL; + time_t newTTD = now + newTTL; if(ce.d_ttd > newTTD) // do never renew expired or older TTLs @@ -287,7 +287,7 @@ uint64_t MemRecursorCache::doDump(int fd) for(auto j=i->d_records.cbegin(); j != i->d_records.cend(); ++j) { count++; try { - fprintf(fp, "%s %d IN %s %s ; %s\n", i->d_qname.toString().c_str(), (int32_t)(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_qtype).c_str(), (*j)->getZoneRepresentation().c_str(), i->d_netmask.empty() ? "" : i->d_netmask.toString().c_str()); + fprintf(fp, "%s %" PRId64 " IN %s %s ; %s\n", i->d_qname.toString().c_str(), static_cast(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_qtype).c_str(), (*j)->getZoneRepresentation().c_str(), i->d_netmask.empty() ? "" : i->d_netmask.toString().c_str()); } catch(...) { fprintf(fp, "; error printing '%s'\n", i->d_qname.empty() ? "EMPTY" : i->d_qname.toString().c_str()); diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 333257e9f..48c1803f2 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -53,7 +53,7 @@ public: } unsigned int size(); unsigned int bytes(); - int get(time_t, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures=0); + int32_t get(time_t, const DNSName &qname, const QType& qt, vector* res, const ComboAddress& who, vector>* signatures=0); void replace(time_t, const DNSName &qname, const QType& qt, const vector& content, const vector>& signatures, bool auth, boost::optional ednsmask=boost::optional()); void doPrune(void); @@ -62,7 +62,7 @@ public: uint64_t doDumpNSSpeeds(int fd); int doWipeCache(const DNSName& name, bool sub, uint16_t qtype=0xffff); - bool doAgeCache(time_t now, const DNSName& name, uint16_t qtype, int32_t newTTL); + bool doAgeCache(time_t now, const DNSName& name, uint16_t qtype, uint32_t newTTL); uint64_t cacheHits, cacheMisses; private: @@ -75,7 +75,7 @@ private: typedef vector> records_t; vector> d_signatures; - uint32_t getTTD() const + time_t getTTD() const { return d_ttd; } @@ -83,7 +83,7 @@ private: DNSName d_qname; uint16_t d_qtype; bool d_auth; - uint32_t d_ttd; + time_t d_ttd; records_t d_records; Netmask d_netmask; }; -- 2.40.0