dnsdist: clang-tidy fixes

PowerDNS · Jan 16, 2024 · 8595da5 · 8595da5
1 parent db86777
commit 8595da5
Show file tree

Hide file tree

Showing 8 changed files with 597 additions and 528 deletions.
diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc
diff --git a/pdns/dnsdist-lua.hh b/pdns/dnsdist-lua.hh
@@ -33,7 +33,7 @@ struct ResponseConfig
   boost::optional<bool> setRA{boost::none};
   uint32_t ttl{60};
 };
-void setResponseHeadersFromConfig(dnsheader& dh, const ResponseConfig& config);
+void setResponseHeadersFromConfig(dnsheader& dnsheader, const ResponseConfig& config);
 
 class SpoofAction : public DNSAction
 {
@@ -66,7 +66,7 @@ public:
   {
   }
 
-  DNSAction::Action operator()(DNSQuestion* dq, string* ruleresult) const override;
+  DNSAction::Action operator()(DNSQuestion* dnsquestion, string* ruleresult) const override;
 
   string toString() const override
   {
@@ -84,9 +84,13 @@ public:
     return ret;
   }
 
+  [[nodiscard]] ResponseConfig& getResponseConfig()
+  {
+    return d_responseConfig;
+  }
 
-  ResponseConfig d_responseConfig;
 private:
+  ResponseConfig d_responseConfig;
   static thread_local std::default_random_engine t_randomEngine;
   std::vector<ComboAddress> d_addrs;
   std::unordered_set<uint16_t> d_types;

diff --git a/pdns/dnsdist-protobuf.hh b/pdns/dnsdist-protobuf.hh
@@ -30,8 +30,10 @@
 class DNSDistProtoBufMessage
 {
 public:
-  DNSDistProtoBufMessage(const DNSQuestion& dq);
-  DNSDistProtoBufMessage(const DNSResponse& dr, bool includeCNAME);
+  DNSDistProtoBufMessage(const DNSQuestion& dnsquestion);
+  DNSDistProtoBufMessage(const DNSResponse& dnsresponse, bool includeCNAME);
+  DNSDistProtoBufMessage(const DNSQuestion&&) = delete;
+  DNSDistProtoBufMessage(const DNSResponse&&, bool) = delete;
 
   void setServerIdentity(const std::string& serverId);
   void setRequestor(const ComboAddress& requestor);
@@ -45,15 +47,15 @@ public:
   void setTime(time_t sec, uint32_t usec);
   void setQueryTime(time_t sec, uint32_t usec);
   void setQuestion(const DNSName& name, uint16_t qtype, uint16_t qclass);
-  void setEDNSSubnet(const Netmask& nm);
+  void setEDNSSubnet(const Netmask& netmask);
 
   void addTag(const std::string& strValue);
   void addMeta(const std::string& key, std::vector<std::string>&& strValues, const std::vector<int64_t>& intValues);
   void addRR(DNSName&& qname, uint16_t uType, uint16_t uClass, uint32_t uTTL, const std::string& data);
 
   void serialize(std::string& data) const;
 
-  std::string toDebugString() const;
+  [[nodiscard]] std::string toDebugString() const;
 
 private:
   struct PBRecord
@@ -66,8 +68,8 @@ private:
   };
   struct PBQuestion
   {
-    PBQuestion(const DNSName& name, uint16_t type, uint16_t class_) :
-      d_name(name), d_type(type), d_class(class_)
+    PBQuestion(DNSName name, uint16_t type, uint16_t class_) :
+      d_name(std::move(name)), d_type(type), d_class(class_)
     {
     }
 
@@ -85,6 +87,8 @@ private:
   };
   std::unordered_map<std::string, MetaValue> d_metaTags;
 
+  // FIXME wondering if the cost of moving to a shared_ptr would be that bad
+  // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
   const DNSQuestion& d_dq;
   const DNSResponse* d_dr{nullptr};
   const std::string* d_ServerIdentityRef{nullptr};
@@ -123,9 +127,9 @@ class ProtoBufMetaKey
 
   struct KeyTypeDescription
   {
-    const std::string d_name;
-    const Type d_type;
-    const std::function<std::vector<std::string>(const DNSQuestion&, const std::string&, uint8_t)> d_func;
+    std::string d_name;
+    Type d_type;
+    std::function<std::vector<std::string>(const DNSQuestion&, const std::string&, uint8_t)> d_func;
     bool d_prefix{false};
     bool d_caseSensitive{true};
     bool d_numeric{false};
@@ -138,20 +142,19 @@ class ProtoBufMetaKey
   {
   };
 
-  typedef boost::multi_index_container<
+  using TypeContainer = boost::multi_index_container<
     KeyTypeDescription,
     indexed_by<
       hashed_unique<tag<NameTag>, member<KeyTypeDescription, const std::string, &KeyTypeDescription::d_name>>,
-      hashed_unique<tag<TypeTag>, member<KeyTypeDescription, const Type, &KeyTypeDescription::d_type>>>>
-    TypeContainer;
+      hashed_unique<tag<TypeTag>, member<KeyTypeDescription, const Type, &KeyTypeDescription::d_type>>>>;
 
   static const TypeContainer s_types;
 
 public:
   ProtoBufMetaKey(const std::string& key);
 
-  const std::string& getName() const;
-  std::vector<std::string> getValues(const DNSQuestion& dq) const;
+  [[nodiscard]] const std::string& getName() const;
+  [[nodiscard]] std::vector<std::string> getValues(const DNSQuestion& dnsquestion) const;
 
 private:
   std::string d_subKey;

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
@@ -149,6 +149,13 @@ struct DNSQuestion
     ids.qTag->insert_or_assign(key, value);
   }
 
+  void setTag(const std::string& key, std::string&& value) {
+    if (!ids.qTag) {
+      ids.qTag = std::make_unique<QTag>();
+    }
+    ids.qTag->insert_or_assign(key, std::move(value));
+  }
+
   const struct timespec& getQueryRealTime() const
   {
     return ids.queryRealTime.d_start;

diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
@@ -100,7 +100,7 @@ DNSName::DNSName(const std::string_view sw)
 }
 
 
-DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, uint16_t minOffset)
+DNSName::DNSName(const char* pos, size_t len, size_t offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed, uint16_t minOffset)
 {
   if (offset >= len)
     throw std::range_error("Trying to read past the end of the buffer ("+std::to_string(offset)+ " >= "+std::to_string(len)+")");

diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
@@ -100,7 +100,7 @@ public:
   DNSName(DNSName&& a) = default;
 
   explicit DNSName(std::string_view sw); //!< Constructs from a human formatted, escaped presentation
-  DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=nullptr, uint16_t* qclass=nullptr, unsigned int* consumed=nullptr, uint16_t minOffset=0); //!< Construct from a DNS Packet, taking the first question if offset=12. If supplied, consumed is set to the number of bytes consumed from the packet, which will not be equal to the wire length of the resulting name in case of compression.
+  DNSName(const char* pos, size_t len, size_t offset, bool uncompress, uint16_t* qtype = nullptr, uint16_t* qclass = nullptr, unsigned int* consumed = nullptr, uint16_t minOffset = 0); //!< Construct from a DNS Packet, taking the first question if offset=12. If supplied, consumed is set to the number of bytes consumed from the packet, which will not be equal to the wire length of the resulting name in case of compression.
 
   bool isPartOf(const DNSName& rhs) const;   //!< Are we part of the rhs name? Note that name.isPartOf(name).
   inline bool operator==(const DNSName& rhs) const; //!< DNS-native comparison (case insensitive) - empty compares to empty

