dht: log cleanup

include/opendht/dht_interface.h

@@ -215,15 +215,16 @@ public:
     /**
      * Enable or disable logging of DHT internal messages
      */
-    virtual void setLoggers(LogMethod error = NOLOG, LogMethod warn = NOLOG, LogMethod debug = NOLOG)
-    {
+    virtual void setLoggers(LogMethod error = {}, LogMethod warn = {}, LogMethod debug = {}) {
         DHT_LOG.DBG = debug;
         DHT_LOG.WARN = warn;
         DHT_LOG.ERR = error;
     }
 
     virtual void setLogger(const Logger& l) {
-        DHT_LOG = l;
+        DHT_LOG.DBG = l.DBG;
+        DHT_LOG.WARN = l.WARN;
+        DHT_LOG.ERR = l.ERR;
     }
 
     /**
@@ -243,8 +244,6 @@ public:
     virtual void pushNotificationReceived(const std::map<std::string, std::string>& data) = 0;
 
 protected:
-    bool logFilerEnable_ {};
-    InfoHash logFiler_ {};
     Logger DHT_LOG;
 };
 

include/opendht/dhtrunner.h

@@ -323,7 +323,7 @@ public:
     std::vector<ValuesExport> exportValues() const;
 
     void setLogger(const Logger& logger = {});
-    void setLoggers(LogMethod err = NOLOG, LogMethod warn = NOLOG, LogMethod debug = NOLOG);
+    void setLoggers(LogMethod err = {}, LogMethod warn = {}, LogMethod debug = {});
 
     /**
      * Only print logs related to the given InfoHash (if given), or disable filter (if zeroes).

include/opendht/log_enable.h

@@ -32,11 +32,6 @@ namespace dht {
 
 // Logging related utility functions
 
-/**
- * Dummy function used to disable logging
- */
-inline void NOLOG(char const*, va_list) {}
-
 /**
  * Wrapper for logging methods
  */
@@ -85,9 +80,10 @@ private:
 };
 
 struct Logger {
-    LogMethod DBG = NOLOG;
-    LogMethod WARN = NOLOG;
-    LogMethod ERR = NOLOG;
+    LogMethod DBG = {};
+    LogMethod WARN = {};
+    LogMethod ERR = {};
+
     Logger() = default;
     Logger(LogMethod&& err, LogMethod&& warn, LogMethod&& dbg)
         : DBG(std::move(dbg)), WARN(std::move(warn)), ERR(std::move(err)) {}

include/opendht/securedht.h

@@ -311,10 +311,9 @@ public:
         dht_->pushNotificationReceived(notification);
     }
 
