From: Remi Gacogne Date: Tue, 7 Oct 2025 08:46:23 +0000 (+0200) Subject: dnsdist: Appease clang-tidy X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=debc75c7028eec6e3a9b1bccbf3c00692c723c8e;p=thirdparty%2Fpdns.git dnsdist: Appease clang-tidy Signed-off-by: Remi Gacogne --- diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc index ad8d7fde31..701564e96f 100644 --- a/pdns/dnsdistdist/dnsdist-backend.cc +++ b/pdns/dnsdistdist/dnsdist-backend.cc @@ -55,7 +55,8 @@ void DownstreamState::addXSKDestination(int fd) { auto socklen = d_config.remote.getSocklen(); ComboAddress local; - if (getsockname(fd, reinterpret_cast(&local), &socklen)) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): sorry, it's the API + if (getsockname(fd, reinterpret_cast(&local), &socklen) != 0) { return; } @@ -73,7 +74,8 @@ void DownstreamState::removeXSKDestination(int fd) { auto socklen = d_config.remote.getSocklen(); ComboAddress local; - if (getsockname(fd, reinterpret_cast(&local), &socklen)) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): sorry, it's the API + if (getsockname(fd, reinterpret_cast(&local), &socklen) != 0) { return; } @@ -86,8 +88,8 @@ void DownstreamState::removeXSKDestination(int fd) bool DownstreamState::reconnect(bool initialAttempt) { - std::unique_lock tl(connectLock, std::try_to_lock); - if (!tl.owns_lock() || isStopped()) { + std::unique_lock lock(connectLock, std::try_to_lock); + if (!lock.owns_lock() || isStopped()) { /* we are already reconnecting or stopped anyway */ return false; } @@ -199,7 +201,7 @@ bool DownstreamState::reconnect(bool initialAttempt) } if (connected) { - tl.unlock(); + lock.unlock(); d_connectedWait.notify_all(); if (!initialAttempt) { /* we need to be careful not to start this @@ -252,17 +254,17 @@ void DownstreamState::hash() { const auto hashPerturbation = dnsdist::configuration::getImmutableConfiguration().d_hashPerturbation; vinfolog("Computing hashes for id=%s and weight=%d, hash_perturbation=%d", *d_config.id, d_config.d_weight, hashPerturbation); - auto w = d_config.d_weight; + auto weight = d_config.d_weight; auto idStr = boost::str(boost::format("%s") % *d_config.id); auto lockedHashes = hashes.write_lock(); lockedHashes->clear(); - lockedHashes->reserve(w); - while (w > 0) { - std::string uuid = boost::str(boost::format("%s-%d") % idStr % w); + lockedHashes->reserve(weight); + while (weight > 0) { + std::string uuid = boost::str(boost::format("%s-%d") % idStr % weight); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): sorry, it's the burtle API unsigned int wshash = burtleCI(reinterpret_cast(uuid.c_str()), uuid.size(), hashPerturbation); lockedHashes->push_back(wshash); - --w; + --weight; } std::sort(lockedHashes->begin(), lockedHashes->end()); hashesComputed = true; @@ -458,10 +460,10 @@ void DownstreamState::handleUDPTimeout(IDState& ids) } if (g_rings.shouldRecordResponses()) { - struct timespec ts; + timespec ts{}; gettime(&ts); - struct dnsheader fake; + dnsheader fake{}; memset(&fake, 0, sizeof(fake)); fake.id = ids.internal.origID; uint16_t* flags = getFlagsFromDNSHeader(&fake); @@ -1044,11 +1046,11 @@ const ServerPolicy::NumberedServerVector& ServerPool::getServers() const void ServerPool::addServer(std::shared_ptr& server) { - unsigned int count = static_cast(d_servers.size()); + auto count = static_cast(d_servers.size()); d_servers.emplace_back(++count, server); /* we need to reorder based on the server 'order' */ - std::stable_sort(d_servers.begin(), d_servers.end(), [](const std::pair >& a, const std::pair >& b) { - return a.second->d_config.order < b.second->d_config.order; + std::stable_sort(d_servers.begin(), d_servers.end(), [](const std::pair >& lhs, const std::pair >& rhs) { + return lhs.second->d_config.order < rhs.second->d_config.order; }); /* and now we need to renumber for Lua (custom policies) */ size_t idx = 1; diff --git a/pdns/dnsdistdist/dnsdist-lbpolicies.cc b/pdns/dnsdistdist/dnsdist-lbpolicies.cc index dae8630e53..fc23139186 100644 --- a/pdns/dnsdistdist/dnsdist-lbpolicies.cc +++ b/pdns/dnsdistdist/dnsdist-lbpolicies.cc @@ -80,14 +80,14 @@ std::optional leastOutstanding(const Serve return getLeastOutstanding(servers); } -std::optional firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq) +std::optional firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion) { - for (auto& server : servers) { + for (const auto& server : servers) { if (server.second->isUp() && (!server.second->d_qpsLimiter || server.second->d_qpsLimiter->checkOnly())) { return server.first; } } - return leastOutstanding(servers, dq); + return leastOutstanding(servers, dnsQuestion); } template static std::optional getValRandom(const ServerPolicy::NumberedServerVector& servers, T& poss, const unsigned int val, const double targetLoad) @@ -97,16 +97,16 @@ template static std::optional ge size_t usableServers = 0; const auto weightedBalancingFactor = dnsdist::configuration::getImmutableConfiguration().d_weightedBalancingFactor; - for (const auto& d : servers) { // w=1, w=10 -> 1, 11 - if (d.second->isUp() && (weightedBalancingFactor == 0 || (static_cast(d.second->outstanding.load()) <= (targetLoad * d.second->d_config.d_weight)))) { + for (const auto& server : servers) { // w=1, w=10 -> 1, 11 + if (server.second->isUp() && (weightedBalancingFactor == 0 || (static_cast(server.second->outstanding.load()) <= (targetLoad * server.second->d_config.d_weight)))) { // Don't overflow sum when adding high weights - if (d.second->d_config.d_weight > max - sum) { + if (server.second->d_config.d_weight > max - sum) { sum = max; } else { - sum += d.second->d_config.d_weight; + sum += server.second->d_config.d_weight; } - poss.at(usableServers) = std::pair(sum, d.first); + poss.at(usableServers) = std::pair(sum, server.first); usableServers++; } } @@ -116,13 +116,13 @@ template static std::optional ge return std::nullopt; } - int r = val % sum; - auto p = std::upper_bound(poss.begin(), poss.begin() + usableServers, r, [](int r_, const typename T::value_type& a) { return r_ < a.first;}); - if (p == poss.begin() + usableServers) { + int randomVal = val % sum; + auto selected = std::upper_bound(poss.begin(), poss.begin() + usableServers, randomVal, [](int randomVal_, const typename T::value_type& serverPair) { return randomVal_ < serverPair.first;}); + if (selected == poss.begin() + usableServers) { return std::nullopt; } - return p->second; + return selected->second; } static std::optional valrandom(const unsigned int val, const ServerPolicy::NumberedServerVector& servers) @@ -166,17 +166,18 @@ std::optional whashedFromHash(const Server return valrandom(hash, servers); } -std::optional whashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq) +std::optional whashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion) { const auto hashPerturbation = dnsdist::configuration::getImmutableConfiguration().d_hashPerturbation; - return whashedFromHash(servers, dq->ids.qname.hash(hashPerturbation)); + return whashedFromHash(servers, dnsQuestion->ids.qname.hash(hashPerturbation)); } std::optional chashedFromHash(const ServerPolicy::NumberedServerVector& servers, size_t qhash) { unsigned int sel = std::numeric_limits::max(); unsigned int min = std::numeric_limits::max(); - std::optional ret, first; + std::optional ret; + std::optional first; double targetLoad = std::numeric_limits::max(); const auto consistentHashBalancingFactor = dnsdist::configuration::getImmutableConfiguration().d_consistentHashBalancingFactor; @@ -196,15 +197,15 @@ std::optional chashedFromHash(const Server } } - for (const auto& d: servers) { - if (d.second->isUp() && (consistentHashBalancingFactor == 0 || static_cast(d.second->outstanding.load()) <= (targetLoad * d.second->d_config.d_weight))) { + for (const auto& serverPair: servers) { + if (serverPair.second->isUp() && (consistentHashBalancingFactor == 0 || static_cast(serverPair.second->outstanding.load()) <= (targetLoad * serverPair.second->d_config.d_weight))) { // make sure hashes have been computed - if (!d.second->hashesComputed) { - d.second->hash(); + if (!serverPair.second->hashesComputed) { + serverPair.second->hash(); } { - const auto position = d.first; - const auto& server = d.second; + const auto position = serverPair.first; + const auto& server = serverPair.second; auto hashes = server->hashes.read_lock(); // we want to keep track of the last hash if (min > *(hashes->begin())) { @@ -231,10 +232,10 @@ std::optional chashedFromHash(const Server return std::nullopt; } -std::optional chashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq) +std::optional chashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion) { const auto hashPerturbation = dnsdist::configuration::getImmutableConfiguration().d_hashPerturbation; - return chashedFromHash(servers, dq->ids.qname.hash(hashPerturbation)); + return chashedFromHash(servers, dnsQuestion->ids.qname.hash(hashPerturbation)); } std::optional roundrobin(const ServerPolicy::NumberedServerVector& servers, [[maybe_unused]] const DNSQuestion* dnsQuestion) @@ -246,9 +247,9 @@ std::optional roundrobin(const ServerPolic vector candidates; candidates.reserve(servers.size()); - for (auto& d : servers) { - if (d.second->isUp()) { - candidates.push_back(d.first); + for (const auto& server : servers) { + if (server.second->isUp()) { + candidates.push_back(server.first); } } @@ -256,8 +257,8 @@ std::optional roundrobin(const ServerPolic if (dnsdist::configuration::getCurrentRuntimeConfiguration().d_roundrobinFailOnNoServer) { return std::nullopt; } - for (auto& d : servers) { - candidates.push_back(d.first); + for (const auto& server : servers) { + candidates.push_back(server.first); } } @@ -265,7 +266,7 @@ std::optional roundrobin(const ServerPolic return candidates.at((counter++) % candidates.size()); } -std::optional orderedWrandUntag(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsq) +std::optional orderedWrandUntag(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion) { if (servers.empty()) { return std::nullopt; @@ -280,13 +281,13 @@ std::optional orderedWrandUntag(const Serv unsigned int curNumber = 1; for (const auto& svr : servers) { - if (svr.second->isUp() && (!dnsq->ids.qTag || dnsq->ids.qTag->count(svr.second->getNameWithAddr()) == 0)) { + if (svr.second->isUp() && (!dnsQuestion->ids.qTag || dnsQuestion->ids.qTag->count(svr.second->getNameWithAddr()) == 0)) { // the servers in a pool are already sorted in ascending order by its 'order', see ``ServerPool::addServer()`` if (svr.second->d_config.order > curOrder) { break; } curOrder = svr.second->d_config.order; - candidates.push_back(ServerPolicy::NumberedServer(curNumber++, svr.second)); + candidates.emplace_back(curNumber++, svr.second); positionsMap.push_back(svr.first); } } @@ -295,7 +296,7 @@ std::optional orderedWrandUntag(const Serv return std::nullopt; } - auto selected = wrandom(candidates, dnsq); + auto selected = wrandom(candidates, dnsQuestion); if (selected) { return positionsMap.at(*selected - 1); } @@ -380,7 +381,7 @@ const ServerPool& getPool(const std::string& poolName) const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; auto poolIt = pools.find(poolName); if (poolIt == pools.end()) { - throw std::out_of_range("No pool named " + poolName); + throw std::out_of_range(std::string("No pool named ") + poolName); } return poolIt->second; @@ -419,7 +420,7 @@ const ServerPolicy::ffipolicyfunc_t& ServerPolicy::getPerThreadPolicy() const return state->d_policies.at(d_name); } -ServerPolicy::SelectedBackend ServerPolicy::getSelectedBackend(const ServerPolicy::NumberedServerVector& servers, DNSQuestion& dq) const +ServerPolicy::SelectedBackend ServerPolicy::getSelectedBackend(const ServerPolicy::NumberedServerVector& servers, DNSQuestion& dnsQuestion) const { ServerPolicy::SelectedBackend result{servers}; @@ -428,7 +429,7 @@ ServerPolicy::SelectedBackend ServerPolicy::getSelectedBackend(const ServerPolic std::optional position; { auto lock = g_lua.lock(); - position = d_policy(servers, &dq); + position = d_policy(servers, &dnsQuestion); } if (position && *position > 0 && *position <= servers.size()) { result.setSelected(*position - 1); @@ -436,7 +437,7 @@ ServerPolicy::SelectedBackend ServerPolicy::getSelectedBackend(const ServerPolic return result; } - dnsdist_ffi_dnsquestion_t dnsq(&dq); + dnsdist_ffi_dnsquestion_t dnsq(&dnsQuestion); dnsdist_ffi_servers_list_t serversList(servers); ServerPolicy::SelectedServerPosition selected = 0; @@ -458,7 +459,7 @@ ServerPolicy::SelectedBackend ServerPolicy::getSelectedBackend(const ServerPolic return result; } - auto position = d_policy(servers, &dq); + auto position = d_policy(servers, &dnsQuestion); if (position && *position > 0 && *position <= servers.size()) { result.setSelected(*position - 1); } diff --git a/pdns/dnsdistdist/dnsdist-lbpolicies.hh b/pdns/dnsdistdist/dnsdist-lbpolicies.hh index 554b4adaa2..5f3a50d9d8 100644 --- a/pdns/dnsdistdist/dnsdist-lbpolicies.hh +++ b/pdns/dnsdistdist/dnsdist-lbpolicies.hh @@ -99,7 +99,7 @@ public: std::optional d_selected; }; - SelectedBackend getSelectedBackend(const ServerPolicy::NumberedServerVector& servers, DNSQuestion& dq) const; + SelectedBackend getSelectedBackend(const ServerPolicy::NumberedServerVector& servers, DNSQuestion& dnsQuestion) const; const std::string& getName() const { @@ -140,15 +140,15 @@ void removeServerFromPool(const std::string& poolName, std::shared_ptr firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); -std::optional leastOutstanding(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); -std::optional wrandom(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); -std::optional whashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); +std::optional firstAvailable(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); +std::optional leastOutstanding(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); +std::optional wrandom(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); +std::optional whashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); std::optional whashedFromHash(const ServerPolicy::NumberedServerVector& servers, size_t hash); -std::optional chashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); +std::optional chashed(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); std::optional chashedFromHash(const ServerPolicy::NumberedServerVector& servers, size_t hash); -std::optional roundrobin(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dq); -std::optional orderedWrandUntag(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsq); +std::optional roundrobin(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); +std::optional orderedWrandUntag(const ServerPolicy::NumberedServerVector& servers, const DNSQuestion* dnsQuestion); #include diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh index 4dc8dfaefc..6b6cc73f0f 100644 --- a/pdns/dnsdistdist/dnsdist.hh +++ b/pdns/dnsdistdist/dnsdist.hh @@ -965,7 +965,7 @@ enum class ProcessQueryResult : uint8_t #include "dnsdist-rule-chains.hh" ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr& selectedBackend); -ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ptr& selectedBackend); +ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ptr& outgoingBackend); bool processResponse(PacketBuffer& response, DNSResponse& dnsResponse, bool muted); bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dnsQuestion, std::string& ruleresult, bool& drop); bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, bool muted);