From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 8 Mar 2019 08:44:00 +0000 (+0100)
Subject: Eliminate the loop in SyncRess:getAddrs().
X-Git-Tag: dnsdist-1.4.0-beta1~25^2~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=493f56829a1ccdd266413225ca16dcbb3bd0b9bc;p=thirdparty%2Fpdns.git

Eliminate the loop in SyncRess:getAddrs().

Makes the code much more readable.
---

diff --git a/pdns/qtype.hh b/pdns/qtype.hh
index e8147def79..8d7eab185f 100644
--- a/pdns/qtype.hh
+++ b/pdns/qtype.hh
@@ -131,6 +131,10 @@ public:
     LUA=65402
   };
 
+  QType(typeenum orig) : code(orig)
+  {
+  }
+
   typedef pair<string,uint16_t> namenum;
   static vector<namenum> names;
 
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 3cf44f16a3..29306d86e5 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -680,12 +680,9 @@ struct speedOrderCA
 vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere, bool cacheOnly)
 {
   typedef vector<DNSRecord> res_t;
-  res_t res;
-
   typedef vector<ComboAddress> ret_t;
   ret_t ret;
 
-  QType type;
   bool oldCacheOnly = d_cacheonly;
   bool oldRequireAuthData = d_requireAuthData;
   bool oldValidationRequested = d_DNSSECValidationRequested;
@@ -693,48 +690,45 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,
   d_DNSSECValidationRequested = false;
   d_cacheonly = cacheOnly;
 
-  for(int j=1; j<2+s_doIPv6; j++)
-  {
-    bool done=false;
-    switch(j) {
-      case 0:
-        type = QType::ANY;
-        break;
-      case 1:
-        type = QType::A;
-        break;
-      case 2:
-        type = QType::AAAA;
-        break;
-    }
-
+  if (true) { // IPv4 always matters
     vState newState = Indeterminate;
-    if(!doResolve(qname, type, res,depth+1, beenthere, newState) && !res.empty()) {  // this consults cache, OR goes out
-      for(res_t::const_iterator i=res.begin(); i!= res.end(); ++i) {
-        if(i->d_type == QType::A || i->d_type == QType::AAAA) {
-	  if(auto rec = getRR<ARecordContent>(*i))
-	    ret.push_back(rec->getCA(53));
-	  else if(auto aaaarec = getRR<AAAARecordContent>(*i))
-	    ret.push_back(aaaarec->getCA(53));
-          done=true;
+    res_t res;
+    if (doResolve(qname, QType::A, res, depth+1, beenthere, newState) == 0) {  // this consults cache, OR goes out
+      for (res_t::const_iterator i = res.begin(); i != res.end(); ++i) {
+        if (i->d_type == QType::A) {
+          if (auto rec = getRR<ARecordContent>(*i)) {
+            ret.push_back(rec->getCA(53));
+          }
         }
       }
     }
-    if(done) {
-      if(j==1 && s_doIPv6) { // we got an A record, see if we have some AAAA lying around
-	vector<DNSRecord> cset;
-	if(t_RC->get(d_now.tv_sec, qname, QType(QType::AAAA), false, &cset, d_cacheRemote) > 0) {
-	  for(auto k=cset.cbegin();k!=cset.cend();++k) {
-	    if(k->d_ttl > (unsigned int)d_now.tv_sec ) {
-	      if (auto drc = getRR<AAAARecordContent>(*k)) {
-	        ComboAddress ca=drc->getCA(53);
-	        ret.push_back(ca);
-	      }
-	    }
-	  }
-	}
+  }
+  if (s_doIPv6) {
+    if (ret.empty()) {
+      // We did not find IPv4 addresses, try to get IPv6 ones
+      vState newState = Indeterminate;
+      res_t res;
+      if (doResolve(qname, QType::AAAA, res,depth+1, beenthere, newState) == 0) {  // this consults cache, OR goes out
+        for (res_t::const_iterator i = res.begin(); i != res.end(); ++i) {
+          if (i->d_type == QType::AAAA) {
+            if (auto rec = getRR<AAAARecordContent>(*i))
+              ret.push_back(rec->getCA(53));
+          }
+        }
+      }
+    } else {
+      // We have some IPv4 records, don't bother with going out to get IPv6, but do consult the cache
+      // Once IPv6 adoption matters, this needs to be revisited
+      res_t cset;
+      if (t_RC->get(d_now.tv_sec, qname, QType(QType::AAAA), false, &cset, d_cacheRemote) > 0) {
+        for (auto k = cset.cbegin(); k != cset.cend(); ++k) {
+          if (k->d_ttl > (unsigned int)d_now.tv_sec ) {
+            if (auto rec = getRR<AAAARecordContent>(*k)) {
+              ret.push_back(rec->getCA(53));
+            }
+          }
+        }
       }
-      break;
     }
   }