Process review commment from @fredmorcos, thanks!

diff --git a/pdns/fstrm_logger.hh b/pdns/fstrm_logger.hh
index 7147a471c0975e0f789ecbf1ad5e38cf56d54d05..3eafb3d997f8a9a6617cc694cb3535d310308339 100644 (file)
--- a/pdns/fstrm_logger.hh
+++ b/pdns/fstrm_logger.hh
@@ -40,21 +40,22 @@ public:
   ~FrameStreamLogger();
   [[nodiscard]] RemoteLoggerInterface::Result queueData(const std::string& data) override;
 
-  std::string address() const override
+  [[nodiscard]] std::string address() const override
   {
     return d_address;
   }
 
-  const std::string name() const override
+  [[nodiscard]] std::string name() const override
   {
     return "dnstap";
   }
-  std::string toString() override
+  
+  [[nodiscard]] std::string toString() override
   {
     return "FrameStreamLogger to " + d_address + " (" + std::to_string(d_framesSent) + " frames sent, " + std::to_string(d_queueFullDrops) + " dropped, " + std::to_string(d_permanentFailures) + " permanent failures)";
   }
 
-  RemoteLoggerInterface::Stats getStats() override
+  [[nodiscard]] RemoteLoggerInterface::Stats getStats() override
   {
     return Stats{.d_queued = d_framesSent,
                  .d_pipeFull = d_queueFullDrops,

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 149f68606ba7a318f726af94a22cb88b3b9412c8..cff6e03508589bd73441a9ef57096407c5cd8e23 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -944,8 +944,8 @@ static RemoteLoggerStats_t* pleaseGetRemoteLoggerStats()
   auto ret = make_unique<RemoteLoggerStats_t>();
 
   if (t_protobufServers) {
-    for (const auto& s : *t_protobufServers) {
-      ret->emplace(std::make_pair(s->address(), s->getStats()));
+    for (const auto& server : *t_protobufServers) {
+      ret->emplace(std::make_pair(server->address(), server->getStats()));
     }
   }
   return ret.release();
@@ -967,8 +967,8 @@ static RemoteLoggerStats_t* pleaseGetOutgoingRemoteLoggerStats()
   auto ret = make_unique<RemoteLoggerStats_t>();
 
   if (t_outgoingProtobufServers) {
-    for (const auto& s : *t_outgoingProtobufServers) {
-      ret->emplace(std::make_pair(s->address(), s->getStats()));
+    for (const auto& server : *t_outgoingProtobufServers) {
+      ret->emplace(std::make_pair(server->address(), server->getStats()));
     }
   }
   return ret.release();
@@ -980,38 +980,38 @@ static RemoteLoggerStats_t* pleaseGetFramestreamLoggerStats()
   auto ret = make_unique<RemoteLoggerStats_t>();
 
   if (t_frameStreamServersInfo.servers) {
-    for (const auto& s : *t_frameStreamServersInfo.servers) {
-      ret->emplace(std::make_pair(s->address(), s->getStats()));
+    for (const auto& server : *t_frameStreamServersInfo.servers) {
+      ret->emplace(std::make_pair(server->address(), server->getStats()));
     }
   }
   return ret.release();
 }
 #endif
 
-static void remoteLoggerStats(const string& type, const RemoteLoggerStats_t& stats, ostringstream& os)
+static void remoteLoggerStats(const string& type, const RemoteLoggerStats_t& stats, ostringstream& outpustStream)
 {
   if (stats.empty()) {
     return;
   }
   for (const auto& [key, entry] : stats) {
-    os << entry.d_queued << '\t' << entry.d_pipeFull << '\t' << entry.d_tooLarge << '\t' << entry.d_otherError << '\t' << key << '\t' << type << endl;
+    outpustStream << entry.d_queued << '\t' << entry.d_pipeFull << '\t' << entry.d_tooLarge << '\t' << entry.d_otherError << '\t' << key << '\t' << type << endl;
   }
 }
 
 static string getRemoteLoggerStats()
 {
-  ostringstream os;
-  os << "Queued\tPipe-\tToo-\tOther-\tAddress\tType" << endl;
-  os << "\tFull\tLarge\terror" << endl;
+  ostringstream outputStream;
+  outputStream << "Queued\tPipe-\tToo-\tOther-\tAddress\tType" << endl;
+  outputStream << "\tFull\tLarge\terror" << endl;
   auto stats = broadcastAccFunction<RemoteLoggerStats_t>(pleaseGetRemoteLoggerStats);
-  remoteLoggerStats("protobuf", stats, os);
+  remoteLoggerStats("protobuf", stats, outputStream);
   stats = broadcastAccFunction<RemoteLoggerStats_t>(pleaseGetOutgoingRemoteLoggerStats);
-  remoteLoggerStats("outgoingProtobuf", stats, os);
+  remoteLoggerStats("outgoingProtobuf", stats, outputStream);
 #ifdef HAVE_FSTRM
   stats = broadcastAccFunction<RemoteLoggerStats_t>(pleaseGetFramestreamLoggerStats);
-  remoteLoggerStats("dnstapFrameStream", stats, os);
+  remoteLoggerStats("dnstapFrameStream", stats, outputStream);
 #endif
-  return os.str();
+  return outputStream.str();
 }
 
 static uint64_t calculateUptime()

diff --git a/pdns/remote_logger.hh b/pdns/remote_logger.hh
index 058b0c6c96b33fbf338821b2fbeb90c972baf1f4..a5390f27eea526ee53809f57888103b500eba26d 100644 (file)
--- a/pdns/remote_logger.hh
+++ b/pdns/remote_logger.hh
@@ -66,9 +66,9 @@ public:
 
   virtual ~RemoteLoggerInterface() {};
   virtual Result queueData(const std::string& data) = 0;
-  virtual std::string address() const = 0;
-  virtual std::string toString() = 0;
-  virtual const std::string name() const = 0;
+  [[nodiscard]] virtual std::string address() const = 0;
+  [[nodiscard]] virtual std::string toString() = 0;
+  [[nodiscard]] virtual std::string name() const = 0;
   bool logQueries(void) const { return d_logQueries; }
   bool logResponses(void) const { return d_logResponses; }
   void setLogQueries(bool flag) { d_logQueries = flag; }
@@ -91,7 +91,7 @@ public:
     }
   };
 
-  virtual Stats getStats() = 0;
+  [[nodiscard]] virtual Stats getStats() = 0;
 
 private:
   bool d_logQueries{true};
@@ -118,17 +118,17 @@ public:
   }
 
   [[nodiscard]] Result queueData(const std::string& data) override;
-  const std::string name() const override
+  [[nodiscard]] std::string name() const override
   {
     return "protobuf";
   }
-  std::string toString() override
+  [[nodiscard]] std::string toString() override
   {
     auto runtime = d_runtime.lock();
     return d_remote.toStringWithPort() + " (" + std::to_string(runtime->d_stats.d_queued) + " processed, " + std::to_string(runtime->d_stats.d_pipeFull + runtime->d_stats.d_tooLarge + runtime->d_stats.d_otherError) + " dropped)";
   }
 
-  virtual RemoteLoggerInterface::Stats getStats() override
+  [[nodiscard]] RemoteLoggerInterface::Stats getStats() override
   {
     return d_runtime.lock()->d_stats;
   }