diff --git a/pdns/dnstap.cc b/pdns/dnstap.cc
@@ -57,7 +57,12 @@ enum : protozero::pbf_tag_type
 };
 }
 
-DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, DnstapMessage::ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, boost::optional<const DNSName&> auth) :
+std::string&& DnstapMessage::getBuffer()
+{
+  return std::move(d_buffer);
+}
+
+DnstapMessage::DnstapMessage(std::string&& buffer, DnstapMessage::MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, DnstapMessage::ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, const boost::optional<const DNSName&>& auth) :
   d_buffer(buffer)
 {
   protozero::pbf_writer pbf{d_buffer};
@@ -68,7 +73,9 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ
 
   protozero::pbf_writer pbf_message{pbf, DnstapBaseFields::message};
 
+  // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
   pbf_message.add_enum(DnstapMessageFields::type, static_cast<protozero::pbf_tag_type>(type));
+  // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions)
   pbf_message.add_enum(DnstapMessageFields::socket_protocol, static_cast<protozero::pbf_tag_type>(protocol));
 
   if (requestor != nullptr) {
@@ -80,19 +87,23 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ
 
   if (requestor != nullptr) {
     if (requestor->sin4.sin_family == AF_INET) {
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
       pbf_message.add_bytes(DnstapMessageFields::query_address, reinterpret_cast<const char*>(&requestor->sin4.sin_addr.s_addr), sizeof(requestor->sin4.sin_addr.s_addr));
     }
     else if (requestor->sin4.sin_family == AF_INET6) {
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
       pbf_message.add_bytes(DnstapMessageFields::query_address, reinterpret_cast<const char*>(&requestor->sin6.sin6_addr.s6_addr), sizeof(requestor->sin6.sin6_addr.s6_addr));
     }
     pbf_message.add_uint32(DnstapMessageFields::query_port, ntohs(requestor->sin4.sin_port));
   }
 
   if (responder != nullptr) {
     if (responder->sin4.sin_family == AF_INET) {
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
       pbf_message.add_bytes(DnstapMessageFields::response_address, reinterpret_cast<const char*>(&responder->sin4.sin_addr.s_addr), sizeof(responder->sin4.sin_addr.s_addr));
     }
     else if (responder->sin4.sin_family == AF_INET6) {
+      // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
       pbf_message.add_bytes(DnstapMessageFields::response_address, reinterpret_cast<const char*>(&responder->sin6.sin6_addr.s6_addr), sizeof(responder->sin6.sin6_addr.s6_addr));
     }
     pbf_message.add_uint32(DnstapMessageFields::response_port, ntohs(responder->sin4.sin_port));
@@ -109,8 +120,8 @@ DnstapMessage::DnstapMessage(std::string& buffer, DnstapMessage::MessageType typ
   }
 
   if (packet != nullptr && len >= sizeof(dnsheader)) {
-    const dnsheader_aligned dh(packet);
-    if (!dh->qr) {
+    const dnsheader_aligned dnsheader(packet);
+    if (!dnsheader->qr) {
       pbf_message.add_bytes(DnstapMessageFields::query_message, packet, len);
     }
     else {

diff --git a/pdns/dnstap.hh b/pdns/dnstap.hh
@@ -61,12 +61,13 @@ public:
     DoQ = 7
   };
 
-  DnstapMessage(std::string& buffer, MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, ProtocolType protocol, const char* packet, const size_t len, const struct timespec* queryTime, const struct timespec* responseTime, boost::optional<const DNSName&> auth = boost::none);
+  DnstapMessage(std::string&& buffer, MessageType type, const std::string& identity, const ComboAddress* requestor, const ComboAddress* responder, ProtocolType protocol, const char* packet, size_t len, const struct timespec* queryTime, const struct timespec* responseTime, const boost::optional<const DNSName&>& auth = boost::none);
 
   void setExtra(const std::string& extra);
+  std::string&& getBuffer();
 
-protected:
-  std::string& d_buffer;
+private:
+  std::string d_buffer;
 };
 
 #endif /* DISABLE_PROTOBUF */