From cdcb5fbd2b16e14cf4ec628ecf9069b1c7e5f116 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 22 Mar 2018 15:22:38 +0100
Subject: [PATCH] dnsdist: Use C++11-style loops for walking the ring buffers

Suggested by @chbruyand (thanks!).
---
 pdns/dnsdist-lua-inspection.cc | 74 +++++++++++++++++-----------------
 pdns/dnsdist-rings.cc          | 16 ++++----
 pdns/dnsdist-rings.hh          | 12 +++---
 3 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/pdns/dnsdist-lua-inspection.cc b/pdns/dnsdist-lua-inspection.cc
index cc115a40f..c7afa0fc0 100644
--- a/pdns/dnsdist-lua-inspection.cc
+++ b/pdns/dnsdist-lua-inspection.cc
@@ -32,10 +32,10 @@ static std::unordered_map<unsigned int, vector<boost::variant<string,double>>> g
   map<DNSName, unsigned int> counts;
   unsigned int total=0;
   {
-    for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-      std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
+    for (auto& shard : g_rings.d_shards) {
+      std::lock_guard<std::mutex> rl(shard->respLock);
       if(!labels) {
-        for(const auto& a : g_rings.d_shards[idx]->respRing) {
+        for(const auto& a : shard->respRing) {
           if(!pred(a))
             continue;
           counts[a.name]++;
@@ -44,7 +44,7 @@ static std::unordered_map<unsigned int, vector<boost::variant<string,double>>> g
       }
       else {
         unsigned int lab = *labels;
-        for(auto a : g_rings.d_shards[idx]->respRing) {
+        for(auto a : shard->respRing) {
           if(!pred(a))
             continue;
 
@@ -106,10 +106,10 @@ static void statNodeRespRing(statvisitor_t visitor, unsigned int seconds)
   cutoff.tv_sec -= seconds;
 
   StatNode root;
-  for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-    std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
+  for (auto& shard : g_rings.d_shards) {
+    std::lock_guard<std::mutex> rl(shard->respLock);
 
-    for(const auto& c : g_rings.d_shards[idx]->respRing) {
+    for(const auto& c : shard->respRing) {
       if (now < c.when)
         continue;
 
@@ -130,12 +130,12 @@ static vector<pair<unsigned int, std::unordered_map<string,string> > > getRespRi
   typedef std::unordered_map<string,string>  entry_t;
   vector<pair<unsigned int, entry_t > > ret;
 
-  for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-    std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
+  for (auto& shard : g_rings.d_shards) {
+    std::lock_guard<std::mutex> rl(shard->respLock);
 
     entry_t e;
     unsigned int count=1;
-    for(const auto& c : g_rings.d_shards[idx]->respRing) {
+    for(const auto& c : shard->respRing) {
       if(rcode && (rcode.get() != c.dh.rcode))
         continue;
       e["qname"]=c.name.toString();
@@ -158,9 +158,9 @@ static counts_t exceedRespGen(unsigned int rate, int seconds, std::function<void
 
   counts.reserve(g_rings.getNumberOfResponseEntries());
 
-  for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-    std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
-    for(const auto& c : g_rings.d_shards[idx]->respRing) {
+  for (auto& shard : g_rings.d_shards) {
+    std::lock_guard<std::mutex> rl(shard->respLock);
+    for(const auto& c : shard->respRing) {
 
       if(seconds && c.when < cutoff)
         continue;
@@ -187,9 +187,9 @@ static counts_t exceedQueryGen(unsigned int rate, int seconds, std::function<voi
 
   counts.reserve(g_rings.getNumberOfQueryEntries());
 
-  for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-    std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->queryLock);
-    for(const auto& c : g_rings.d_shards[idx]->queryRing) {
+  for (auto& shard : g_rings.d_shards) {
+    std::lock_guard<std::mutex> rl(shard->queryLock);
+    for(const auto& c : shard->queryRing) {
       if(seconds && c.when < cutoff)
         continue;
       if(now < c.when)
@@ -230,9 +230,9 @@ void setupLuaInspection()
       map<ComboAddress, unsigned int,ComboAddress::addressOnlyLessThan > counts;
       unsigned int total=0;
       {
-        for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->queryLock);
-          for(const auto& c : g_rings.d_shards[idx]->queryRing) {
+        for (auto& shard : g_rings.d_shards) {
+          std::lock_guard<std::mutex> rl(shard->queryLock);
+          for(const auto& c : shard->queryRing) {
             counts[c.requestor]++;
             total++;
           }
@@ -263,9 +263,9 @@ void setupLuaInspection()
       map<DNSName, unsigned int> counts;
       unsigned int total=0;
       if(!labels) {
-        for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->queryLock);
-          for(const auto& a : g_rings.d_shards[idx]->queryRing) {
+        for (auto& shard : g_rings.d_shards) {
+          std::lock_guard<std::mutex> rl(shard->queryLock);
+          for(const auto& a : shard->queryRing) {
             counts[a.name]++;
             total++;
           }
@@ -273,9 +273,9 @@ void setupLuaInspection()
       }
       else {
 	unsigned int lab = *labels;
-        for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->queryLock);
-          for(auto a : g_rings.d_shards[idx]->queryRing) {
+        for (auto& shard : g_rings.d_shards) {
+          std::lock_guard<std::mutex> rl(shard->queryLock);
+          for(auto a : shard->queryRing) {
             a.name.trimToLabels(lab);
             counts[a.name]++;
             total++;
@@ -313,12 +313,12 @@ void setupLuaInspection()
       size_t totalEntries = 0;
       std::vector<boost::circular_buffer<Rings::Response>> rings;
       rings.reserve(g_rings.getNumberOfShards());
-      for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
+      for (auto& shard : g_rings.d_shards) {
         {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
-          rings[idx] = g_rings.d_shards[idx]->respRing;
+          std::lock_guard<std::mutex> rl(shard->respLock);
+          rings.push_back(shard->respRing);
         }
-        totalEntries += rings[idx].size();
+        totalEntries += rings.back().size();
       }
       vector<std::unordered_map<string, boost::variant<string, unsigned int> > > ret;
       ret.reserve(totalEntries);
@@ -409,16 +409,16 @@ void setupLuaInspection()
       std::vector<Rings::Response> rr;
       qr.reserve(g_rings.getNumberOfQueryEntries());
       rr.reserve(g_rings.getNumberOfResponseEntries());
-      for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
+      for (auto& shard : g_rings.d_shards) {
         {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->queryLock);
-          for (const auto& entry : g_rings.d_shards[idx]->queryRing) {
+          std::lock_guard<std::mutex> rl(shard->queryLock);
+          for (const auto& entry : shard->queryRing) {
             qr.push_back(entry);
           }
         }
         {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
-          for (const auto& entry : g_rings.d_shards[idx]->respRing) {
+          std::lock_guard<std::mutex> rl(shard->respLock);
+          for (const auto& entry : shard->respRing) {
             rr.push_back(entry);
           }
         }
@@ -503,9 +503,9 @@ void setupLuaInspection()
       double totlat=0;
       unsigned int size=0;
       {
-        for (size_t idx = 0; idx < g_rings.getNumberOfShards(); idx++) {
-          std::lock_guard<std::mutex> rl(g_rings.d_shards[idx]->respLock);
-          for(const auto& r : g_rings.d_shards[idx]->respRing) {
+        for (auto& shard : g_rings.d_shards) {
+          std::lock_guard<std::mutex> rl(shard->respLock);
+          for(const auto& r : shard->respRing) {
             /* skip actively discovered timeouts */
             if (r.usec == std::numeric_limits<unsigned int>::max())
               continue;
diff --git a/pdns/dnsdist-rings.cc b/pdns/dnsdist-rings.cc
index 8f8ab0ddb..c1155653c 100644
--- a/pdns/dnsdist-rings.cc
+++ b/pdns/dnsdist-rings.cc
@@ -25,9 +25,9 @@
 size_t Rings::numDistinctRequestors()
 {
   std::set<ComboAddress, ComboAddress::addressOnlyLessThan> s;
-  for (size_t idx = 0; idx < getNumberOfShards(); idx++) {
-    std::lock_guard<std::mutex> rl(d_shards[idx]->queryLock);
-    for(const auto& q : d_shards[idx]->queryRing) {
+  for (auto& shard : d_shards) {
+    std::lock_guard<std::mutex> rl(shard->queryLock);
+    for(const auto& q : shard->queryRing) {
       s.insert(q.requestor);
     }
   }
@@ -38,17 +38,17 @@ std::unordered_map<int, vector<boost::variant<string,double>>> Rings::getTopBand
 {
   map<ComboAddress, unsigned int, ComboAddress::addressOnlyLessThan> counts;
   uint64_t total=0;
-  for (size_t idx = 0; idx < getNumberOfShards(); idx++) {
+  for (auto& shard : d_shards) {
     {
-      std::lock_guard<std::mutex> rl(d_shards[idx]->queryLock);
-      for(const auto& q : d_shards[idx]->queryRing) {
+      std::lock_guard<std::mutex> rl(shard->queryLock);
+      for(const auto& q : shard->queryRing) {
         counts[q.requestor]+=q.size;
         total+=q.size;
       }
     }
     {
-      std::lock_guard<std::mutex> rl(d_shards[idx]->respLock);
-      for(const auto& r : d_shards[idx]->respRing) {
+      std::lock_guard<std::mutex> rl(shard->respLock);
+      for(const auto& r : shard->respRing) {
         counts[r.requestor]+=r.size;
         total+=r.size;
       }
diff --git a/pdns/dnsdist-rings.hh b/pdns/dnsdist-rings.hh
index b9f37654a..f9810efad 100644
--- a/pdns/dnsdist-rings.hh
+++ b/pdns/dnsdist-rings.hh
@@ -81,15 +81,15 @@ struct Rings {
     d_numberOfShards = numberOfShards;
 
     /* resize all the rings */
-    for (size_t idx = 0; idx < numberOfShards; idx++) {
-      d_shards[idx] = std::unique_ptr<Shard>(new Shard());
+    for (auto& shard : d_shards) {
+      shard = std::unique_ptr<Shard>(new Shard());
       {
-        std::lock_guard<std::mutex> wl(d_shards[idx]->queryLock);
-        d_shards[idx]->queryRing.set_capacity(newCapacity / numberOfShards);
+        std::lock_guard<std::mutex> wl(shard->queryLock);
+        shard->queryRing.set_capacity(newCapacity / numberOfShards);
       }
       {
-        std::lock_guard<std::mutex> wl(d_shards[idx]->respLock);
-        d_shards[idx]->respRing.set_capacity(newCapacity / numberOfShards);
+        std::lock_guard<std::mutex> wl(shard->respLock);
+        shard->respRing.set_capacity(newCapacity / numberOfShards);
       }
     }
   }
-- 
2.40.0