tidy

diff --git a/pdns/fstrm_logger.cc b/pdns/fstrm_logger.cc
index 70863fde493510b143896ddd5f176347cfbd56c4..6f31fb1450eb60ab270318267cffad647d4a7e3e 100644 (file)
--- a/pdns/fstrm_logger.cc
+++ b/pdns/fstrm_logger.cc
@@ -11,34 +11,34 @@
 #include "dolog.hh"
 #endif
 
-#define DNSTAP_CONTENT_TYPE            "protobuf:dnstap.Dnstap"
+#define DNSTAP_CONTENT_TYPE "protobuf:dnstap.Dnstap"
 
 #ifdef HAVE_FSTRM
 
-FrameStreamLogger::FrameStreamLogger(const int family, const std::string& address, bool connect,
-    const std::unordered_map<string,unsigned>& options): d_family(family), d_address(address)
+FrameStreamLogger::FrameStreamLogger(const int family, std::string address, bool connect, const std::unordered_map<string, unsigned>& options) :
+  d_family(family), d_address(std::move(address))
 {
-  fstrm_res res;
-
   try {
     d_fwopt = fstrm_writer_options_init();
-    if (!d_fwopt) {
+    if (d_fwopt == nullptr) {
       throw std::runtime_error("FrameStreamLogger: fstrm_writer_options_init failed.");
     }
 
-    res = fstrm_writer_options_add_content_type(d_fwopt, DNSTAP_CONTENT_TYPE, sizeof(DNSTAP_CONTENT_TYPE) - 1);
+    auto res = fstrm_writer_options_add_content_type(d_fwopt, DNSTAP_CONTENT_TYPE, sizeof(DNSTAP_CONTENT_TYPE) - 1);
     if (res != fstrm_res_success) {
       throw std::runtime_error("FrameStreamLogger: fstrm_writer_options_add_content_type failed: " + std::to_string(res));
     }
 
     if (d_family == AF_UNIX) {
-      struct sockaddr_un local;
-      if (makeUNsockaddr(d_address, &local)) {
+      struct sockaddr_un local
+      {
+      };
+      if (makeUNsockaddr(d_address, &local) != 0) {
         throw std::runtime_error("FrameStreamLogger: Unable to use '" + d_address + "', it is not a valid UNIX socket path.");
       }
 
       d_uwopt = fstrm_unix_writer_options_init();
-      if (!d_uwopt) {
+      if (d_uwopt == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_unix_writer_options_init failed.");
       }
 
@@ -46,37 +46,40 @@ FrameStreamLogger::FrameStreamLogger(const int family, const std::string& addres
       fstrm_unix_writer_options_set_socket_path(d_uwopt, d_address.c_str());
 
       d_writer = fstrm_unix_writer_init(d_uwopt, d_fwopt);
-      if (!d_writer) {
+      if (d_writer == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_unix_writer_init() failed.");
       }
-  #ifdef HAVE_FSTRM_TCP_WRITER_INIT
-    } else if (family == AF_INET || family == AF_INET6) {
+#ifdef HAVE_FSTRM_TCP_WRITER_INIT
+    }
+    else if (family == AF_INET || family == AF_INET6) {
       d_twopt = fstrm_tcp_writer_options_init();
-      if (!d_twopt) {
+      if (d_twopt == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_tcp_writer_options_init failed.");
       }
 
       try {
-        ComboAddress ca(d_address);
+        ComboAddress inetAddress(d_address);
 
         // void return, no error checking.
-        fstrm_tcp_writer_options_set_socket_address(d_twopt, ca.toString().c_str());
-        fstrm_tcp_writer_options_set_socket_port(d_twopt, std::to_string(ca.getPort()).c_str());
-      } catch (PDNSException &e) {
+        fstrm_tcp_writer_options_set_socket_address(d_twopt, inetAddress.toString().c_str());
+        fstrm_tcp_writer_options_set_socket_port(d_twopt, std::to_string(inetAddress.getPort()).c_str());
+      }
+      catch (PDNSException& e) {
         throw std::runtime_error("FrameStreamLogger: Unable to use '" + d_address + "': " + e.reason);
       }
 
       d_writer = fstrm_tcp_writer_init(d_twopt, d_fwopt);
-      if (!d_writer) {
+      if (d_writer == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_tcp_writer_init() failed.");
       }
-  #endif
-    } else {
+#endif
+    }
+    else {
       throw std::runtime_error("FrameStreamLogger: family " + std::to_string(family) + " not supported");
     }
 
     d_iothropt = fstrm_iothr_options_init();
-    if (!d_iothropt) {
+    if (d_iothropt == nullptr) {
       throw std::runtime_error("FrameStreamLogger: fstrm_iothr_options_init() failed.");
     }
 
@@ -85,39 +88,40 @@ FrameStreamLogger::FrameStreamLogger(const int family, const std::string& addres
       throw std::runtime_error("FrameStreamLogger: fstrm_iothr_options_set_queue_model failed: " + std::to_string(res));
     }
 
-    const struct {
+    struct setters
+    {
       const std::string name;
-      fstrm_res (*function)(struct fstrm_iothr_options *, const unsigned int);
-    } list[] = {
-      { "bufferHint", fstrm_iothr_options_set_buffer_hint },
-      { "flushTimeout", fstrm_iothr_options_set_flush_timeout },
-      { "inputQueueSize", fstrm_iothr_options_set_input_queue_size },
-      { "outputQueueSize", fstrm_iothr_options_set_output_queue_size },
-      { "queueNotifyThreshold", fstrm_iothr_options_set_queue_notify_threshold },
-      { "setReopenInterval", fstrm_iothr_options_set_reopen_interval }
+      fstrm_res (*function)(struct fstrm_iothr_options*, const unsigned int);
     };
-
-    for (const auto& i : list) {
-      if (options.find(i.name) != options.end() && options.at(i.name)) {
-        fstrm_res r = i.function(d_iothropt, options.at(i.name));
-        if (r != fstrm_res_success) {
-          throw std::runtime_error("FrameStreamLogger: setting " + string(i.name) + " failed: " + std::to_string(r));
+    const std::array<struct setters, 6> list = {{{"bufferHint", fstrm_iothr_options_set_buffer_hint},
+                                                 {"flushTimeout", fstrm_iothr_options_set_flush_timeout},
+                                                 {"inputQueueSize", fstrm_iothr_options_set_input_queue_size},
+                                                 {"outputQueueSize", fstrm_iothr_options_set_output_queue_size},
+                                                 {"queueNotifyThreshold", fstrm_iothr_options_set_queue_notify_threshold},
+                                                 {"setReopenInterval", fstrm_iothr_options_set_reopen_interval}}};
+
+    for (const auto& entry : list) {
+      if (auto option = options.find(entry.name); option != options.end() && option->second != 0) {
+        auto result = entry.function(d_iothropt, option->second);
+        if (result != fstrm_res_success) {
+          throw std::runtime_error("FrameStreamLogger: setting " + string(entry.name) + " failed: " + std::to_string(result));
         }
       }
     }
 
     if (connect) {
       d_iothr = fstrm_iothr_init(d_iothropt, &d_writer);
-      if (!d_iothr) {
+      if (d_iothr == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_iothr_init() failed.");
       }
 
       d_ioqueue = fstrm_iothr_get_input_queue(d_iothr);
-      if (!d_ioqueue) {
+      if (d_ioqueue == nullptr) {
         throw std::runtime_error("FrameStreamLogger: fstrm_iothr_get_input_queue() failed.");
       }
     }
-  } catch (std::runtime_error &e) {
+  }
+  catch (std::runtime_error& e) {
     this->cleanup();
     throw;
   }
@@ -160,34 +164,34 @@ FrameStreamLogger::~FrameStreamLogger()
 
 RemoteLoggerInterface::Result FrameStreamLogger::queueData(const std::string& data)
 {
-  if (!d_ioqueue || !d_iothr) {
+  if ((d_ioqueue == nullptr) || d_iothr == nullptr) {
     ++d_permanentFailures;
     return Result::OtherError;
   }
-  uint8_t *frame = (uint8_t*)malloc(data.length());
-  if (!frame) {
+  uint8_t* frame = (uint8_t*)malloc(data.length()); // NOLINT: it's the API
+  if (frame == nullptr) {
     ++d_queueFullDrops; // XXX separate count?
     return Result::TooLarge;
   }
   memcpy(frame, data.c_str(), data.length());
 
-  fstrm_res res;
-  res = fstrm_iothr_submit(d_iothr, d_ioqueue, frame, data.length(), fstrm_free_wrapper, nullptr);
+  auto res = fstrm_iothr_submit(d_iothr, d_ioqueue, frame, data.length(), fstrm_free_wrapper, nullptr);
 
   if (res == fstrm_res_success) {
     // Frame successfully queued.
     ++d_framesSent;
+    // do not call free here
     return Result::Queued;
-  } else if (res == fstrm_res_again) {
-    free(frame);
+  }
+  if (res == fstrm_res_again) {
+    free(frame); // NOLINT: it's the API
     ++d_queueFullDrops;
     return Result::PipeFull;
- } else {
-    // Permanent failure.
-    free(frame);
-    ++d_permanentFailures;
-    return Result::OtherError;
   }
+  // Permanent failure.
+  free(frame); // NOLINT: it's the API
+  ++d_permanentFailures;
+  return Result::OtherError;
 }
 
 #endif /* HAVE_FSTRM */

diff --git a/pdns/fstrm_logger.hh b/pdns/fstrm_logger.hh
index 3eafb3d997f8a9a6617cc694cb3535d310308339..711c65a17189388daa39006b9b1f55a946840c42 100644 (file)
--- a/pdns/fstrm_logger.hh
+++ b/pdns/fstrm_logger.hh
@@ -33,11 +33,15 @@
 #include <fstrm/tcp_writer.h>
 #endif
 
-class FrameStreamLogger : public RemoteLoggerInterface, boost::noncopyable
+class FrameStreamLogger : public RemoteLoggerInterface
 {
 public:
-  FrameStreamLogger(int family, const std::string& address, bool connect, const std::unordered_map<string,unsigned>& options = std::unordered_map<string,unsigned>());
-  ~FrameStreamLogger();
+  FrameStreamLogger(int family, std::string address, bool connect, const std::unordered_map<string, unsigned>& options = std::unordered_map<string, unsigned>());
+  FrameStreamLogger(const FrameStreamLogger&) = delete;
+  FrameStreamLogger(FrameStreamLogger&&) = delete;
+  FrameStreamLogger& operator=(const FrameStreamLogger&) = delete;
+  FrameStreamLogger& operator=(FrameStreamLogger&&) = delete;
+  ~FrameStreamLogger() override;
   [[nodiscard]] RemoteLoggerInterface::Result queueData(const std::string& data) override;
 
   [[nodiscard]] std::string address() const override
@@ -49,7 +53,7 @@ public:
   {
     return "dnstap";
   }
-  
+
   [[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)";
@@ -60,23 +64,21 @@ public:
     return Stats{.d_queued = d_framesSent,
                  .d_pipeFull = d_queueFullDrops,
                  .d_tooLarge = 0,
-                 .d_otherError = d_permanentFailures
-    };
+                 .d_otherError = d_permanentFailures};
   }
 
 private:
-
   const int d_family;
   const std::string d_address;
-  struct fstrm_iothr_queue *d_ioqueue{nullptr};
-  struct fstrm_writer_options *d_fwopt{nullptr};
-  struct fstrm_unix_writer_options *d_uwopt{nullptr};
+  struct fstrm_iothr_queue* d_ioqueue{nullptr};
+  struct fstrm_writer_options* d_fwopt{nullptr};
+  struct fstrm_unix_writer_options* d_uwopt{nullptr};
 #ifdef HAVE_FSTRM_TCP_WRITER_INIT
-  struct fstrm_tcp_writer_options *d_twopt{nullptr};
+  struct fstrm_tcp_writer_options* d_twopt{nullptr};
 #endif
-  struct fstrm_writer *d_writer{nullptr};
-  struct fstrm_iothr_options *d_iothropt{nullptr};
-  struct fstrm_iothr *d_iothr{nullptr};
+  struct fstrm_writer* d_writer{nullptr};
+  struct fstrm_iothr_options* d_iothropt{nullptr};
+  struct fstrm_iothr* d_iothr{nullptr};
   std::atomic<uint64_t> d_framesSent{0};
   std::atomic<uint64_t> d_queueFullDrops{0};
   std::atomic<uint64_t> d_permanentFailures{0};
@@ -85,5 +87,11 @@ private:
 };
 
 #else
-class FrameStreamLogger : public RemoteLoggerInterface, boost::noncopyable {};
+class FrameStreamLogger : public RemoteLoggerInterface
+{
+  FrameStreamLogger(const FrameStreamLogger&) = delete;
+  FrameStreamLogger(FrameStreamLogger&&) = delete;
+  FrameStreamLogger& operator=(const FrameStreamLogger&) = delete;
+  FrameStreamLogger& operator=(FrameStreamLogger&&) = delete;
+};
 #endif /* HAVE_FSTRM */