From: Remi Gacogne Date: Thu, 18 Jul 2024 07:53:13 +0000 (+0200) Subject: dnsdist: Fix/silence Coverity "auto causes copy" warnings X-Git-Tag: rec-5.2.0-alpha1~163^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F14496%2Fhead;p=thirdparty%2Fpdns.git dnsdist: Fix/silence Coverity "auto causes copy" warnings I'm not very happy with what Coverity is doing with this rule: while it does find some actual potential performance improvements, more often than not what it suggests would actually lead to a use-after-free issue. --- diff --git a/pdns/dnsdistdist/dnsdist-carbon.cc b/pdns/dnsdistdist/dnsdist-carbon.cc index ee396d0452..6fa67e55d4 100644 --- a/pdns/dnsdistdist/dnsdist-carbon.cc +++ b/pdns/dnsdistdist/dnsdist-carbon.cc @@ -349,7 +349,7 @@ Carbon::Endpoint Carbon::newEndpoint(const std::string& address, std::string our } return Carbon::Endpoint{ComboAddress(address, 2003), !namespace_name.empty() ? namespace_name : "dnsdist", - ourName, + std::move(ourName), !instance_name.empty() ? instance_name : "main", interval < std::numeric_limits::max() ? static_cast(interval) : 30}; } diff --git a/pdns/dnsdistdist/dnsdist-console.cc b/pdns/dnsdistdist/dnsdist-console.cc index f5ed5c705f..8994c338af 100644 --- a/pdns/dnsdistdist/dnsdist-console.cc +++ b/pdns/dnsdistdist/dnsdist-console.cc @@ -225,7 +225,9 @@ namespace dnsdist::console { void doClient(const std::string& command) { + //coverity[auto_causes_copy] const auto consoleKey = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleKey; + //coverity[auto_causes_copy] const auto server = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleServerAddress; if (!dnsdist::crypto::authenticated::isValidKey(consoleKey)) { cerr << "The currently configured console key is not valid, please configure a valid key using the setKey() directive" << endl; @@ -932,6 +934,7 @@ static void controlClientThread(ConsoleConnection&& conn) setTCPNoDelay(conn.getFD()); + //coverity[auto_causes_copy] const auto consoleKey = dnsdist::configuration::getCurrentRuntimeConfiguration().d_consoleKey; dnsdist::crypto::authenticated::Nonce theirs; dnsdist::crypto::authenticated::Nonce ours; @@ -957,6 +960,7 @@ static void controlClientThread(ConsoleConnection&& conn) } std::string line; + //coverity[tainted_data] line.resize(len); readn2(conn.getFD(), line.data(), len); diff --git a/pdns/dnsdistdist/dnsdist-lua-rules.cc b/pdns/dnsdistdist/dnsdist-lua-rules.cc index 6343dba706..10937c03fc 100644 --- a/pdns/dnsdistdist/dnsdist-lua-rules.cc +++ b/pdns/dnsdistdist/dnsdist-lua-rules.cc @@ -211,6 +211,7 @@ static void moveRuleToTop(IdentifierTypeT chainIdentifier) if (rules.empty()) { return; } + //coverity[auto_causes_copy] auto subject = *rules.rbegin(); rules.erase(std::prev(rules.end())); rules.insert(rules.begin(), subject); @@ -227,6 +228,7 @@ static void mvRule(IdentifierTypeT chainIdentifier, unsigned int from, unsigned g_outputBuffer = "Error: attempt to move rules from/to invalid index\n"; return; } + //coverity[auto_causes_copy] auto subject = rules[from]; rules.erase(rules.begin() + from); if (destination > rules.size()) { diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index e75227d01a..bc3d3df0ec 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -1244,6 +1244,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) }); luaCtx.writeFunction("getPoolServers", [](const string& pool) { + //coverity[auto_causes_copy] const auto poolServers = getDownstreamCandidates(pool); return *poolServers; }); @@ -1852,7 +1853,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) // 1 2 3 4 ret << (fmt % "Name" % "Cache" % "ServerPolicy" % "Servers") << endl; + //coverity[auto_causes_copy] const auto defaultPolicyName = dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName(); + //coverity[auto_causes_copy] const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; for (const auto& entry : pools) { const string& name = entry.first; @@ -1890,7 +1893,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) setLuaNoSideEffect(); LuaArray ret; int count = 1; - const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; + const auto& pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; for (const auto& entry : pools) { const string& name = entry.first; ret.emplace_back(count++, name); diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 7790e755f0..acf1d31f89 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -1238,7 +1238,7 @@ static void handleStats(const YaHTTP::Request& req, YaHTTP::Response& resp) Json::array pools; { num = 0; - const auto localPools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; + const auto& localPools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; pools.reserve(localPools.size()); for (const auto& pool : localPools) { const auto& cache = pool.second->packetCache; diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index 41e8c1ca6d..5c712f268c 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -1497,6 +1497,7 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ ++dnsdist::metrics::g_stats.cacheMisses; + //coverity[auto_causes_copy] const auto existingPool = dnsQuestion.ids.poolName; const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains; const auto& cacheMissRuleActions = dnsdist::rules::getRuleChain(chains, dnsdist::rules::RuleChain::CacheMissRules); @@ -2423,7 +2424,7 @@ static void checkFileDescriptorsLimits(size_t udpBindsCount, size_t tcpBindsCoun const auto& immutableConfig = dnsdist::configuration::getImmutableConfiguration(); /* stdin, stdout, stderr */ rlim_t requiredFDsCount = 3; - const auto backends = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends; + const auto& backends = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends; /* UDP sockets to backends */ size_t backendUDPSocketsCount = 0; for (const auto& backend : backends) { @@ -3484,6 +3485,7 @@ int main(int argc, char** argv) checkFileDescriptorsLimits(udpBindsCount, tcpBindsCount); { + //coverity[auto_causes_copy] const auto states = dnsdist::configuration::getCurrentRuntimeConfiguration().d_backends; // it is a copy, but the internal shared_ptrs are the real deal auto mplexer = std::unique_ptr(FDMultiplexer::getMultiplexerSilent(states.size())); for (auto& dss : states) {