From 9c2268fa41a381332c2e32cb9a2daa1d6cac1329 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 29 Mar 2022 10:15:00 -0400
Subject: [PATCH] fix various cppcheck warnings/errors

---
 src/crypto.cpp           |  4 +++-
 src/dht_proxy_client.cpp |  9 ++++++---
 src/dht_proxy_server.cpp |  6 +++---
 src/dhtrunner.cpp        |  4 ++--
 src/log.cpp              |  4 ++--
 src/node.cpp             |  5 +----
 src/op_cache.cpp         |  2 +-
 src/op_cache.h           |  8 ++++----
 src/search.h             | 27 +++++++++++++--------------
 src/value_cache.h        |  2 +-
 10 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/src/crypto.cpp b/src/crypto.cpp
index 13c68129..9eb80f6e 100644
--- a/src/crypto.cpp
+++ b/src/crypto.cpp
@@ -1057,6 +1057,8 @@ Certificate::generateOcspRequest(gnutls_x509_crt_t& issuer)
     Blob noncebuf(32);
     gnutls_datum_t nonce = { noncebuf.data(), (unsigned)noncebuf.size() };
     err = gnutls_rnd(GNUTLS_RND_NONCE, nonce.data, nonce.size);
+    if (err < 0)
+        throw CryptoException(gnutls_strerror(err));
     err = gnutls_ocsp_req_set_nonce(req.get(), 0, &nonce);
     if (err < 0)
         throw CryptoException(gnutls_strerror(err));
@@ -1473,7 +1475,7 @@ RevocationList::RevocationList(const Blob& b)
     } catch (const std::exception& e) {
         gnutls_x509_crl_deinit(crl);
         crl = nullptr;
-        throw e;
+        throw;
     }
 }
 
