From 8f291e7bc1e42690ae8036ef625c6f7e38419a14 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Wed, 9 Jul 2025 15:04:42 +0200 Subject: [PATCH] rec: asssorted tidy Signed-off-by: Otto Moerbeek --- pdns/recursordist/logging.cc | 2 +- pdns/recursordist/lua-recursor4.cc | 14 +- pdns/recursordist/lua-recursor4.hh | 4 - pdns/recursordist/mtasker_context.cc | 4 +- pdns/recursordist/negcache.cc | 77 ++-- pdns/recursordist/negcache.hh | 33 +- pdns/recursordist/nod.cc | 2 - pdns/recursordist/nod.hh | 3 +- pdns/recursordist/ratelimitedlog.hh | 1 - pdns/recursordist/rec-main.cc | 2 +- pdns/recursordist/rec_channel.cc | 166 ++++---- pdns/recursordist/rec_channel.hh | 27 +- pdns/recursordist/rec_channel_rec.cc | 594 ++++++++++++++------------- pdns/recursordist/rec_control.cc | 50 +-- 14 files changed, 518 insertions(+), 461 deletions(-) diff --git a/pdns/recursordist/logging.cc b/pdns/recursordist/logging.cc index af188f45ef..b6aaef6624 100644 --- a/pdns/recursordist/logging.cc +++ b/pdns/recursordist/logging.cc @@ -50,7 +50,7 @@ void Logger::info(Logr::Priority prio, const std::string& msg) const void Logger::logMessage(const std::string& msg, boost::optional err) const { - return logMessage(msg, Logr::Absent, std::move(err)); + logMessage(msg, Logr::Absent, std::move(err)); } void Logger::logMessage(const std::string& msg, Logr::Priority prio, boost::optional err) const diff --git a/pdns/recursordist/lua-recursor4.cc b/pdns/recursordist/lua-recursor4.cc index 8a278bb524..9552dc2f78 100644 --- a/pdns/recursordist/lua-recursor4.cc +++ b/pdns/recursordist/lua-recursor4.cc @@ -380,29 +380,29 @@ void RecursorLua4::postPrepareContext() // NOLINT(readability-function-cognitive d_lw->registerFunction("check",(bool (SuffixMatchNode::*)(const DNSName&) const) &SuffixMatchNode::check); d_lw->registerFunction("toString",(string (SuffixMatchNode::*)() const) &SuffixMatchNode::toString); - d_pd.push_back({"policykinds", in_t { + d_pd.emplace_back("policykinds", in_t { {"NoAction", (int)DNSFilterEngine::PolicyKind::NoAction}, {"Drop", (int)DNSFilterEngine::PolicyKind::Drop }, {"NXDOMAIN", (int)DNSFilterEngine::PolicyKind::NXDOMAIN}, {"NODATA", (int)DNSFilterEngine::PolicyKind::NODATA }, {"Truncate", (int)DNSFilterEngine::PolicyKind::Truncate}, {"Custom", (int)DNSFilterEngine::PolicyKind::Custom } - }}); + }); - d_pd.push_back({"policytypes", in_t { + d_pd.emplace_back("policytypes", in_t { {"None", (int)DNSFilterEngine::PolicyType::None }, {"QName", (int)DNSFilterEngine::PolicyType::QName }, {"ClientIP", (int)DNSFilterEngine::PolicyType::ClientIP }, {"ResponseIP", (int)DNSFilterEngine::PolicyType::ResponseIP }, {"NSDName", (int)DNSFilterEngine::PolicyType::NSDName }, {"NSIP", (int)DNSFilterEngine::PolicyType::NSIP } - }}); + }); for(const auto& name : QType::names) { d_pd.emplace_back(name.first, name.second); } - d_pd.push_back({"validationstates", in_t{ + d_pd.emplace_back("validationstates", in_t{ {"Indeterminate", static_cast(vState::Indeterminate) }, {"BogusNoValidDNSKEY", static_cast(vState::BogusNoValidDNSKEY) }, {"BogusInvalidDenial", static_cast(vState::BogusInvalidDenial) }, @@ -423,7 +423,7 @@ void RecursorLua4::postPrepareContext() // NOLINT(readability-function-cognitive {"Secure", static_cast(vState::Secure) }, /* in order not to break compatibility with older scripts: */ {"Bogus", static_cast(255) }, - }}); + }); d_lw->writeFunction("isValidationStateBogus", [](vState state) { return vStateIsBogus(state); @@ -1356,5 +1356,5 @@ const char* pdns_postresolve_ffi_handle_get_authip(pdns_postresolve_ffi_handle_t void pdns_postresolve_ffi_handle_get_authip_raw(pdns_postresolve_ffi_handle_t* ref, const void** addr, size_t* addrSize) { - return pdns_ffi_comboaddress_to_raw(*ref->getHandle().d_dq.fromAuthIP, addr, addrSize); + pdns_ffi_comboaddress_to_raw(*ref->getHandle().d_dq.fromAuthIP, addr, addrSize); } diff --git a/pdns/recursordist/lua-recursor4.hh b/pdns/recursordist/lua-recursor4.hh index 0d3874f07d..a460fe47b1 100644 --- a/pdns/recursordist/lua-recursor4.hh +++ b/pdns/recursordist/lua-recursor4.hh @@ -21,20 +21,16 @@ */ #pragma once -#ifdef HAVE_CONFIG_H #include "config.h" -#endif #include "iputils.hh" #include "dnsname.hh" #include "namespaces.hh" -#include "dnsrecords.hh" #include "filterpo.hh" #include "ednsoptions.hh" #include "validate.hh" #include "lua-base4.hh" #include "proxy-protocol.hh" -#include "noinitvector.hh" #include "rec-eventtrace.hh" #include diff --git a/pdns/recursordist/mtasker_context.cc b/pdns/recursordist/mtasker_context.cc index ed0459a0f7..976b8a3c66 100644 --- a/pdns/recursordist/mtasker_context.cc +++ b/pdns/recursordist/mtasker_context.cc @@ -95,7 +95,7 @@ using boost::context::detail::jump_fcontext; using boost::context::detail::transfer_t; #endif /* BOOST_VERSION < 106100 */ -static_assert(std::is_pointer::value, +static_assert(std::is_pointer_v, "Boost Context has changed the fcontext_t type again :-("); #endif @@ -224,7 +224,7 @@ void pdns_swapcontext(pdns_ucontext_t& __restrict octx, pdns_ucontext_t const& _ if (origctx && origctx->exception) std::rethrow_exception(origctx->exception); #else - transfer_t res = jump_fcontext(static_cast(ctx.uc_mcontext), &octx.uc_mcontext); + transfer_t res = jump_fcontext(static_cast(ctx.uc_mcontext), static_cast(&octx.uc_mcontext)); CET_ENDBR; if (res.data != nullptr) { /* if res.data is not a nullptr, it holds a pointer to the context diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc index cb7f7e3cc2..7d4f45f532 100644 --- a/pdns/recursordist/negcache.cc +++ b/pdns/recursordist/negcache.cc @@ -24,7 +24,6 @@ #include "negcache.hh" #include "misc.hh" #include "cachecleaner.hh" -#include "utility.hh" #include "rec-taskqueue.hh" // For a description on how ServeStale works, see recursor_cache.cc, the general structure is the same. @@ -78,7 +77,7 @@ void NegCache::updateStaleEntry(time_t now, negcache_t::iterator& entry, QType q // We we look how old the entry is, and increase d_servedStale accordingly, taking care not to overflow const time_t howlong = std::max(static_cast(1), now - entry->d_ttd); const uint32_t extension = std::max(1U, std::min(entry->d_orig_ttl, s_serveStaleExtensionPeriod)); - entry->d_servedStale = std::min(entry->d_servedStale + 1 + howlong / extension, static_cast(s_maxServedStaleExtensions)); + entry->d_servedStale = std::min(entry->d_servedStale + 1 + (howlong / extension), static_cast(s_maxServedStaleExtensions)); entry->d_ttd = now + std::min(entry->d_orig_ttl, s_serveStaleExtensionPeriod); if (qtype == QType::ENT) { @@ -97,7 +96,7 @@ void NegCache::updateStaleEntry(time_t now, negcache_t::iterator& entry, QType q * \param ne A NegCacheEntry that is filled when there is a cache entry * \return true if ne was filled out, false otherwise */ -bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch, bool serveStale, bool refresh) +bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now, NegCacheEntry& negEntry, bool typeMustMatch, bool serveStale, bool refresh) { auto& map = getMap(qname); auto content = map.lock(); @@ -124,10 +123,10 @@ bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now, } if (now.tv_sec < ni->d_ttd) { // Not expired - ne = *ni; + negEntry = *ni; moveCacheItemToBack(content->d_map, firstIndexIterator); // when refreshing, we consider served-stale entries outdated - return !(refresh && ni->d_servedStale > 0); + return !refresh || ni->d_servedStale <= 0; } } } @@ -139,12 +138,12 @@ bool NegCache::get(const DNSName& qname, QType qtype, const struct timeval& now, * * \param ne The NegCacheEntry to add to the cache */ -void NegCache::add(const NegCacheEntry& ne) +void NegCache::add(const NegCacheEntry& negEntry) { bool inserted = false; - auto& map = getMap(ne.d_name); + auto& map = getMap(negEntry.d_name); auto content = map.lock(); - inserted = lruReplacingInsert(content->d_map, ne); + inserted = lruReplacingInsert(content->d_map, negEntry); if (inserted) { map.incEntriesCount(); } @@ -159,8 +158,7 @@ void NegCache::add(const NegCacheEntry& ne) */ void NegCache::updateValidationStatus(const DNSName& qname, const QType qtype, const vState newState, boost::optional capTTD) { - auto& mc = getMap(qname); - auto map = mc.lock(); + auto map = getMap(qname).lock(); auto range = map->d_map.equal_range(std::tie(qname, qtype)); if (range.first != range.second) { @@ -208,11 +206,12 @@ size_t NegCache::wipe(const DNSName& name, bool subtree) size_t ret = 0; if (subtree) { for (auto& map : d_maps) { - auto m = map.lock(); - for (auto i = m->d_map.lower_bound(std::tie(name)); i != m->d_map.end();) { - if (!i->d_name.isPartOf(name)) + auto lockedMap = map.lock(); + for (auto i = lockedMap->d_map.lower_bound(std::tie(name)); i != lockedMap->d_map.end();) { + if (!i->d_name.isPartOf(name)) { break; - i = m->d_map.erase(i); + } + i = lockedMap->d_map.erase(i); ret++; map.decEntriesCount(); } @@ -223,9 +222,9 @@ size_t NegCache::wipe(const DNSName& name, bool subtree) auto& map = getMap(name); auto content = map.lock(); auto range = content->d_map.equal_range(std::tie(name)); - auto i = range.first; - while (i != range.second) { - i = content->d_map.erase(i); + auto iter = range.first; + while (iter != range.second) { + iter = content->d_map.erase(iter); ret++; map.decEntriesCount(); } @@ -238,15 +237,15 @@ size_t NegCache::wipeTyped(const DNSName& qname, QType qtype) auto& map = getMap(qname); auto content = map.lock(); auto range = content->d_map.equal_range(std::tie(qname)); - auto i = range.first; - while (i != range.second) { - if (i->d_qtype == QType::ENT || i->d_qtype == qtype) { - i = content->d_map.erase(i); + auto iter = range.first; + while (iter != range.second) { + if (iter->d_qtype == QType::ENT || iter->d_qtype == qtype) { + iter = content->d_map.erase(iter); ++ret; map.decEntriesCount(); } else { - ++i; + ++iter; } } return ret; @@ -258,8 +257,8 @@ size_t NegCache::wipeTyped(const DNSName& qname, QType qtype) void NegCache::clear() { for (auto& map : d_maps) { - auto m = map.lock(); - m->d_map.clear(); + auto lockedMap = map.lock(); + lockedMap->d_map.clear(); map.clearEntriesCount(); } } @@ -280,9 +279,9 @@ void NegCache::prune(time_t now, size_t maxEntries) * * \param fd A pointer to an open FILE object */ -size_t NegCache::doDump(int fd, size_t maxCacheEntries, time_t now) +size_t NegCache::doDump(int fileDesc, size_t maxCacheEntries, time_t now) { - int newfd = dup(fd); + int newfd = dup(fileDesc); if (newfd == -1) { return 0; } @@ -298,28 +297,28 @@ size_t NegCache::doDump(int fd, size_t maxCacheEntries, time_t now) size_t shard = 0; size_t min = std::numeric_limits::max(); size_t max = 0; - for (auto& mc : d_maps) { - auto m = mc.lock(); - const auto shardSize = m->d_map.size(); + for (auto& map : d_maps) { + auto lockedMap = map.lock(); + const auto shardSize = lockedMap->d_map.size(); fprintf(filePtr.get(), "; negcache shard %zu; size %zu\n", shard, shardSize); min = std::min(min, shardSize); max = std::max(max, shardSize); shard++; - auto& sidx = m->d_map.get(); - for (const NegCacheEntry& ne : sidx) { + auto& sidx = lockedMap->d_map.get(); + for (const NegCacheEntry& negEntry : sidx) { ret++; - int64_t ttl = ne.d_ttd - now; - fprintf(filePtr.get(), "%s %" PRId64 " IN %s VIA %s ; (%s) origttl=%" PRIu32 " ss=%hu\n", ne.d_name.toString().c_str(), ttl, ne.d_qtype.toString().c_str(), ne.d_auth.toString().c_str(), vStateToString(ne.d_validationState).c_str(), ne.d_orig_ttl, ne.d_servedStale); - for (const auto& rec : ne.authoritySOA.records) { - fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s ; (%s)\n", rec.d_name.toString().c_str(), ttl, DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.getContent()->getZoneRepresentation().c_str(), vStateToString(ne.d_validationState).c_str()); + int64_t ttl = negEntry.d_ttd - now; + fprintf(filePtr.get(), "%s %" PRId64 " IN %s VIA %s ; (%s) origttl=%" PRIu32 " ss=%hu\n", negEntry.d_name.toString().c_str(), ttl, negEntry.d_qtype.toString().c_str(), negEntry.d_auth.toString().c_str(), vStateToString(negEntry.d_validationState).c_str(), negEntry.d_orig_ttl, negEntry.d_servedStale); + for (const auto& rec : negEntry.authoritySOA.records) { + fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s ; (%s)\n", rec.d_name.toString().c_str(), ttl, DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.getContent()->getZoneRepresentation().c_str(), vStateToString(negEntry.d_validationState).c_str()); } - for (const auto& sig : ne.authoritySOA.signatures) { + for (const auto& sig : negEntry.authoritySOA.signatures) { fprintf(filePtr.get(), "%s %" PRId64 " IN RRSIG %s ;\n", sig.d_name.toString().c_str(), ttl, sig.getContent()->getZoneRepresentation().c_str()); } - for (const auto& rec : ne.DNSSECRecords.records) { - fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s ; (%s)\n", rec.d_name.toString().c_str(), ttl, DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.getContent()->getZoneRepresentation().c_str(), vStateToString(ne.d_validationState).c_str()); + for (const auto& rec : negEntry.DNSSECRecords.records) { + fprintf(filePtr.get(), "%s %" PRId64 " IN %s %s ; (%s)\n", rec.d_name.toString().c_str(), ttl, DNSRecordContent::NumberToType(rec.d_type).c_str(), rec.getContent()->getZoneRepresentation().c_str(), vStateToString(negEntry.d_validationState).c_str()); } - for (const auto& sig : ne.DNSSECRecords.signatures) { + for (const auto& sig : negEntry.DNSSECRecords.signatures) { fprintf(filePtr.get(), "%s %" PRId64 " IN RRSIG %s ;\n", sig.d_name.toString().c_str(), ttl, sig.getContent()->getZoneRepresentation().c_str()); } } diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh index d0d87ee400..7c78b37091 100644 --- a/pdns/recursordist/negcache.hh +++ b/pdns/recursordist/negcache.hh @@ -21,7 +21,6 @@ */ #pragma once -#include #include #include #include @@ -45,11 +44,11 @@ using namespace ::boost::multi_index; * * typedef vector recordsAndSignatures; */ -typedef struct +struct recordsAndSignatures { vector records; vector signatures; -} recordsAndSignatures; +}; class NegCache : public boost::noncopyable { @@ -80,9 +79,7 @@ public: if (s_maxServedStaleExtensions > 0) { return d_ttd + static_cast(s_maxServedStaleExtensions) * std::min(s_serveStaleExtensionPeriod, d_orig_ttl) < now; } - else { - return d_ttd < now; - } + return d_ttd < now; }; bool isEntryUsable(time_t now, bool serveStale) const @@ -92,18 +89,18 @@ public: } }; - void add(const NegCacheEntry& ne); + void add(const NegCacheEntry& negEntry); void updateValidationStatus(const DNSName& qname, QType qtype, vState newState, boost::optional capTTD); - bool get(const DNSName& qname, QType qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch = false, bool serverStale = false, bool refresh = false); + bool get(const DNSName& qname, QType qtype, const struct timeval& now, NegCacheEntry& negEntry, bool typeMustMatch = false, bool serveStale = false, bool refresh = false); bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& negEntry, bool serveStale, bool refresh); size_t count(const DNSName& qname); size_t count(const DNSName& qname, QType qtype); void prune(time_t now, size_t maxEntries); void clear(); - size_t doDump(int fd, size_t maxCacheEntries, time_t now = time(nullptr)); + size_t doDump(int fileDesc, size_t maxCacheEntries, time_t now = time(nullptr)); size_t wipe(const DNSName& name, bool subtree = false); size_t wipeTyped(const DNSName& name, QType qtype); - size_t size() const; + [[nodiscard]] size_t size() const; private: struct CompositeKey @@ -112,7 +109,7 @@ private: struct SequenceTag { }; - typedef boost::multi_index_container< + using negcache_t = boost::multi_index_container< NegCacheEntry, indexed_by< ordered_unique, @@ -121,19 +118,21 @@ private: member, member>, composite_key_compare< - CanonDNSNameCompare, std::less>>, + CanonDNSNameCompare, std::less<>>>, sequenced>, hashed_non_unique, - member>>> - negcache_t; + member>>>; - void updateStaleEntry(time_t now, negcache_t::iterator& entry, QType qtype); + static void updateStaleEntry(time_t now, negcache_t::iterator& entry, QType qtype); struct MapCombo { - MapCombo() {} + MapCombo() = default; MapCombo(const MapCombo&) = delete; MapCombo& operator=(const MapCombo&) = delete; + ~MapCombo() = default; + MapCombo(MapCombo&&) = delete; + MapCombo& operator=(MapCombo&&) = delete; struct LockedContent { negcache_t d_map; @@ -185,7 +184,7 @@ private: { return d_maps.at(qname.hash() % d_maps.size()); } - const MapCombo& getMap(const DNSName& qname) const + [[nodiscard]] const MapCombo& getMap(const DNSName& qname) const { return d_maps.at(qname.hash() % d_maps.size()); } diff --git a/pdns/recursordist/nod.cc b/pdns/recursordist/nod.cc index 6d0f01c44a..b1c12fc441 100644 --- a/pdns/recursordist/nod.cc +++ b/pdns/recursordist/nod.cc @@ -24,12 +24,10 @@ #include #include "pdnsexception.hh" #include -#include #include #include #include "threadname.hh" #include -#include "logger.hh" #include "logging.hh" #include "misc.hh" diff --git a/pdns/recursordist/nod.hh b/pdns/recursordist/nod.hh index fbefbb9aca..daa355b815 100644 --- a/pdns/recursordist/nod.hh +++ b/pdns/recursordist/nod.hh @@ -20,10 +20,11 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ #pragma once -#include + #include #include #include + #include "dnsname.hh" #include "lock.hh" #include "stable-bloom.hh" diff --git a/pdns/recursordist/ratelimitedlog.hh b/pdns/recursordist/ratelimitedlog.hh index a252bed557..568cbda663 100644 --- a/pdns/recursordist/ratelimitedlog.hh +++ b/pdns/recursordist/ratelimitedlog.hh @@ -22,7 +22,6 @@ #pragma once #include "lock.hh" -#include "logger.hh" #include "logging.hh" namespace pdns diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 2f4540b20f..457005e683 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -2993,7 +2993,7 @@ static void recursorThread() } if (threadInfo.isHandler()) { - t_fdm->addReadFD(g_rcc.d_fd, handleRCC); // control channel + t_fdm->addReadFD(g_rcc.getDescriptor(), handleRCC); // control channel } #ifdef HAVE_SYSTEMD diff --git a/pdns/recursordist/rec_channel.cc b/pdns/recursordist/rec_channel.cc index 47a1832d0c..3157dbd93c 100644 --- a/pdns/recursordist/rec_channel.cc +++ b/pdns/recursordist/rec_channel.cc @@ -1,22 +1,39 @@ -#ifdef HAVE_CONFIG_H +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + #include "config.h" -#endif + #include "rec_channel.hh" -#include "utility.hh" + #include #include -#include "misc.hh" -#include #include #include #include #include -#include -#include - -#include "pdnsexception.hh" +#include "misc.hh" #include "namespaces.hh" +#include "pdnsexception.hh" /* g++ defines __SANITIZE_THREAD__ clang++ supports the nice __has_feature(thread_sanitizer), @@ -32,71 +49,80 @@ std::atomic RecursorControlChannel::stop = false; -RecursorControlChannel::RecursorControlChannel() +RecursorControlChannel::RecursorControlChannel() : + d_fd(-1) { - d_fd = -1; - *d_local.sun_path = 0; - d_local.sun_family = 0; + memset(&d_local, 0, sizeof(d_local)); } RecursorControlChannel::~RecursorControlChannel() { - if (d_fd > 0) + if (d_fd > 0) { close(d_fd); - if (*d_local.sun_path) - unlink(d_local.sun_path); + } + if (d_local.sun_path[0] != '\0') { + unlink(d_local.sun_path); // NOLINT + } } -int RecursorControlChannel::listen(const string& fname) +int RecursorControlChannel::listen(const string& filename) { d_fd = socket(AF_UNIX, SOCK_STREAM, 0); - setCloseOnExec(d_fd); - if (d_fd < 0) + if (d_fd < 0) { throw PDNSException("Creating UNIX domain socket: " + stringerror()); + } + setCloseOnExec(d_fd); int tmp = 1; - if (setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR, (char*)&tmp, sizeof tmp) < 0) + if (setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR, &tmp, sizeof tmp) < 0) { throw PDNSException("Setsockopt failed: " + stringerror()); + } - int err = unlink(fname.c_str()); - if (err < 0 && errno != ENOENT) - throw PDNSException("Can't remove (previous) controlsocket '" + fname + "': " + stringerror() + " (try --socket-dir)"); + int err = unlink(filename.c_str()); + if (err < 0 && errno != ENOENT) { + throw PDNSException("Can't remove (previous) controlsocket '" + filename + "': " + stringerror() + " (try --socket-dir)"); + } - if (makeUNsockaddr(fname, &d_local)) - throw PDNSException("Unable to bind to controlsocket, path '" + fname + "' is not a valid UNIX socket path."); + if (makeUNsockaddr(filename, &d_local) != 0) { + throw PDNSException("Unable to bind to controlsocket, path '" + filename + "' is not a valid UNIX socket path."); + } - if (bind(d_fd, (sockaddr*)&d_local, sizeof(d_local)) < 0) - throw PDNSException("Unable to bind to controlsocket '" + fname + "': " + stringerror()); + if (bind(d_fd, reinterpret_cast(&d_local), sizeof(d_local)) < 0) { // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + throw PDNSException("Unable to bind to controlsocket '" + filename + "': " + stringerror()); + } if (::listen(d_fd, 0) == -1) { - throw PDNSException("Unable to listen on controlsocket '" + fname + "': " + stringerror()); + throw PDNSException("Unable to listen on controlsocket '" + filename + "': " + stringerror()); } return d_fd; } -void RecursorControlChannel::connect(const string& path, const string& fname) +void RecursorControlChannel::connect(const string& path, const string& filename) { - struct sockaddr_un remote; + struct sockaddr_un remote{}; d_fd = socket(AF_UNIX, SOCK_STREAM, 0); setCloseOnExec(d_fd); - if (d_fd < 0) + if (d_fd < 0) { throw PDNSException("Creating UNIX domain socket: " + stringerror()); - + } try { int tmp = 1; - if (setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR, (char*)&tmp, sizeof tmp) < 0) + if (setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR, &tmp, sizeof tmp) < 0) { throw PDNSException("Setsockopt failed: " + stringerror()); + } - string remotename = path + "/" + fname; - if (makeUNsockaddr(remotename, &remote)) + string remotename = path + "/" + filename; + if (makeUNsockaddr(remotename, &remote) != 0) { throw PDNSException("Unable to connect to controlsocket, path '" + remotename + "' is not a valid UNIX socket path."); + } - if (::connect(d_fd, (sockaddr*)&remote, sizeof(remote)) < 0) { - if (*d_local.sun_path) - unlink(d_local.sun_path); - throw PDNSException("Unable to connect to remote '" + string(remote.sun_path) + "': " + stringerror()); + if (::connect(d_fd, reinterpret_cast(&remote), sizeof(remote)) < 0) { // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + if (d_local.sun_path[0] != '\0') { + unlink(d_local.sun_path); // NOLINT + } + throw PDNSException("Unable to connect to remote '" + remotename + "': " + stringerror()); } } catch (...) { @@ -107,72 +133,72 @@ void RecursorControlChannel::connect(const string& path, const string& fname) } } -static void sendfd(int s, int fd) +static void sendfd(int socket, int fd_to_pass) { - struct msghdr msg; - struct cmsghdr* cmsg; + struct msghdr msg{}; + struct cmsghdr* cmsg{}; union { struct cmsghdr hdr; - unsigned char buf[CMSG_SPACE(sizeof(int))]; - } cmsgbuf; - struct iovec io_vector[1]; - char ch = 'X'; + std::array buf; + } cmsgbuf{}; + std::array io_vector{}; + char character = 'X'; - io_vector[0].iov_base = &ch; + io_vector[0].iov_base = &character; io_vector[0].iov_len = 1; memset(&msg, 0, sizeof(msg)); msg.msg_control = &cmsgbuf.buf; msg.msg_controllen = sizeof(cmsgbuf.buf); - msg.msg_iov = io_vector; - msg.msg_iovlen = 1; + msg.msg_iov = io_vector.data(); + msg.msg_iovlen = io_vector.size(); cmsg = CMSG_FIRSTHDR(&msg); cmsg->cmsg_len = CMSG_LEN(sizeof(int)); cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; - *(int*)CMSG_DATA(cmsg) = fd; + *reinterpret_cast(CMSG_DATA(cmsg)) = fd_to_pass; // NOLINT(cppcoreguidelines-pro-reinterpret-cast> - if (sendmsg(s, &msg, 0) == -1) { + if (sendmsg(socket, &msg, 0) == -1) { throw PDNSException("Unable to send fd message over control channel: " + stringerror()); } } -void RecursorControlChannel::send(int fd, const Answer& msg, unsigned int timeout, int fd_to_pass) +void RecursorControlChannel::send(int fileDesc, const Answer& msg, unsigned int timeout, int fd_to_pass) { - int ret = waitForRWData(fd, false, timeout, 0); + int ret = waitForRWData(fileDesc, false, static_cast(timeout), 0); if (ret == 0) { throw PDNSException("Timeout sending message over control channel"); } - else if (ret < 0) { + if (ret < 0) { throw PDNSException("Error sending message over control channel:" + stringerror()); } - if (::send(fd, &msg.d_ret, sizeof(msg.d_ret), 0) < 0) { + if (::send(fileDesc, &msg.d_ret, sizeof(msg.d_ret), 0) < 0) { throw PDNSException("Unable to send return code over control channel: " + stringerror()); } size_t len = msg.d_str.length(); - if (::send(fd, &len, sizeof(len), 0) < 0) { + if (::send(fileDesc, &len, sizeof(len), 0) < 0) { throw PDNSException("Unable to send length over control channel: " + stringerror()); } - if (::send(fd, msg.d_str.c_str(), len, 0) != static_cast(len)) { + if (::send(fileDesc, msg.d_str.c_str(), len, 0) != static_cast(len)) { throw PDNSException("Unable to send message over control channel: " + stringerror()); } if (fd_to_pass != -1) { - sendfd(fd, fd_to_pass); + sendfd(fileDesc, fd_to_pass); } } -static void waitForRead(int fd, unsigned int timeout, time_t start) +static void waitForRead(int fileDesc, unsigned int timeout, time_t start) { time_t elapsed = time(nullptr) - start; if (elapsed >= timeout) { throw PDNSException("Timeout waiting for control channel data"); } // coverity[store_truncates_time_t] - int ret = waitForData(fd, timeout - elapsed, 0); + int ret = waitForData(fileDesc, static_cast(timeout - static_cast(elapsed)), 0); if (ret == 0) { throw PDNSException("Timeout waiting for control channel data"); } @@ -194,14 +220,14 @@ static size_t getArgMax() return 4096; } -RecursorControlChannel::Answer RecursorControlChannel::recv(int fd, unsigned int timeout) +RecursorControlChannel::Answer RecursorControlChannel::recv(int fileDesc, unsigned int timeout) { // timeout covers the operation of all read ops combined const time_t start = time(nullptr); - waitForRead(fd, timeout, start); + waitForRead(fileDesc, timeout, start); int err{}; - auto ret = ::recv(fd, &err, sizeof(err), 0); + auto ret = ::recv(fileDesc, &err, sizeof(err), 0); if (ret == 0) { #if defined(__SANITIZE_THREAD__) return {0, "bye nicely\n"}; // Hack because TSAN enabled build justs _exits on quit-nicely @@ -212,9 +238,9 @@ RecursorControlChannel::Answer RecursorControlChannel::recv(int fd, unsigned int throw PDNSException("Unable to receive return status over control channel: " + stringerror()); } - waitForRead(fd, timeout, start); - size_t len; - if (::recv(fd, &len, sizeof(len), 0) != sizeof(len)) { + waitForRead(fileDesc, timeout, start); + size_t len{}; + if (::recv(fileDesc, &len, sizeof(len), 0) != sizeof(len)) { throw PDNSException("Unable to receive length over control channel: " + stringerror()); } @@ -225,15 +251,15 @@ RecursorControlChannel::Answer RecursorControlChannel::recv(int fd, unsigned int string str; str.reserve(len); while (str.length() < len) { - char buffer[1024]; - waitForRead(fd, timeout, start); + std::array buffer{}; + waitForRead(fileDesc, timeout, start); size_t toRead = std::min(len - str.length(), sizeof(buffer)); - ssize_t recvd = ::recv(fd, buffer, toRead, 0); + ssize_t recvd = ::recv(fileDesc, buffer.data(), toRead, 0); if (recvd <= 0) { // EOF means we have a length error throw PDNSException("Unable to receive message over control channel: " + stringerror()); } - str.append(buffer, recvd); + str.append(buffer.data(), recvd); } return {err, std::move(str)}; diff --git a/pdns/recursordist/rec_channel.hh b/pdns/recursordist/rec_channel.hh index f86092107c..061683b67b 100644 --- a/pdns/recursordist/rec_channel.hh +++ b/pdns/recursordist/rec_channel.hh @@ -26,9 +26,7 @@ #include #include #include -#include #include -#include #include #include "iputils.hh" #include "dnsname.hh" @@ -44,6 +42,10 @@ class RecursorControlChannel public: RecursorControlChannel(); + RecursorControlChannel(const RecursorControlChannel&) = delete; + RecursorControlChannel(RecursorControlChannel&&) = delete; + RecursorControlChannel& operator=(const RecursorControlChannel&) = delete; + RecursorControlChannel& operator=(RecursorControlChannel&&) = delete; ~RecursorControlChannel(); int listen(const std::string& filename); @@ -65,14 +67,19 @@ public: std::string d_str; }; - void send(int remote, const Answer&, unsigned int timeout = 5, int fd_to_pass = -1); - RecursorControlChannel::Answer recv(int fd, unsigned int timeout = 5); + static void send(int fileDesc, const Answer&, unsigned int timeout = 5, int fd_to_pass = -1); + static RecursorControlChannel::Answer recv(int fileDesc, unsigned int timeout = 5); - int d_fd; static std::atomic stop; + [[nodiscard]] int getDescriptor() const + { + return d_fd; + } + private: - struct sockaddr_un d_local; + int d_fd; + struct sockaddr_un d_local{}; }; class RecursorControlParser @@ -85,7 +92,7 @@ public: static RecursorControlChannel::Answer getAnswer(int socket, const std::string& question, func_t** command); }; -enum class StatComponent +enum class StatComponent : uint8_t { API, Carbon, @@ -102,13 +109,13 @@ struct StatsMapEntry class PrefixDashNumberCompare { private: - static std::pair prefixAndTrailingNum(const std::string& a); + static std::pair prefixAndTrailingNum(const std::string& arg); public: - bool operator()(const std::string& a, const std::string& b) const; + bool operator()(const std::string& lhs, const std::string& rhs) const; }; -typedef std::map StatsMap; +using StatsMap = std::map; StatsMap getAllStatsMap(StatComponent component); diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc index 6d4f44b11f..c3274a0fee 100644 --- a/pdns/recursordist/rec_channel_rec.cc +++ b/pdns/recursordist/rec_channel_rec.cc @@ -1,6 +1,27 @@ -#ifdef HAVE_CONFIG_H +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + #include "config.h" -#endif + #include "utility.hh" #include "rec_channel.hh" @@ -58,36 +79,34 @@ #include #endif -std::pair PrefixDashNumberCompare::prefixAndTrailingNum(const std::string& a) +std::pair PrefixDashNumberCompare::prefixAndTrailingNum(const std::string& arg) { - auto i = a.length(); - if (i == 0) { - return {a, ""}; + auto length = arg.length(); + if (length == 0) { + return {arg, ""}; } - --i; - if (!std::isdigit(a[i])) { - return {a, ""}; + --length; + if (std::isdigit(arg[length]) == 0) { + return {arg, ""}; } - while (i > 0) { - if (!std::isdigit(a[i])) { + while (length > 0) { + if (std::isdigit(arg[length]) == 0) { break; } - --i; + --length; } - return {a.substr(0, i + 1), a.substr(i + 1, a.size() - i - 1)}; + return {arg.substr(0, length + 1), arg.substr(length + 1, arg.size() - length - 1)}; } -bool PrefixDashNumberCompare::operator()(const std::string& a, const std::string& b) const +bool PrefixDashNumberCompare::operator()(const std::string& lhs, const std::string& rhs) const { - auto [aprefix, anum] = prefixAndTrailingNum(a); - auto [bprefix, bnum] = prefixAndTrailingNum(b); + auto [aprefix, anum] = prefixAndTrailingNum(lhs); + auto [bprefix, bnum] = prefixAndTrailingNum(rhs); if (aprefix != bprefix || anum.length() == 0 || bnum.length() == 0) { - return a < b; + return lhs < rhs; } - auto aa = std::stoull(anum); - auto bb = std::stoull(bnum); - return aa < bb; + return std::stoull(anum) < std::stoull(bnum); } static map d_getatomics; @@ -119,8 +138,8 @@ void disableStats(StatComponent component, const string& stats) std::vector disabledStats; stringtok(disabledStats, stats, ", "); auto& map = s_disabledStats[component]; - for (const auto& st : disabledStats) { - map.insert(st); + for (const auto& stat : disabledStats) { + map.insert(stat); } } @@ -152,16 +171,16 @@ static std::string getPrometheusName(const std::string& arg) { std::string name = arg; std::replace_if( - name.begin(), name.end(), [](char c) { return !isalnum(static_cast(c)); }, '_'); + name.begin(), name.end(), [](char letter) { return isalnum(static_cast(letter)) == 0; }, '_'); return "pdns_recursor_" + name; } std::atomic* getDynMetric(const std::string& str, const std::string& prometheusName) { - auto dm = d_dynmetrics.lock(); - auto f = dm->find(str); - if (f != dm->end()) { - return f->second.d_ptr; + auto locked = d_dynmetrics.lock(); + auto iter = locked->find(str); + if (iter != locked->end()) { + return iter->second.d_ptr; } std::string name(str); @@ -173,7 +192,7 @@ std::atomic* getDynMetric(const std::string& str, const std::stri } auto ret = dynmetrics{new std::atomic(), std::move(name)}; - (*dm)[str] = ret; + (*locked)[str] = ret; return ret.d_ptr; } @@ -189,10 +208,10 @@ static std::optional get(const string& name) } { - auto dm = d_dynmetrics.lock(); - auto f = rplookup(*dm, name); - if (f) { - return f->d_ptr->load(); + auto lcoked = d_dynmetrics.lock(); + const auto* ptr = rplookup(*lcoked, name); + if (ptr != nullptr) { + return ptr->d_ptr->load(); } } @@ -236,9 +255,9 @@ StatsMap getAllStatsMap(StatComponent component) } { - for (const auto& a : *(d_dynmetrics.lock())) { - if (disabledlistMap.count(a.first) == 0) { - ret.emplace(a.first, StatsMapEntry{a.second.d_prometheusName, std::to_string(*a.second.d_ptr)}); + for (const auto& value : *(d_dynmetrics.lock())) { + if (disabledlistMap.count(value.first) == 0) { + ret.emplace(value.first, StatsMapEntry{value.second.d_prometheusName, std::to_string(*value.second.d_ptr)}); } } } @@ -256,28 +275,30 @@ static string getAllStats() return ret; } -template -static string doGet(T begin, T end) +using ArgIterator = vector::iterator; + +static string doGet(ArgIterator begin, ArgIterator end) { string ret; - for (T i = begin; i != end; ++i) { + for (auto i = begin; i != end; ++i) { std::optional num = get(*i); - if (num) + if (num) { ret += std::to_string(*num) + "\n"; - else + } + else { ret += "UNKNOWN\n"; + } } return ret; } -template -string static doGetParameter(T begin, T end) +string static doGetParameter(ArgIterator begin, ArgIterator end) { string ret; string parm; using boost::replace_all; - for (T i = begin; i != end; ++i) { + for (auto i = begin; i != end; ++i) { if (::arg().parmIsset(*i)) { parm = ::arg()[*i]; replace_all(parm, "\\", "\\\\"); @@ -285,59 +306,60 @@ string static doGetParameter(T begin, T end) replace_all(parm, "\n", "\\n"); ret += *i + "=\"" + parm + "\"\n"; } - else + else { ret += *i + " not known\n"; + } } return ret; } /* Read an (open) fd from the control channel */ static FDWrapper -getfd(int s) +getfd(int socket) { - int fd = -1; - struct msghdr msg; - struct cmsghdr* cmsg; + int fileDesc = -1; + struct msghdr msg{}; + struct cmsghdr* cmsg{}; union { struct cmsghdr hdr; - unsigned char buf[CMSG_SPACE(sizeof(int))]; + std::array buf{}; } cmsgbuf; - struct iovec io_vector[1]; - char ch; + std::array io_vector{}; + char character = 0; - io_vector[0].iov_base = &ch; + io_vector[0].iov_base = &character; io_vector[0].iov_len = 1; memset(&msg, 0, sizeof(msg)); msg.msg_control = &cmsgbuf.buf; msg.msg_controllen = sizeof(cmsgbuf.buf); - msg.msg_iov = io_vector; - msg.msg_iovlen = 1; + msg.msg_iov = io_vector.data(); + msg.msg_iovlen = io_vector.size(); - if (recvmsg(s, &msg, 0) == -1) { + if (recvmsg(socket, &msg, 0) == -1) { throw PDNSException("recvmsg"); } - if ((msg.msg_flags & MSG_TRUNC) || (msg.msg_flags & MSG_CTRUNC)) { + if ((msg.msg_flags & MSG_TRUNC) != 0 || (msg.msg_flags & MSG_CTRUNC) != 0) { throw PDNSException("control message truncated"); } - for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != nullptr; cmsg = CMSG_NXTHDR(&msg, cmsg)) { if (cmsg->cmsg_len == CMSG_LEN(sizeof(int)) && cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { - fd = *(int*)CMSG_DATA(cmsg); + fileDesc = *reinterpret_cast(CMSG_DATA(cmsg)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) break; } } - return FDWrapper(fd); + return fileDesc; } -static uint64_t dumpAggressiveNSECCache(int fd) +static uint64_t dumpAggressiveNSECCache(int fileDesc) { if (!g_aggressiveNSECCache) { return 0; } - int newfd = dup(fd); + int newfd = dup(fileDesc); if (newfd == -1) { return 0; } @@ -347,50 +369,52 @@ static uint64_t dumpAggressiveNSECCache(int fd) } fprintf(filePtr.get(), "; aggressive NSEC cache dump follows\n;\n"); - struct timeval now; + struct timeval now{}; Utility::gettimeofday(&now, nullptr); return g_aggressiveNSECCache->dumpToFile(filePtr, now); } -static uint64_t* pleaseDumpEDNSMap(int fd) +// NOLINTBEGIN(cppcoreguidelines-owning-memory) +static uint64_t* pleaseDumpEDNSMap(int fileDesc) { - return new uint64_t(SyncRes::doEDNSDump(fd)); + return new uint64_t(SyncRes::doEDNSDump(fileDesc)); } -static uint64_t* pleaseDumpNSSpeeds(int fd) +static uint64_t* pleaseDumpNSSpeeds(int fileDesc) { - return new uint64_t(SyncRes::doDumpNSSpeeds(fd)); + return new uint64_t(SyncRes::doDumpNSSpeeds(fileDesc)); } -static uint64_t* pleaseDumpThrottleMap(int fd) +static uint64_t* pleaseDumpThrottleMap(int fileDesc) { - return new uint64_t(SyncRes::doDumpThrottleMap(fd)); + return new uint64_t(SyncRes::doDumpThrottleMap(fileDesc)); } -static uint64_t* pleaseDumpFailedServers(int fd) +static uint64_t* pleaseDumpFailedServers(int fileDesc) { - return new uint64_t(SyncRes::doDumpFailedServers(fd)); + return new uint64_t(SyncRes::doDumpFailedServers(fileDesc)); } -static uint64_t* pleaseDumpSavedParentNSSets(int fd) +static uint64_t* pleaseDumpSavedParentNSSets(int fileDesc) { - return new uint64_t(SyncRes::doDumpSavedParentNSSets(fd)); + return new uint64_t(SyncRes::doDumpSavedParentNSSets(fileDesc)); } -static uint64_t* pleaseDumpNonResolvingNS(int fd) +static uint64_t* pleaseDumpNonResolvingNS(int fileDesc) { - return new uint64_t(SyncRes::doDumpNonResolvingNS(fd)); + return new uint64_t(SyncRes::doDumpNonResolvingNS(fileDesc)); } -static uint64_t* pleaseDumpDoTProbeMap(int fd) +static uint64_t* pleaseDumpDoTProbeMap(int fileDesc) { - return new uint64_t(SyncRes::doDumpDoTProbeMap(fd)); + return new uint64_t(SyncRes::doDumpDoTProbeMap(fileDesc)); } +// NOLINTEND(cppcoreguidelines-owning-memory) // Generic dump to file command -static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)(int s), const string& name, bool threads = true) +static RecursorControlChannel::Answer doDumpToFile(int socket, uint64_t* (*function)(int), const string& name, bool threads = true) { - auto fdw = getfd(s); + auto fdw = getfd(socket); if (fdw < 0) { return {1, name + ": error opening dump file for writing: " + stringerror() + "\n"}; @@ -399,13 +423,13 @@ static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)( uint64_t total = 0; try { if (threads) { - int fd = fdw; - total = broadcastAccFunction([function, fd] { return function(fd); }); + int fileDesc = fdw; + total = broadcastAccFunction([function, fileDesc] { return function(fileDesc); }); } else { - auto ret = function(fdw); + auto* ret = function(fdw); total = *ret; - delete ret; + delete ret; // NOLINT(cppcoreguidelines-owning-memory) } } catch (std::exception& e) { @@ -419,8 +443,7 @@ static RecursorControlChannel::Answer doDumpToFile(int s, uint64_t* (*function)( } // Does not follow the generic dump to file pattern, has a more complex lambda -template -static RecursorControlChannel::Answer doDumpCache(int socket, T begin, T end) +static RecursorControlChannel::Answer doDumpCache(int socket, ArgIterator begin, ArgIterator end) { auto fdw = getfd(socket); @@ -467,27 +490,27 @@ static RecursorControlChannel::Answer doDumpCache(int socket, T begin, T end) } } catch (...) { + ; } return {0, "dumped " + std::to_string(total) + " records\n"}; } // Does not follow the generic dump to file pattern, has an argument -template -static RecursorControlChannel::Answer doDumpRPZ(int s, T begin, T end) +static RecursorControlChannel::Answer doDumpRPZ(int socket, ArgIterator begin, ArgIterator end) { - auto fdw = getfd(s); + auto fdw = getfd(socket); if (fdw < 0) { return {1, "Error opening dump file for writing: " + stringerror() + "\n"}; } - T i = begin; + auto iter = begin; - if (i == end) { + if (iter == end) { return {1, "No zone name specified\n"}; } - string zoneName = *i; + const string& zoneName = *iter; auto luaconf = g_luaconfs.getLocal(); const auto zone = luaconf->dfe.getZone(zoneName); @@ -506,11 +529,10 @@ static RecursorControlChannel::Answer doDumpRPZ(int s, T begin, T end) return {0, "done\n"}; } -template -static string doWipeCache(T begin, T end, uint16_t qtype) +static string doWipeCache(ArgIterator begin, ArgIterator end, uint16_t qtype) { vector> toWipe; - for (T i = begin; i != end; ++i) { + for (auto i = begin; i != end; ++i) { DNSName canon; bool subtree = false; @@ -529,7 +551,9 @@ static string doWipeCache(T begin, T end, uint16_t qtype) toWipe.emplace_back(canon, subtree); } - int count = 0, pcount = 0, countNeg = 0; + int count = 0; + int pcount = 0; + int countNeg = 0; for (const auto& wipe : toWipe) { try { auto res = wipeCaches(wipe.first, wipe.second, qtype); @@ -545,8 +569,7 @@ static string doWipeCache(T begin, T end, uint16_t qtype) return "wiped " + std::to_string(count) + " records, " + std::to_string(countNeg) + " negative records, " + std::to_string(pcount) + " packets\n"; } -template -static string doSetCarbonServer(T begin, T end) +static string doSetCarbonServer(ArgIterator begin, ArgIterator end) { auto config = g_carbonConfig.getCopy(); if (begin == end) { @@ -589,15 +612,14 @@ static string doSetCarbonServer(T begin, T end) return ret; } -template -static string doSetDnssecLogBogus(T begin, T end) +static string doSetDnssecLogBogus(ArgIterator begin, ArgIterator end) { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration, not changing the Bogus logging setting\n"; - - if (begin == end) + } + if (begin == end) { return "No DNSSEC Bogus logging setting specified\n"; - + } if (pdns_iequals(*begin, "on") || pdns_iequals(*begin, "yes")) { if (!g_dnssecLogBogus) { g_log << Logger::Warning << "Enabling DNSSEC Bogus logging, requested via control channel" << endl; @@ -619,15 +641,14 @@ static string doSetDnssecLogBogus(T begin, T end) return "Unknown DNSSEC Bogus setting: '" + *begin + "'\n"; } -template -static string doAddNTA(T begin, T end) +static string doAddNTA(ArgIterator begin, ArgIterator end) { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration, not adding a Negative Trust Anchor\n"; - - if (begin == end) + } + if (begin == end) { return "No NTA specified, doing nothing\n"; - + } DNSName who; try { who = DNSName(*begin); @@ -640,12 +661,13 @@ static string doAddNTA(T begin, T end) } begin++; - string why(""); + string why; while (begin != end) { why += *begin; begin++; - if (begin != end) + if (begin != end) { why += " "; + } } g_log << Logger::Warning << "Adding Negative Trust Anchor for " << who << " with reason '" << why << "', requested via control channel" << endl; g_luaconfs.modify([who, why](LuaConfigItems& lci) { @@ -661,15 +683,14 @@ static string doAddNTA(T begin, T end) return "Added Negative Trust Anchor for " + who.toLogString() + " with reason '" + why + "'\n"; } -template -static string doClearNTA(T begin, T end) +static string doClearNTA(ArgIterator begin, ArgIterator end) { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration, not removing a Negative Trust Anchor\n"; - - if (begin == end) + } + if (begin == end) { return "No Negative Trust Anchor specified, doing nothing.\n"; - + } if (begin + 1 == end && *begin == "*") { g_log << Logger::Warning << "Clearing all Negative Trust Anchors, requested via control channel" << endl; g_luaconfs.modify([](LuaConfigItems& lci) { @@ -681,8 +702,9 @@ static string doClearNTA(T begin, T end) vector toRemove; DNSName who; while (begin != end) { - if (*begin == "*") + if (*begin == "*") { return "Don't mix all Negative Trust Anchor removal with multiple Negative Trust Anchor removal. Nothing removed\n"; + } try { who = DNSName(*begin); } @@ -696,7 +718,7 @@ static string doClearNTA(T begin, T end) begin++; } - string removed(""); + string removed; bool first(true); try { for (auto const& entry : toRemove) { @@ -722,25 +744,25 @@ static string doClearNTA(T begin, T end) static string getNTAs() { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration\n"; - + } string ret("Configured Negative Trust Anchors:\n"); auto luaconf = g_luaconfs.getLocal(); - for (const auto& negAnchor : luaconf->negAnchors) + for (const auto& negAnchor : luaconf->negAnchors) { ret += negAnchor.first.toLogString() + "\t" + negAnchor.second + "\n"; + } return ret; } -template -static string doAddTA(T begin, T end) +static string doAddTA(ArgIterator begin, ArgIterator end) { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration, not adding a Trust Anchor\n"; - - if (begin == end) + } + if (begin == end) { return "No TA specified, doing nothing\n"; - + } DNSName who; try { who = DNSName(*begin); @@ -753,7 +775,7 @@ static string doAddTA(T begin, T end) } begin++; - string what(""); + string what; while (begin != end) { what += *begin + " "; begin++; @@ -762,8 +784,8 @@ static string doAddTA(T begin, T end) try { g_log << Logger::Warning << "Adding Trust Anchor for " << who << " with data '" << what << "', requested via control channel"; g_luaconfs.modify([who, what](LuaConfigItems& lci) { - auto ds = std::dynamic_pointer_cast(DSRecordContent::make(what)); - lci.dsAnchors[who].insert(*ds); + auto dsRecord = std::dynamic_pointer_cast(DSRecordContent::make(what)); + lci.dsAnchors[who].insert(*dsRecord); }); wipeCaches(who, true, 0xffff); g_log << Logger::Warning << endl; @@ -775,15 +797,14 @@ static string doAddTA(T begin, T end) } } -template -static string doClearTA(T begin, T end) +static string doClearTA(ArgIterator begin, ArgIterator end) { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration, not removing a Trust Anchor\n"; - - if (begin == end) + } + if (begin == end) { return "No Trust Anchor to clear\n"; - + } vector toRemove; DNSName who; while (begin != end) { @@ -796,14 +817,15 @@ static string doClearTA(T begin, T end) ret += ". No Anchors removed\n"; return ret; } - if (who.isRoot()) + if (who.isRoot()) { return "Refusing to remove root Trust Anchor, no Anchors removed\n"; + } toRemove.push_back(who); begin++; } - string removed(""); - bool first(true); + string removed; + bool first = true; try { for (auto const& entry : toRemove) { g_log << Logger::Warning << "Removing Trust Anchor for " << entry << ", requested via control channel" << endl; @@ -828,26 +850,26 @@ static string doClearTA(T begin, T end) static string getTAs() { - if (checkDNSSECDisabled()) + if (checkDNSSECDisabled()) { return "DNSSEC is disabled in the configuration\n"; - + } string ret("Configured Trust Anchors:\n"); auto luaconf = g_luaconfs.getLocal(); for (const auto& anchor : luaconf->dsAnchors) { ret += anchor.first.toLogString() + "\n"; - for (const auto& e : anchor.second) { - ret += "\t\t" + e.getZoneRepresentation() + "\n"; + for (const auto& entry : anchor.second) { + ret += "\t\t" + entry.getZoneRepresentation() + "\n"; } } return ret; } -template -static string setMinimumTTL(T begin, T end) +static string setMinimumTTL(ArgIterator begin, ArgIterator end) { - if (end - begin != 1) + if (end - begin != 1) { return "Need to supply new minimum TTL number\n"; + } try { pdns::checked_stoi_into(SyncRes::s_minimumTTL, *begin); return "New minimum TTL: " + std::to_string(SyncRes::s_minimumTTL) + "\n"; @@ -857,11 +879,11 @@ static string setMinimumTTL(T begin, T end) } } -template -static string setMinimumECSTTL(T begin, T end) +static string setMinimumECSTTL(ArgIterator begin, ArgIterator end) { - if (end - begin != 1) + if (end - begin != 1) { return "Need to supply new ECS minimum TTL number\n"; + } try { pdns::checked_stoi_into(SyncRes::s_minimumECSTTL, *begin); return "New minimum ECS TTL: " + std::to_string(SyncRes::s_minimumECSTTL) + "\n"; @@ -871,11 +893,11 @@ static string setMinimumECSTTL(T begin, T end) } } -template -static string setMaxCacheEntries(T begin, T end) +static string setMaxCacheEntries(ArgIterator begin, ArgIterator end) { - if (end - begin != 1) + if (end - begin != 1) { return "Need to supply new cache size\n"; + } try { g_maxCacheEntries = pdns::checked_stoi(*begin); return "New max cache entries: " + std::to_string(g_maxCacheEntries) + "\n"; @@ -885,11 +907,11 @@ static string setMaxCacheEntries(T begin, T end) } } -template -static string setMaxPacketCacheEntries(T begin, T end) +static string setMaxPacketCacheEntries(ArgIterator begin, ArgIterator end) { - if (end - begin != 1) + if (end - begin != 1) { return "Need to supply new packet cache size\n"; + } if (::arg().mustDo("disable-packetcache")) { return "Packet cache is disabled\n"; } @@ -903,8 +925,7 @@ static string setMaxPacketCacheEntries(T begin, T end) } } -template -static RecursorControlChannel::Answer setAggrNSECCacheSize(T begin, T end) +static RecursorControlChannel::Answer setAggrNSECCacheSize(ArgIterator begin, ArgIterator end) { if (end - begin != 1) { return {1, "Need to supply new aggressive NSEC cache size\n"}; @@ -924,16 +945,16 @@ static RecursorControlChannel::Answer setAggrNSECCacheSize(T begin, T end) static uint64_t getSysTimeMsec() { - struct rusage ru; - getrusage(RUSAGE_SELF, &ru); - return (ru.ru_stime.tv_sec * 1000ULL + ru.ru_stime.tv_usec / 1000); + struct rusage usage{}; + getrusage(RUSAGE_SELF, &usage); + return (usage.ru_stime.tv_sec * 1000ULL) + (usage.ru_stime.tv_usec / 1000); } static uint64_t getUserTimeMsec() { - struct rusage ru; - getrusage(RUSAGE_SELF, &ru); - return (ru.ru_utime.tv_sec * 1000ULL + ru.ru_utime.tv_usec / 1000); + struct rusage usage{}; + getrusage(RUSAGE_SELF, &usage); + return (usage.ru_utime.tv_sec * 1000ULL) + (usage.ru_utime.tv_usec / 1000); } /* This is a pretty weird set of functions. To get per-thread cpu usage numbers, @@ -954,7 +975,7 @@ static ThreadTimes* pleaseGetThreadCPUMsec() ret = (ru.ru_utime.tv_sec * 1000ULL + ru.ru_utime.tv_usec / 1000); ret += (ru.ru_stime.tv_sec * 1000ULL + ru.ru_stime.tv_usec / 1000); #endif - return new ThreadTimes{ret, vector()}; + return new ThreadTimes{ret, vector()}; // NOLINT(cppcoreguidelines-owning-memory) } /* Next up, when you want msec data for a specific thread, we check @@ -963,24 +984,24 @@ static ThreadTimes* pleaseGetThreadCPUMsec() We then answer you from the (re)fresh(ed) ThreadTimes. */ -static uint64_t doGetThreadCPUMsec(int n) +static uint64_t doGetThreadCPUMsec(unsigned int n) { static std::mutex s_mut; static time_t last = 0; - static ThreadTimes tt; + static ThreadTimes threadTimes; auto lock = std::scoped_lock(s_mut); if (last != time(nullptr)) { - tt = broadcastAccFunction(pleaseGetThreadCPUMsec); + threadTimes = broadcastAccFunction(pleaseGetThreadCPUMsec); last = time(nullptr); } - return tt.times.at(n); + return threadTimes.times.at(n); } static ProxyMappingStats_t* pleaseGetProxyMappingStats() { - auto ret = new ProxyMappingStats_t; + auto* ret = new ProxyMappingStats_t; // NOLINT(cppcoreguidelines-owning-memory) if (t_proxyMapping) { for (const auto& [key, entry] : *t_proxyMapping) { ret->emplace(key, ProxyMappingCounts{entry.stats.netmaskMatches, entry.stats.suffixMatches}); @@ -991,7 +1012,7 @@ static ProxyMappingStats_t* pleaseGetProxyMappingStats() static RemoteLoggerStats_t* pleaseGetRemoteLoggerStats() { - auto ret = make_unique(); + auto ret = make_unique(); // NOLINT(cppcoreguidelines-owning-memory) if (t_protobufServers.servers) { for (const auto& server : *t_protobufServers.servers) { @@ -1081,29 +1102,30 @@ static string getRemoteLoggerStats() static string* pleaseGetCurrentQueries() { ostringstream ostr; - struct timeval now; - gettimeofday(&now, 0); + struct timeval now{}; + gettimeofday(&now, nullptr); ostr << getMT()->getWaiters().size() << " currently outstanding questions\n"; boost::format fmt("%1% %|40t|%2% %|47t|%3% %|63t|%4% %|68t|%5% %|78t|%6%\n"); ostr << (fmt % "qname" % "qtype" % "remote" % "tcp" % "chained" % "spent(ms)"); - unsigned int n = 0; + unsigned int count = 0; for (const auto& mthread : getMT()->getWaiters()) { const std::shared_ptr& pident = mthread.key; const double spent = g_networkTimeoutMsec - (DiffTime(now, mthread.ttd) * 1000); ostr << (fmt % pident->domain.toLogString() /* ?? */ % DNSRecordContent::NumberToType(pident->type) - % pident->remote.toString() % (pident->tcpsock ? 'Y' : 'n') + % pident->remote.toString() % ((pident->tcpsock != 0) ? 'Y' : 'n') % (pident->fd == -1 ? 'Y' : 'n') % (spent > 0 ? spent : '0')); - ++n; - if (n >= 100) + ++count; + if (count >= 100) { break; + } } ostr << " - done\n"; - return new string(ostr.str()); + return new string(ostr.str()); // NOLINT(cppcoreguidelines-owning-memory) } static string doCurrentQueries() @@ -1118,7 +1140,7 @@ static uint64_t getNegCacheSize() uint64_t* pleaseGetConcurrentQueries() { - return new uint64_t(getMT() ? getMT()->numProcesses() : 0); + return new uint64_t((getMT() != nullptr) ? getMT()->numProcesses() : 0); // NOLINT(cppcoreguidelines-owning-memory) } static uint64_t getConcurrentQueries() @@ -1149,16 +1171,16 @@ static StatsMap toStatsMap(const string& name, const pdns::Histogram& histogram) const auto& data = histogram.getCumulativeBuckets(); const string pbasename = getPrometheusName(name); StatsMap entries; - char buf[32]; + std::array buf{}; for (const auto& bucket : data) { - snprintf(buf, sizeof(buf), "%g", bucket.d_boundary / 1e6); - std::string pname = pbasename + "seconds_bucket{" + "le=\"" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf) + "\"}"; + snprintf(buf.data(), buf.size(), "%g", static_cast(bucket.d_boundary) / 1e6); + std::string pname = pbasename + "seconds_bucket{" + "le=\"" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf.data()) + "\"}"; entries.emplace(bucket.d_name, StatsMapEntry{std::move(pname), std::to_string(bucket.d_count)}); } - snprintf(buf, sizeof(buf), "%g", histogram.getSum() / 1e6); - entries.emplace(name + "sum", StatsMapEntry{pbasename + "seconds_sum", buf}); + snprintf(buf.data(), buf.size(), "%g", static_cast(histogram.getSum()) / 1e6); + entries.emplace(name + "sum", StatsMapEntry{pbasename + "seconds_sum", buf.data()}); entries.emplace(name + "count", StatsMapEntry{pbasename + "seconds_count", std::to_string(data.back().d_count)}); return entries; @@ -1168,27 +1190,27 @@ static StatsMap toStatsMap(const string& name, const pdns::Histogram& histogram4 { const string pbasename = getPrometheusName(name); StatsMap entries; - char buf[32]; + std::array buf{}; std::string pname; const auto& data4 = histogram4.getCumulativeBuckets(); for (const auto& bucket : data4) { - snprintf(buf, sizeof(buf), "%g", bucket.d_boundary / 1e6); - pname = pbasename + "seconds_bucket{ipversion=\"v4\",le=\"" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf) + "\"}"; + snprintf(buf.data(), buf.size(), "%g", static_cast(bucket.d_boundary) / 1e6); + pname = pbasename + R"(seconds_bucket{ipversion="v4",le=")" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf.data()) + "\"}"; entries.emplace(bucket.d_name + "4", StatsMapEntry{pname, std::to_string(bucket.d_count)}); } - snprintf(buf, sizeof(buf), "%g", histogram4.getSum() / 1e6); - entries.emplace(name + "sum4", StatsMapEntry{pbasename + "seconds_sum{ipversion=\"v4\"}", buf}); + snprintf(buf.data(), buf.size(), "%g", static_cast(histogram4.getSum()) / 1e6); + entries.emplace(name + "sum4", StatsMapEntry{pbasename + "seconds_sum{ipversion=\"v4\"}", buf.data()}); entries.emplace(name + "count4", StatsMapEntry{pbasename + "seconds_count{ipversion=\"v4\"}", std::to_string(data4.back().d_count)}); const auto& data6 = histogram6.getCumulativeBuckets(); for (const auto& bucket : data6) { - snprintf(buf, sizeof(buf), "%g", bucket.d_boundary / 1e6); - pname = pbasename + "seconds_bucket{ipversion=\"v6\",le=\"" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf) + "\"}"; + snprintf(buf.data(), buf.size(), "%g", static_cast(bucket.d_boundary) / 1e6); + pname = pbasename + R"(seconds_bucket{ipversion="v6",le=")" + (bucket.d_boundary == std::numeric_limits::max() ? "+Inf" : buf.data()) + "\"}"; entries.emplace(bucket.d_name + "6", StatsMapEntry{pname, std::to_string(bucket.d_count)}); } - snprintf(buf, sizeof(buf), "%g", histogram6.getSum() / 1e6); - entries.emplace(name + "sum6", StatsMapEntry{pbasename + "seconds_sum{ipversion=\"v6\"}", buf}); + snprintf(buf.data(), buf.size(), "%g", static_cast(histogram6.getSum()) / 1e6); + entries.emplace(name + "sum6", StatsMapEntry{pbasename + "seconds_sum{ipversion=\"v6\"}", buf.data()}); entries.emplace(name + "count6", StatsMapEntry{pbasename + "seconds_count{ipversion=\"v6\"}", std::to_string(data6.back().d_count)}); return entries; @@ -1199,13 +1221,14 @@ static StatsMap toAuthRCodeStatsMap(const string& name) const string pbasename = getPrometheusName(name); StatsMap entries; - uint8_t n = 0; + uint8_t count = 0; auto rcodes = g_Counters.sum(rec::RCode::auth).rcodeCounters; for (const auto& entry : rcodes) { - const auto key = RCode::to_short_s(n); - std::string pname = pbasename + "{rcode=\"" + key + "\"}"; + const auto key = RCode::to_short_s(count); + std::string pname = pbasename; + pname += "{rcode=\"" + key + "\"}"; entries.emplace("auth-" + key + "-answers", StatsMapEntry{std::move(pname), std::to_string(entry)}); - n++; + count++; } return entries; } @@ -1216,10 +1239,10 @@ static StatsMap toCPUStatsMap(const string& name) StatsMap entries; // Handler is not reported - for (unsigned int n = 0; n < RecThreadInfo::numRecursorThreads() - 1; ++n) { - uint64_t tm = doGetThreadCPUMsec(n); - std::string pname = pbasename + "{thread=\"" + std::to_string(n) + "\"}"; - entries.emplace(name + "-thread-" + std::to_string(n), StatsMapEntry{std::move(pname), std::to_string(tm)}); + for (unsigned int thread = 0; thread < RecThreadInfo::numRecursorThreads() - 1; ++thread) { + uint64_t timeTaken = doGetThreadCPUMsec(thread); + std::string pname = pbasename + "{thread=\"" + std::to_string(thread) + "\"}"; + entries.emplace(name + "-thread-" + std::to_string(thread), StatsMapEntry{std::move(pname), std::to_string(timeTaken)}); } return entries; } @@ -1233,14 +1256,17 @@ static StatsMap toRPZStatsMap(const string& name, const std::unordered_map>* pleaseGetQueryRing() { - typedef pair query_t; - vector* ret = new vector(); - if (!t_queryring) + using query_t = pair; + auto* ret = new vector(); + if (!t_queryring) { return ret; + } ret->reserve(t_queryring->size()); - for (const query_t& q : *t_queryring) { - ret->push_back(q); + for (const query_t& query : *t_queryring) { + ret->emplace_back(query); } return ret; } vector>* pleaseGetServfailQueryRing() { - typedef pair query_t; - vector* ret = new vector(); - if (!t_servfailqueryring) + using query_t = pair; + auto* ret = new vector(); + if (!t_servfailqueryring) { return ret; + } ret->reserve(t_servfailqueryring->size()); - for (const query_t& q : *t_servfailqueryring) { - ret->push_back(q); + for (const query_t& query : *t_servfailqueryring) { + ret->emplace_back(query); } return ret; } vector>* pleaseGetBogusQueryRing() { - typedef pair query_t; - vector* ret = new vector(); - if (!t_bogusqueryring) + using query_t = pair; + auto* ret = new vector(); + if (!t_bogusqueryring) { return ret; + } ret->reserve(t_bogusqueryring->size()); - for (const query_t& q : *t_bogusqueryring) { - ret->push_back(q); + for (const query_t& query : *t_bogusqueryring) { + ret->emplace_back(query); } return ret; } -typedef std::function*()> pleaseremotefunc_t; -typedef std::function>*()> pleasequeryfunc_t; +using pleaseremotefunc_t = std::function*()>; +using pleasequeryfunc_t = std::function>*()>; vector* pleaseGetRemotes() { - vector* ret = new vector(); - if (!t_remotes) + auto* ret = new vector(); + if (!t_remotes) { return ret; - + } ret->reserve(t_remotes->size()); - for (const ComboAddress& ca : *t_remotes) { - ret->push_back(ca); + for (const ComboAddress& address : *t_remotes) { + ret->emplace_back(address); } return ret; } vector* pleaseGetServfailRemotes() { - vector* ret = new vector(); - if (!t_servfailremotes) + auto* ret = new vector(); + if (!t_servfailremotes) { return ret; + } ret->reserve(t_servfailremotes->size()); - for (const ComboAddress& ca : *t_servfailremotes) { - ret->push_back(ca); + for (const ComboAddress& address : *t_servfailremotes) { + ret->push_back(address); } return ret; } vector* pleaseGetBogusRemotes() { - vector* ret = new vector(); - if (!t_bogusremotes) + auto* ret = new vector(); + if (!t_bogusremotes) { return ret; + } ret->reserve(t_bogusremotes->size()); - for (const ComboAddress& ca : *t_bogusremotes) { - ret->push_back(ca); + for (const ComboAddress& address : *t_bogusremotes) { + ret->emplace_back(address); } return ret; } vector* pleaseGetLargeAnswerRemotes() { - vector* ret = new vector(); - if (!t_largeanswerremotes) + auto* ret = new vector(); + if (!t_largeanswerremotes) { return ret; + } ret->reserve(t_largeanswerremotes->size()); - for (const ComboAddress& ca : *t_largeanswerremotes) { - ret->push_back(ca); + for (const ComboAddress& address : *t_largeanswerremotes) { + ret->emplace_back(address); } return ret; } vector* pleaseGetTimeouts() { - vector* ret = new vector(); - if (!t_timeouts) + auto* ret = new vector(); + if (!t_timeouts) { return ret; + } ret->reserve(t_timeouts->size()); - for (const ComboAddress& ca : *t_timeouts) { - ret->push_back(ca); + for (const ComboAddress& address : *t_timeouts) { + ret->emplace_back(address); } return ret; } +// NOLINTEND(cppcoreguidelines-owning-memory) static string doGenericTopRemotes(const pleaseremotefunc_t& func) { @@ -1543,8 +1580,9 @@ static string doGenericTopRemotes(const pleaseremotefunc_t& func) DNSName getRegisteredName(const DNSName& dom) { auto parts = dom.getRawLabels(); - if (parts.size() <= 2) + if (parts.size() <= 2) { return dom; + } reverse(parts.begin(), parts.end()); for (string& str : parts) { str = toLower(str); @@ -1556,11 +1594,11 @@ DNSName getRegisteredName(const DNSName& dom) if (parts.size() == 1 || binary_search(g_pubs.begin(), g_pubs.end(), parts)) { string ret = std::move(last); - if (!ret.empty()) + if (!ret.empty()) { ret += "."; - - for (auto p = parts.crbegin(); p != parts.crend(); ++p) { - ret += (*p) + "."; + } + for (auto part = parts.crbegin(); part != parts.crend(); ++part) { + ret += (*part) + "."; } return DNSName(ret); } @@ -1612,7 +1650,7 @@ static string doGenericTopQueries(const pleasequeryfunc_t& func, const std::func static string* nopFunction() { - return new string("pong " + RecThreadInfo::self().getName() + '\n'); + return new string("pong " + RecThreadInfo::self().getName() + '\n'); // NOLINT(cppcoreguidelines-owning-memory) } static string getDontThrottleNames() @@ -1627,8 +1665,7 @@ static string getDontThrottleNetmasks() return dtn->toString() + "\n"; } -template -static string addDontThrottleNames(T begin, T end) +static string addDontThrottleNames(ArgIterator begin, ArgIterator end) { if (begin == end) { return "No names specified, keeping existing list\n"; @@ -1636,8 +1673,8 @@ static string addDontThrottleNames(T begin, T end) vector toAdd; while (begin != end) { try { - auto d = DNSName(*begin); - toAdd.push_back(std::move(d)); + auto name = DNSName(*begin); + toAdd.emplace_back(std::move(name)); } catch (const std::exception& e) { return "Problem parsing '" + *begin + "': " + e.what() + ", nothing added\n"; @@ -1648,13 +1685,13 @@ static string addDontThrottleNames(T begin, T end) string ret = "Added"; auto dnt = g_dontThrottleNames.getCopy(); bool first = true; - for (auto const& d : toAdd) { + for (auto const& name : toAdd) { if (!first) { ret += ","; } first = false; - ret += " " + d.toLogString(); - dnt.add(d); + ret += " " + name.toLogString(); + dnt.add(name); } g_dontThrottleNames.setState(std::move(dnt)); @@ -1664,8 +1701,7 @@ static string addDontThrottleNames(T begin, T end) return ret + "\n"; } -template -static string addDontThrottleNetmasks(T begin, T end) +static string addDontThrottleNetmasks(ArgIterator begin, ArgIterator end) { if (begin == end) { return "No netmasks specified, keeping existing list\n"; @@ -1673,8 +1709,8 @@ static string addDontThrottleNetmasks(T begin, T end) vector toAdd; while (begin != end) { try { - auto n = Netmask(*begin); - toAdd.push_back(n); + auto netmask = Netmask(*begin); + toAdd.push_back(netmask); } catch (const std::exception& e) { return "Problem parsing '" + *begin + "': " + e.what() + ", nothing added\n"; @@ -1688,13 +1724,13 @@ static string addDontThrottleNetmasks(T begin, T end) string ret = "Added"; auto dnt = g_dontThrottleNetmasks.getCopy(); bool first = true; - for (auto const& t : toAdd) { + for (auto const& netmask : toAdd) { if (!first) { ret += ","; } first = false; - ret += " " + t.toString(); - dnt.addMask(t); + ret += " " + netmask.toString(); + dnt.addMask(netmask); } g_dontThrottleNetmasks.setState(std::move(dnt)); @@ -1704,12 +1740,11 @@ static string addDontThrottleNetmasks(T begin, T end) return ret + "\n"; } -template -static string clearDontThrottleNames(T begin, T end) +static string clearDontThrottleNames(ArgIterator begin, ArgIterator end) { - if (begin == end) + if (begin == end) { return "No names specified, doing nothing.\n"; - + } if (begin + 1 == end && *begin == "*") { SuffixMatchNode smn; g_dontThrottleNames.setState(std::move(smn)); @@ -1724,7 +1759,7 @@ static string clearDontThrottleNames(T begin, T end) if (*begin == "*") { return "Please don't mix '*' with other names, nothing removed\n"; } - toRemove.push_back(DNSName(*begin)); + toRemove.emplace_back(*begin); } catch (const std::exception& e) { return "Problem parsing '" + *begin + "': " + e.what() + ", nothing removed\n"; @@ -1751,12 +1786,11 @@ static string clearDontThrottleNames(T begin, T end) return ret + "\n"; } -template -static string clearDontThrottleNetmasks(T begin, T end) +static string clearDontThrottleNetmasks(ArgIterator begin, ArgIterator end) { - if (begin == end) + if (begin == end) { return "No netmasks specified, doing nothing.\n"; - + } if (begin + 1 == end && *begin == "*") { auto nmg = g_dontThrottleNetmasks.getCopy(); nmg.clear(); @@ -1773,8 +1807,8 @@ static string clearDontThrottleNetmasks(T begin, T end) if (*begin == "*") { return "Please don't mix '*' with other netmasks, nothing removed\n"; } - auto n = Netmask(*begin); - toRemove.push_back(n); + auto netmask = Netmask(*begin); + toRemove.push_back(netmask); } catch (const std::exception& e) { return "Problem parsing '" + *begin + "': " + e.what() + ", nothing added\n"; @@ -1804,8 +1838,7 @@ static string clearDontThrottleNetmasks(T begin, T end) return ret + "\n"; } -template -static string setEventTracing(T begin, T end) +static string setEventTracing(ArgIterator begin, ArgIterator end) { if (begin == end) { return "No event trace enabled value specified\n"; @@ -1819,15 +1852,15 @@ static string setEventTracing(T begin, T end) } } -static void* pleaseSupplantProxyMapping(const ProxyMapping& pm) +static void* pleaseSupplantProxyMapping(const ProxyMapping& proxyMapping) { - if (pm.empty()) { + if (proxyMapping.empty()) { t_proxyMapping = nullptr; } else { // Copy the existing stats values, for the new config items also present in the old auto newmapping = make_unique(); - for (const auto& [nm, entry] : pm) { + for (const auto& [nm, entry] : proxyMapping) { auto& newentry = newmapping->insert(nm); newentry.second = entry; if (t_proxyMapping) { @@ -1985,8 +2018,7 @@ RecursorControlChannel::Answer luaconfig(bool broadcast) } } -template -static RecursorControlChannel::Answer luaconfig(T begin, T end) +static RecursorControlChannel::Answer luaconfig(ArgIterator begin, ArgIterator end) { if (begin != end) { if (g_luaSettingsInYAML) { diff --git a/pdns/recursordist/rec_control.cc b/pdns/recursordist/rec_control.cc index 9b65630cf8..c8fcf7f772 100644 --- a/pdns/recursordist/rec_control.cc +++ b/pdns/recursordist/rec_control.cc @@ -57,14 +57,14 @@ static void initArguments(int argc, char** argv, Logr::log_t log) arg().laxParse(argc, argv); if (arg().mustDo("version")) { cout << "rec_control version " << VERSION << endl; - exit(0); + exit(0); // NOLINT(concurrency-mt-unsafe) } if (arg().mustDo("help") || arg().getCommands().empty()) { cout << "syntax: rec_control [options] command, options as below: " << endl << endl; cout << arg().helpstring(arg()["help"]) << endl; cout << "In addition, 'rec_control help' can be used to retrieve a list\nof available commands from PowerDNS" << endl; - exit(arg().mustDo("help") ? 0 : 99); + exit(arg().mustDo("help") ? 0 : 99); // NOLINT(concurrency-mt-unsafe) } string configname = ::arg()["config-dir"] + "/recursor"; @@ -344,12 +344,12 @@ int main(int argc, char** argv) initArguments(argc, argv, log); string sockname = "pdns_recursor"; - if (arg()["config-name"] != "") + if (!empty(arg()["config-name"])) { sockname += "-" + arg()["config-name"]; - - if (!arg()["process"].empty()) + } + if (!arg()["process"].empty()) { sockname += "." + arg()["process"]; - + } sockname.append(".controlsocket"); const vector& commands = arg().getCommands(); @@ -385,39 +385,39 @@ int main(int argc, char** argv) } string command; - int fd = -1; - unsigned int i = 0; - while (i < commands.size()) { - if (i > 0) { + int fileDesc = -1; + unsigned int iteration = 0; + while (iteration < commands.size()) { + if (iteration > 0) { command += " "; } - command += commands[i]; + command += commands[iteration]; // special case: trace-regex with no arguments is clear regex auto traceregexClear = command == "trace-regex" && commands.size() == 1; - if (fileCommands.count(commands[i]) > 0 && !traceregexClear) { - if (i + 1 < commands.size()) { + if (fileCommands.count(commands[iteration]) > 0 && !traceregexClear) { + if (iteration + 1 < commands.size()) { // dump-rpz is different, it also has a zonename as argument // trace-regex is different, it also has a regexp as argument - if (commands[i] == "dump-rpz" || commands[i] == "trace-regex") { - if (i + 2 < commands.size()) { - ++i; + if (commands[iteration] == "dump-rpz" || commands[iteration] == "trace-regex") { + if (iteration + 2 < commands.size()) { + ++iteration; command += " "; - command += commands[i]; // add rpzname/regex and continue with filename + command += commands[iteration]; // add rpzname/regex and continue with filename } else { throw PDNSException("Command needs two arguments"); } } - ++i; - if (commands[i] == "-") { - fd = STDOUT_FILENO; + ++iteration; + if (commands[iteration] == "-") { + fileDesc = STDOUT_FILENO; } else { - fd = open(commands[i].c_str(), O_CREAT | O_EXCL | O_WRONLY, 0660); + fileDesc = open(commands[iteration].c_str(), O_CREAT | O_EXCL | O_WRONLY, 0660); } - if (fd == -1) { + if (fileDesc == -1) { int err = errno; throw PDNSException("Error opening dump file for writing: " + stringerror(err)); } @@ -426,15 +426,15 @@ int main(int argc, char** argv) throw PDNSException("Command needs a file argument"); } } - ++i; + ++iteration; } auto timeout = arg().asNum("timeout"); RecursorControlChannel rccS; rccS.connect(arg()["socket-dir"], sockname); - rccS.send(rccS.d_fd, {0, std::move(command)}, timeout, fd); + RecursorControlChannel::send(rccS.getDescriptor(), {0, std::move(command)}, timeout, fileDesc); - auto receive = rccS.recv(rccS.d_fd, timeout); + auto receive = RecursorControlChannel::recv(rccS.getDescriptor(), timeout); if (receive.d_ret != 0) { cerr << receive.d_str; } -- 2.47.2