fix(dnsdist): Don't allocate strings before they are needed

Also make clang-tidy silent by ignoring the decay into pointer warnings

diff --git a/pdns/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc
index 1c5b3cca4c3d68d100adaeb23fed3fa85f548ac3..7e80710a715d74f93e8c20160797dcdde5b8e53a 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.cc
+++ b/pdns/dnsdistdist/dnsdist-idstate.cc
@@ -24,6 +24,8 @@
 #include "dnsdist-doh-common.hh"
 #include "doh3.hh"
 #include "doq.hh"
+#include <string>
+#include <string_view>
 
 InternalQueryState InternalQueryState::partialCloneForXFR() const
 {
@@ -81,7 +83,7 @@ void InternalQueryState::sendDelayedProtobufMessages() const
 #endif
 }
 
-std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID)
+std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const SpanID& parentSpanID)
 {
   std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
 #ifndef DISABLE_PROTOBUF
@@ -89,36 +91,36 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
   // tracingEnabled tells us whether or not tracing is enabled for this query
   // Should tracing be disabled, *but* we have not processed query rules, we will still return a closer if tracing is globally enabled
   if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) {
-    ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(name, parentSpanID));
+    ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(std::string(name), parentSpanID));
   }
 #endif
   return ret;
 }
 
-std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const std::string& parentSpanName)
+std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const std::string_view& parentSpanName)
 {
   std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
 #ifndef DISABLE_PROTOBUF
   if (auto tracer = getTracer(); tracer != nullptr) {
-    auto parentSpanID = d_OTTracer->getLastSpanIDForName(parentSpanName);
+    auto parentSpanID = d_OTTracer->getLastSpanIDForName(std::string(parentSpanName));
     return getCloser(name, parentSpanID);
   }
 #endif
   return ret;
 }
 
-std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string& name)
+std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name)
 {
   std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
 #ifndef DISABLE_PROTOBUF
   if (auto tracer = getTracer(); tracer != nullptr) {
-    return getCloser(name, d_OTTracer->getLastSpanID());
+    return getCloser(std::string(name), d_OTTracer->getLastSpanID());
   }
 #endif
   return ret;
 }
 
-std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName)
+std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRulesCloser([[maybe_unused]] const std::string_view& ruleName, [[maybe_unused]] const std::string_view& parentSpanName)
 {
   std::optional<pdns::trace::dnsdist::Tracer::Closer> ret(std::nullopt);
 #ifndef DISABLE_PROTOBUF
@@ -127,8 +129,8 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRules
   // tracingEnabled tells us whether or not tracing is enabled for this query
   // Should tracing be disabled, *but* we have not processed query rules, we will still return a closer if tracing is globally enabled
   if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) {
-    auto parentSpanID = d_OTTracer->getLastSpanIDForName(parentSpanName);
-    auto name = prefix + ruleName;
+    auto parentSpanID = d_OTTracer->getLastSpanIDForName(std::string(parentSpanName));
+    auto name = prefix + std::string(ruleName);
     ret = std::optional<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(name, parentSpanID));
   }
 #endif

diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh
index 2587ec7874a2efe085050f68c58abd7ae8e73031..25ecbb36c1d624a437cc4f64753dc7b42a4f5a4d 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.hh
+++ b/pdns/dnsdistdist/dnsdist-idstate.hh
@@ -25,6 +25,7 @@
 #include <ctime>
 #include <memory>
 #include <optional>
