From: Pieter Lexis Date: Wed, 12 Nov 2025 15:23:59 +0000 (+0100) Subject: fix(dnsdist): Don't allocate strings before they are needed X-Git-Tag: rec-5.4.0-alpha1~72^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e04fdddd76562ab9d8e048cee28f5279565b1974;p=thirdparty%2Fpdns.git 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 1c5b3cca4c..7e80710a71 100644 --- 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 +#include InternalQueryState InternalQueryState::partialCloneForXFR() const { @@ -81,7 +83,7 @@ void InternalQueryState::sendDelayedProtobufMessages() const #endif } -std::optional InternalQueryState::getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID) +std::optional InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const SpanID& parentSpanID) { std::optional ret(std::nullopt); #ifndef DISABLE_PROTOBUF @@ -89,36 +91,36 @@ std::optional 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(d_OTTracer->openSpan(name, parentSpanID)); + ret = std::optional(d_OTTracer->openSpan(std::string(name), parentSpanID)); } #endif return ret; } -std::optional InternalQueryState::getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const std::string& parentSpanName) +std::optional InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const std::string_view& parentSpanName) { std::optional 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 InternalQueryState::getCloser([[maybe_unused]] const std::string& name) +std::optional InternalQueryState::getCloser([[maybe_unused]] const std::string_view& name) { std::optional 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 InternalQueryState::getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName) +std::optional InternalQueryState::getRulesCloser([[maybe_unused]] const std::string_view& ruleName, [[maybe_unused]] const std::string_view& parentSpanName) { std::optional ret(std::nullopt); #ifndef DISABLE_PROTOBUF @@ -127,8 +129,8 @@ std::optional 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(d_OTTracer->openSpan(name, parentSpanID)); } #endif diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh index 2587ec7874..25ecbb36c1 100644 --- a/pdns/dnsdistdist/dnsdist-idstate.hh +++ b/pdns/dnsdistdist/dnsdist-idstate.hh @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -147,10 +148,10 @@ struct InternalQueryState * * @return */ - std::optional getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID); - std::optional getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const string& parentSpanName); - std::optional getCloser([[maybe_unused]] const std::string& name); - std::optional getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName); + std::optional getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const SpanID& parentSpanID); + std::optional getCloser([[maybe_unused]] const std::string_view& name, [[maybe_unused]] const std::string_view& parentSpanName); + std::optional getCloser([[maybe_unused]] const std::string_view& name); + std::optional 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 6f58c7e9a9..ac143f343d 100644 --- 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& 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& 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& 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 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 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 getUDPCrossProtocolQueryFromDQ(DNSQuestion& ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr& 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& 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;