-    void setLoggers(LogMethod error = NOLOG, LogMethod warn = NOLOG, LogMethod debug = NOLOG) override
-    {
-        DhtInterface::setLoggers(error, warn, debug);
-        dht_->setLoggers(error, warn, debug);
+    void setLogger(const Logger& logger) override {
+        DhtInterface::setLogger(logger);
+        dht_->setLogger(logger);
     }
 
     /**

src/dht.cpp

@@ -1722,9 +1722,7 @@ Dht::Dht(const int& s, const int& s6, const Config& config, const Logger& l)
     search_id = std::uniform_int_distribution<decltype(search_id)>{}(rd);
 
     uniform_duration_distribution<> time_dis {std::chrono::seconds(3), std::chrono::seconds(5)};
-    auto confirm_nodes_time = scheduler.time() + time_dis(rd);
-    DHT_LOG.d(myid, "Scheduling %s", myid.toString().c_str());
-    nextNodesConfirmation = scheduler.add(confirm_nodes_time, std::bind(&Dht::confirmNodes, this));
+    nextNodesConfirmation = scheduler.add(scheduler.time() + time_dis(rd), std::bind(&Dht::confirmNodes, this));
 
     // Fill old secret
     {
@@ -1735,7 +1733,7 @@ Dht::Dht(const int& s, const int& s6, const Config& config, const Logger& l)
 
     expire();
 
-    DHT_LOG.d("DHT initialised with node ID %s", myid.toString().c_str());
+    DHT_LOG.d("DHT node initialised with ID %s", myid.toString().c_str());
 
     if (not persistPath.empty())
         loadState(persistPath);

src/log.cpp

@@ -129,7 +129,7 @@ enableSyslog(dht::DhtRunner &dht, const char* name) {
 
 void
 disableLogging(dht::DhtRunner &dht) {
-    dht.setLoggers(dht::NOLOG, dht::NOLOG, dht::NOLOG);
+    dht.setLogger({});
 }
 
 }

src/securedht.cpp

@@ -54,9 +54,9 @@ SecureDht::SecureDht(std::unique_ptr<DhtInterface> dht, SecureDht::Config conf)
             CERTIFICATE_TYPE,
             *certificate_,
             1
-        }, [this](bool ok) {
+        }, [this, certId](bool ok) {
             if (ok)
-                DHT_LOG.DBG("SecureDht: public key announced successfully");
+                DHT_LOG.d(certId, "SecureDht: public key announced successfully");
         }, {}, true);
     }
 }
@@ -73,7 +73,7 @@ SecureDht::secureType(ValueType&& type)
                 v->signatureValid = v->owner and v->owner->checkSignature(v->getToSign(), v->signature);
             }
             if (!v->signatureValid) {
-                DHT_LOG.WARN("Signature verification failed");
+                DHT_LOG.w("Signature verification failed");
                 return false;
             }
         }
@@ -83,7 +83,7 @@ SecureDht::secureType(ValueType&& type)
         if (!o->isSigned())
             return type.editPolicy(id, o, n, nid, a);
         if (o->owner != n->owner) {
-            DHT_LOG.WARN("Edition forbidden: owner changed.");
+            DHT_LOG.w("Edition forbidden: owner changed.");
             return false;
         }
         if (!n->signatureChecked) {
@@ -91,14 +91,14 @@ SecureDht::secureType(ValueType&& type)
             n->signatureValid = o->owner and o->owner->checkSignature(n->getToSign(), n->signature);
         }
         if (!n->signatureValid) {
-            DHT_LOG.WARN("Edition forbidden: signature verification failed.");
+            DHT_LOG.w("Edition forbidden: signature verification failed.");
             return false;
         }
         if (o->seq == n->seq) {
             // If the data is exactly the same,
             // it can be reannounced, possibly by someone else.
             if (o->getToSign() != n->getToSign()) {
-                DHT_LOG.WARN("Edition forbidden: sequence number must be increasing.");
+                DHT_LOG.w("Edition forbidden: sequence number must be increasing.");
                 return false;
             }
         }
@@ -144,7 +144,7 @@ SecureDht::registerCertificate(const InfoHash& node, const Blob& data)
     }
     InfoHash h = crt->getPublicKey().getId();
     if (node == h) {
-        DHT_LOG.DBG("Registering certificate for %s", h.toString().c_str());
+        DHT_LOG.d("Registering certificate for %s", h.toString().c_str());
         auto it = nodesCertificates_.find(h);
         if (it == nodesCertificates_.end())
             std::tie(it, std::ignore) = nodesCertificates_.emplace(h, std::move(crt));
@@ -152,7 +152,7 @@ SecureDht::registerCertificate(const InfoHash& node, const Blob& data)
             it->second = std::move(crt);
         return it->second;
     } else {
-        DHT_LOG.WARN("Certificate %s for node %s does not match node id !", h.toString().c_str(), node.toString().c_str());
+        DHT_LOG.w("Certificate %s for node %s does not match node id !", h.toString().c_str(), node.toString().c_str());
         return nullptr;
     }
 }
@@ -169,7 +169,7 @@ SecureDht::findCertificate(const InfoHash& node, const std::function<void(const
 {
     Sp<crypto::Certificate> b = getCertificate(node);
     if (b && *b) {
-        DHT_LOG.DBG("Using certificate from cache for %s", node.toString().c_str());
+        DHT_LOG.d("Using certificate from cache for %s", node.toString().c_str());
         if (cb)
             cb(b);
         return;
@@ -177,7 +177,7 @@ SecureDht::findCertificate(const InfoHash& node, const std::function<void(const
     if (localQueryMethod_) {
         auto res = localQueryMethod_(node);
         if (not res.empty()) {
-            DHT_LOG.DBG("Registering certificate from local store for %s", node.toString().c_str());
+            DHT_LOG.d("Registering certificate from local store for %s", node.toString().c_str());
             nodesCertificates_.emplace(node, res.front());
             if (cb)
                 cb(res.front());
@@ -192,7 +192,7 @@ SecureDht::findCertificate(const InfoHash& node, const std::function<void(const
         for (const auto& v : vals) {
             if (auto cert = registerCertificate(node, v->data)) {
                 *found = true;
-                DHT_LOG.DBG("Found certificate for %s", node.toString().c_str());
+                DHT_LOG.d("Found certificate for %s", node.toString().c_str());
                 if (cb)
                     cb(cert);
                 return false;
@@ -210,7 +210,7 @@ SecureDht::findPublicKey(const InfoHash& node, const std::function<void(const Sp
 {
     auto pk = getPublicKey(node);
     if (pk && *pk) {
-        DHT_LOG.DBG("Found public key from cache for %s", node.toString().c_str());
+        DHT_LOG.d("Found public key from cache for %s", node.toString().c_str());
         if (cb)
             cb(pk);
         return;
@@ -254,7 +254,7 @@ SecureDht::checkValue(const Sp<Value>& v)
             }
             // Ignore values belonging to other people
         } catch (const std::exception& e) {
-            DHT_LOG.WARN("Could not decrypt value %s : %s", v->toString().c_str(), e.what());
+            DHT_LOG.w("Could not decrypt value %s : %s", v->toString().c_str(), e.what());
         }
     }
     // Check signed values
@@ -269,7 +269,7 @@ SecureDht::checkValue(const Sp<Value>& v)
             return v;
         }
         else
-            DHT_LOG.WARN("Signature verification failed for %s", v->toString().c_str());
+            DHT_LOG.w("Signature verification failed for %s", v->toString().c_str());
     }
     // Forward normal values
     else {
@@ -352,12 +352,12 @@ SecureDht::putSigned(const InfoHash& hash, Sp<Value> val, DoneCallback callback,
     // Check if data already exists on the dht
     get(hash,
         [val,this] (const std::vector<Sp<Value>>& vals) {
-            DHT_LOG.DBG("Found online previous value being announced.");
+            DHT_LOG.d("Found online previous value being announced.");
             for (const auto& v : vals) {
                 if (!v->isSigned())
-                    DHT_LOG.ERR("Existing non-signed value seems to exists at this location.");
+                    DHT_LOG.e("Existing non-signed value seems to exists at this location.");
                 else if (not v->owner or v->owner->getId() != getId())
-                    DHT_LOG.ERR("Existing signed value belonging to someone else seems to exists at this location.");
+                    DHT_LOG.e("Existing signed value belonging to someone else seems to exists at this location.");
                 else if (val->seq <= v->seq)
                     val->seq = v->seq + 1;
             }
@@ -381,11 +381,11 @@ SecureDht::putEncrypted(const InfoHash& hash, const InfoHash& to, Sp<Value> val,
                 callback(false, {});
             return;
         }
-        DHT_LOG.WARN("Encrypting data for PK: %s", pk->getId().toString().c_str());
+        DHT_LOG.w("Encrypting data for PK: %s", pk->getId().toString().c_str());
         try {
             dht_->put(hash, encrypt(*val, *pk), callback, time_point::max(), permanent);
         } catch (const std::exception& e) {
-            DHT_LOG.ERR("Error putting encrypted data: %s", e.what());
+            DHT_LOG.e("Error putting encrypted data: %s", e.what());
             if (callback)
                 callback(false, {});
         }

tools/dhtnode.cpp

@@ -205,7 +205,7 @@ void cmd_loop(std::shared_ptr<DhtRunner>& node, dht_params& params
         } else if (op == "log") {
             iss >> idstr;
             InfoHash filter(idstr);
-            params.log = filter == InfoHash{} ? !params.log : true;
+            params.log = filter ? true : !params.log;
             if (params.log)
                 log::enableLogging(*node);
             else