rec: Clean up time_t / uint32_t mix for the packet cache's TTD

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index fdeeb2c190015595bee14c0e573a17f8e3a0cfbb..0adbe370df6da4ceeeb3a9ad81caa3032d1e54ca 100644 (file)
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -27,7 +27,7 @@
 // on a miss, move it to the beginning
 template <typename T> 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 dd71567a45180e2c9967bea7de487420bfbeb014..3396e0dd05378f294938b45f73c8d309c0fc0d5f 100644 (file)
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -2,6 +2,7 @@
 #include "config.h"
 #endif
 #include <iostream>
+#include <cinttypes>
 
 #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<uint32_t>(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<int64_t>(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 8c198feb6dc8794fbc2844f5fc72c0d1b815b73a..a68dd2fe8cb4c536f899133456dd619c0ad7e4be 100644 (file)
--- 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 08b6f61ed4d3d210779efa9626e91a2bdf08a504..9dcfe0c68f7218666542d35d692cffac48c3ba44 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -1,6 +1,9 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
+
+#include <cinttypes>
+
 #include "recursor_cache.hh"
 #include "misc.hh"
 #include <iostream>
@@ -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<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures)
+int32_t MemRecursorCache::get(time_t now, const DNSName &qname, const QType& qt, vector<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* 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<uint32_t>(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 : "<<ttd - now<<", "<< (res ? res->size() : 0) <<"\n";
-    return (int)ttd-now;
+    return static_cast<int32_t>(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<time_t>::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<time_t>(i->d_ttl));   // XXX this does weird things if TTLs differ in the set
     //    cerr<<"To store: "<<i->d_content->getZoneRepresentation()<<" with ttl/ttd "<<i->d_ttl<<", capped at: "<<maxTTD<<endl;
     ce.d_records.push_back(i->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<uint32_t>::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<uint32_t>(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<int64_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());
       }
       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 333257e9f30c8546ff08584636dc9b6e2f8e1500..48c1803f29648c7c9b9f7ce58d554fa8f0f50111 100644 (file)
--- 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<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures=0);
+  int32_t get(time_t, const DNSName &qname, const QType& qt, vector<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures=0);
 
   void replace(time_t, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth, boost::optional<Netmask> ednsmask=boost::optional<Netmask>());
   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<std::shared_ptr<DNSRecordContent>> records_t;
     vector<std::shared_ptr<RRSIGRecordContent>> 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;
   };