+#include <string_view>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -147,10 +148,10 @@ struct InternalQueryState
    *
    * @return
    */
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID);
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const string& parentSpanName);
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name);
-  std::optional<pdns::trace::dnsdist::Tracer::Closer> getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const SpanID& parentSpanID);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const std::string_view& parentSpanName);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string_view& name);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> getRulesCloser([[maybe_unused]] const std::string_view& ruleName, [[maybe_unused]] const std::string_view& parentSpanName);
 
   InternalQueryState()
   {

diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 6f58c7e9a985ab84031cf9e456a72dffafd1002c..ac143f343d4343315bcd51d88ddf74559e6802cd 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -456,7 +456,7 @@ static bool encryptResponse(PacketBuffer& response, size_t maximumSize, bool tcp
 
 bool applyRulesToResponse(const std::vector<dnsdist::rules::ResponseRuleAction>& respRuleActions, DNSResponse& dnsResponse)
 {
-  auto closer = dnsResponse.ids.getCloser(__func__);
+  auto closer = dnsResponse.ids.getCloser(__func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   if (respRuleActions.empty()) {
     return true;
   }
@@ -465,7 +465,7 @@ bool applyRulesToResponse(const std::vector<dnsdist::rules::ResponseRuleAction>&
   std::string ruleresult;
 
   for (const auto& rrule : respRuleActions) {
-    auto ruleCloser = dnsResponse.ids.getRulesCloser(rrule.d_name, __func__);
+    auto ruleCloser = dnsResponse.ids.getRulesCloser(rrule.d_name, __func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
     if (rrule.d_rule->matches(&dnsResponse)) {
       ++rrule.d_rule->d_matches;
       action = (*rrule.d_action)(&dnsResponse, &ruleresult);
@@ -519,7 +519,7 @@ bool applyRulesToResponse(const std::vector<dnsdist::rules::ResponseRuleAction>&
 
 bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, [[maybe_unused]] bool muted)
 {
-  auto closer = dnsResponse.ids.getCloser(__func__, "processResponse");
+  auto closer = dnsResponse.ids.getCloser(__func__, "processResponse"); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   bool zeroScope = false;
   if (!fixUpResponse(response, dnsResponse.ids.qname, dnsResponse.ids.origFlags, dnsResponse.ids.ednsAdded, dnsResponse.ids.ecsAdded, dnsResponse.ids.useZeroScope ? &zeroScope : nullptr)) {
     if (closer) {
@@ -551,7 +551,7 @@ bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse,
       cacheKey = dnsResponse.ids.cacheKeyNoECS;
     }
     {
-      auto cacheInsertCloser = dnsResponse.ids.getCloser("packetCacheInsert", __func__);
+      auto cacheInsertCloser = dnsResponse.ids.getCloser("packetCacheInsert", __func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
       dnsResponse.ids.packetCache->insert(cacheKey, zeroScope ? boost::none : dnsResponse.ids.subnet, dnsResponse.ids.cacheFlags, dnsResponse.ids.dnssecOK ? *dnsResponse.ids.dnssecOK : false, dnsResponse.ids.qname, dnsResponse.ids.qtype, dnsResponse.ids.qclass, response, dnsResponse.ids.forwardedOverUDP, dnsResponse.getHeader()->rcode, dnsResponse.ids.tempFailureTTL);
     }
     const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains;
@@ -583,7 +583,7 @@ bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse,
 bool processResponse(PacketBuffer& response, DNSResponse& dnsResponse, bool muted)
 {
   // This is a new root span
-  auto closer = dnsResponse.ids.getCloser(__func__, SpanID{});
+  auto closer = dnsResponse.ids.getCloser(__func__, SpanID{}); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
   const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains;
   const auto& respRuleActions = dnsdist::rules::getResponseRuleChain(chains, dnsdist::rules::ResponseRuleChain::ResponseRules);
@@ -1022,10 +1022,10 @@ static bool applyRulesChainToQuery(const std::vector<dnsdist::rules::RuleAction>
   string ruleresult;
   bool drop = false;
 
-  auto closer = dnsQuestion.ids.getCloser(__func__);
+  auto closer = dnsQuestion.ids.getCloser(__func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
   for (const auto& rule : rules) {
-    auto ruleCloser = dnsQuestion.ids.getRulesCloser(rule.d_name, __func__);
+    auto ruleCloser = dnsQuestion.ids.getRulesCloser(rule.d_name, __func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
     if (!rule.d_rule->matches(&dnsQuestion)) {
       continue;
@@ -1044,7 +1044,7 @@ static bool applyRulesChainToQuery(const std::vector<dnsdist::rules::RuleAction>
 static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
 {
   InternalQueryState::rulesAppliedToQuerySetter tpprs(dnsQuestion.ids.rulesAppliedToQuery); // Ensure IDS knows we are past the rules processing when we exit this function
-  auto closer = dnsQuestion.ids.getCloser(__func__);
+  auto closer = dnsQuestion.ids.getCloser(__func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   if (g_rings.shouldRecordQueries()) {
     g_rings.insertQuery(now, dnsQuestion.ids.origRemote, dnsQuestion.ids.qname, dnsQuestion.ids.qtype, dnsQuestion.getData().size(), *dnsQuestion.getHeader(), dnsQuestion.getProtocol());
   }
@@ -1453,7 +1453,7 @@ static ProcessQueryResult handleQueryTurnedIntoSelfAnsweredResponse(DNSQuestion&
 static ServerPolicy::SelectedBackend selectBackendForOutgoingQuery(DNSQuestion& dnsQuestion, const ServerPool& serverPool)
 {
   // Not exactly processQuery, but it works for now
-  auto closer = dnsQuestion.ids.getCloser(__func__, "processQuery");
+  auto closer = dnsQuestion.ids.getCloser(__func__, "processQuery"); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
   const auto& policy = serverPool.policy != nullptr ? *serverPool.policy : *dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy;
   const auto& servers = serverPool.getServers();
@@ -1779,7 +1779,7 @@ std::unique_ptr<CrossProtocolQuery> getUDPCrossProtocolQueryFromDQ(DNSQuestion&
 ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr<DownstreamState>& selectedBackend)
 {
 
-  auto closer = dnsQuestion.ids.getCloser(__func__);
+  auto closer = dnsQuestion.ids.getCloser(__func__); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
   const uint16_t queryId = ntohs(dnsQuestion.getHeader()->id);
   try {
     /* we need an accurate ("real") value for the response and
@@ -1811,7 +1811,7 @@ ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr<Downst
 
 bool assignOutgoingUDPQueryToBackend(std::shared_ptr<DownstreamState>& downstream, uint16_t queryID, DNSQuestion& dnsQuestion, PacketBuffer& query, bool actuallySend)
 {
-  auto closer = dnsQuestion.ids.getCloser(__func__, "processUDPQuery");
+  auto closer = dnsQuestion.ids.getCloser(__func__, "processUDPQuery"); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
   bool doh = dnsQuestion.ids.du != nullptr;
 
@@ -1887,7 +1887,7 @@ static void processUDPQuery(ClientState& clientState, const struct msghdr* msgh,
   uint16_t queryId = 0;
   InternalQueryState ids;
 
-  auto closer = ids.getCloser(__func__, SpanID{});
+  auto closer = ids.getCloser(__func__, SpanID{}); // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay)
 
   ids.cs = &clientState;
   ids.origRemote = remote;