diff --git a/src/dht_proxy_client.cpp b/src/dht_proxy_client.cpp
index e47373c6..df3b5874 100644
--- a/src/dht_proxy_client.cpp
+++ b/src/dht_proxy_client.cpp
@@ -40,16 +40,17 @@ struct DhtProxyClient::OperationState {
 
 struct DhtProxyClient::Listener
 {
-    Listener(OpValueCache&& c):
+    explicit Listener(OpValueCache&& c):
         cache(std::move(c))
     {}
 
-    unsigned callbackId;
     OpValueCache cache;
     CacheValueCallback cb;
     Sp<OperationState> opstate;
     std::shared_ptr<http::Request> request;
+#ifdef OPENDHT_PUSH_NOTIFICATIONS
     std::unique_ptr<asio::steady_timer> refreshSubscriberTimer;
+#endif
 };
 
 struct PermanentPut {
@@ -814,6 +815,7 @@ DhtProxyClient::listen(const InfoHash& key, ValueCallback cb, Value::Filter filt
             }
             return false;
         };
+#ifdef OPENDHT_PUSH_NOTIFICATIONS
         if (not deviceKey_.empty()) {
             /*
              * Relaunch push listeners even if a timeout is not received
@@ -826,6 +828,7 @@ DhtProxyClient::listen(const InfoHash& key, ValueCallback cb, Value::Filter filt
             l->second.refreshSubscriberTimer->async_wait(std::bind(&DhtProxyClient::handleResubscribe, this,
                                                          std::placeholders::_1, key, token, opstate));
         }
+#endif
         ListenMethod method;
         restinio::http_request_header_t header;
         if (deviceKey_.empty()){ // listen
@@ -1229,7 +1232,7 @@ DhtProxyClient::pushNotificationReceived(const std::map<std::string, std::string
                         ids.emplace_back(std::stoull(substr));
                     }
                     {
-                        std::lock_guard<std::mutex> lock(lockCallbacks_);
+                        std::lock_guard<std::mutex> lockCb(lockCallbacks_);
                         callbacks_.emplace_back([this, key, token, opstate, ids, sendTime]() {
                             if (opstate->stop)
                                 return;
diff --git a/src/dht_proxy_server.cpp b/src/dht_proxy_server.cpp
index 93bcf3bc..26940438 100644
--- a/src/dht_proxy_server.cpp
+++ b/src/dht_proxy_server.cpp
@@ -99,7 +99,7 @@ class DhtProxyServer::ConnectionListener
 {
 public:
     ConnectionListener() {};
-    ConnectionListener(std::function<void(restinio::connection_id_t)> onClosed) : onClosed_(std::move(onClosed)) {};
+    explicit ConnectionListener(std::function<void(restinio::connection_id_t)> onClosed) : onClosed_(std::move(onClosed)) {};
     ~ConnectionListener() {};
 
     /**
@@ -382,9 +382,9 @@ DhtProxyServer::loadState(Is& is, size_t size) {
                     logger_->d("No persistent puts in state");
             }
 #ifdef OPENDHT_PUSH_NOTIFICATIONS
-            if (auto listeners = findMapValue(oh.get(), "pushListeners")) {
+            if (auto pushListeners = findMapValue(oh.get(), "pushListeners")) {
                 std::lock_guard<std::mutex> lock(lockListener_);
-                pushListeners_ = listeners->as<decltype(pushListeners_)>();
+                pushListeners_ = pushListeners->as<decltype(pushListeners_)>();
                 if (logger_)
                     logger_->d("Loading %zu push listeners", pushListeners_.size());
                 for (auto& pushListener : pushListeners_) {
diff --git a/src/dhtrunner.cpp b/src/dhtrunner.cpp
index 75bb07dd..e323eb77 100644
--- a/src/dhtrunner.cpp
+++ b/src/dhtrunner.cpp
@@ -181,8 +181,8 @@ DhtRunner::run(const Config& config, Context&& context)
         config_ = config;
         identityAnnouncedCb_ = context.identityAnnouncedCb;
 #endif
-        auto dht = std::unique_ptr<DhtInterface>(new Dht(std::move(context.sock), SecureDht::getConfig(config.dht_config), context.logger));
-        dht_ = std::unique_ptr<SecureDht>(new SecureDht(std::move(dht), config.dht_config, std::move(context.identityAnnouncedCb), context.logger));
+        auto dht = std::make_unique<Dht>(std::move(context.sock), SecureDht::getConfig(config.dht_config), context.logger);
+        dht_ = std::make_unique<SecureDht>(std::move(dht), config.dht_config, std::move(context.identityAnnouncedCb), context.logger);
         enableProxy(not config.proxy_server.empty());
     } catch(const std::exception& e) {
         config_ = {};
diff --git a/src/log.cpp b/src/log.cpp
index f9ef28e7..100f0f74 100644
--- a/src/log.cpp
+++ b/src/log.cpp
@@ -38,7 +38,7 @@ printLog(std::ostream& s, char const *m, va_list args) {
     // print log to buffer
     std::array<char, 8192> buffer;
     int ret = vsnprintf(buffer.data(), buffer.size(), m, args);
-    if (ret < 0)
+    if (ret <= 0)
         return;
 
     // write timestamp
@@ -89,7 +89,7 @@ std::shared_ptr<Logger>
 getSyslogLogger(const char* name) {
 #ifndef _WIN32
     struct Syslog {
-        Syslog(const char* n) {
+        explicit Syslog(const char* n) {
             openlog(n, LOG_NDELAY, LOG_USER);
         }
         ~Syslog() {
diff --git a/src/node.cpp b/src/node.cpp
index 8fb57f92..c444bbe2 100644
--- a/src/node.cpp
+++ b/src/node.cpp
@@ -137,10 +137,7 @@ Node::openSocket(SocketCb&& cb)
     if (++transaction_id == 0)
         transaction_id = 1;
 
-    auto sock = std::make_shared<Socket>(std::move(cb));
-    auto s = sockets_.emplace(transaction_id, std::move(sock));
-    if (not s.second)
-        s.first->second = std::move(sock);
+    sockets_[transaction_id] = std::make_shared<Socket>(std::move(cb));
     return transaction_id;
 }
 
diff --git a/src/op_cache.cpp b/src/op_cache.cpp
index cdc61c5e..de2c4606 100644
--- a/src/op_cache.cpp
+++ b/src/op_cache.cpp
@@ -175,7 +175,7 @@ SearchCache::listen(const ValueCallback& get_cb, const Sp<Query>& q, const Value
     auto op = getOp(q);
     if (op == ops.end()) {
         // New query
-        op = ops.emplace(q, std::unique_ptr<OpCache>(new OpCache)).first;
+        op = ops.emplace(q, std::make_unique<OpCache>()).first;
         auto& cache = *op->second;
         cache.searchToken = onListen(q, [&](const std::vector<Sp<Value>>& values, bool expired){
             return cache.onValue(values, expired);
diff --git a/src/op_cache.h b/src/op_cache.h
index 1c24dbb5..72b62239 100644
--- a/src/op_cache.h
+++ b/src/op_cache.h
@@ -28,13 +28,13 @@ struct OpCacheValueStorage
     Sp<Value> data {};
     unsigned refCount {1};
     system_clock::time_point updated {system_clock::time_point::min()};
-    OpCacheValueStorage(Sp<Value> val) : data(val) {}
+    explicit OpCacheValueStorage(Sp<Value> val) : data(val) {}
 };
 
 class OpValueCache {
 public:
-    OpValueCache(ValueCallback&& cb) noexcept : callback(std::forward<ValueCallback>(cb)) {}
-    OpValueCache(OpValueCache&& o) noexcept : values(std::move(o.values)), callback(std::move(o.callback)) {
+    explicit OpValueCache(ValueCallback&& cb) noexcept : callback(std::forward<ValueCallback>(cb)) {}
+    explicit OpValueCache(OpValueCache&& o) noexcept : values(std::move(o.values)), callback(std::move(o.callback)) {
         o.callback = {};
     }
 
@@ -150,7 +150,7 @@ public:
         return cache.size();
     }
 
-    size_t searchToken;
+    size_t searchToken {0};
 private:
     constexpr static const std::chrono::seconds EXPIRATION {60};
     OpCache(const OpCache&) = delete;
diff --git a/src/search.h b/src/search.h
index 7969a2b1..5ec9160c 100644
--- a/src/search.h
+++ b/src/search.h
@@ -164,12 +164,12 @@ struct Dht::SearchNode {
         if (node->isExpired())
             return false;
 
-        bool pending {false},
+        bool is_pending {false},
              completed_sq_status {false},
              pending_sq_status {false};
         for (const auto& s : getStatus) {
             if (s.second and s.second->pending())
-                pending = true;
+                is_pending = true;
             if (s.first and q and q->isSatisfiedBy(*s.first) and s.second) {
                 if (s.second->pending())
                     pending_sq_status = true;
@@ -180,7 +180,7 @@ struct Dht::SearchNode {
             }
         }
 
-        return (not pending and now > last_get_reply + Node::NODE_EXPIRE_TIME) or
+        return (not is_pending and now > last_get_reply + Node::NODE_EXPIRE_TIME) or
                 not (completed_sq_status or pending_sq_status or hasStartedPagination(q));
     }
 
@@ -437,13 +437,13 @@ struct Dht::Search {
     ~Search() {
         if (opExpirationJob)
             opExpirationJob->cancel();
-        for (auto& get : callbacks) {
-            get.second.done_cb(false, {});
-            get.second.done_cb = {};
+        for (auto& g : callbacks) {
+            g.second.done_cb(false, {});
+            g.second.done_cb = {};
         }
-        for (auto& put : announce) {
-            put.callback(false, {});
-            put.callback = {};
+        for (auto& a : announce) {
+            a.callback(false, {});
+            a.callback = {};
         }
     }
 
@@ -465,7 +465,7 @@ struct Dht::Search {
     bool insertNode(const Sp<Node>& n, time_point now, const Blob& token={});
 
     SearchNode* getNode(const Sp<Node>& n) {
-        auto srn = std::find_if(nodes.begin(), nodes.end(), [&](std::unique_ptr<SearchNode>& sn) {
+        auto srn = std::find_if(nodes.begin(), nodes.end(), [&](const std::unique_ptr<SearchNode>& sn) {
             return n == sn->node;
         });
         return (srn == nodes.end()) ? nullptr : (*srn).get();
@@ -665,12 +665,11 @@ struct Dht::Search {
     }
     unsigned getNumberOfConsecutiveBadNodes() const {
         unsigned count = 0;
-        std::find_if(nodes.begin(), nodes.end(), [&count](const std::unique_ptr<SearchNode>& sn) {
+        for (const auto& sn : nodes) {
             if (not sn->node->isExpired())
-                return true;
+                break;
             ++count;
-            return false;
-        });
+        }
         return count;
     }
 
diff --git a/src/value_cache.h b/src/value_cache.h
index dbd08197..eb72d111 100644
--- a/src/value_cache.h
+++ b/src/value_cache.h
@@ -28,7 +28,7 @@ using CallbackQueue = std::list<std::function<void()>>;
 
 class ValueCache {
 public:
-    ValueCache(ValueStateCallback&& cb, SyncCallback&& scb = {})
+    explicit ValueCache(ValueStateCallback&& cb, SyncCallback&& scb = {})
         : callback(std::forward<ValueStateCallback>(cb)), syncCallback(std::move(scb))
     {
         if (syncCallback)
-- 
GitLab