From 1d3b1babf42e5d3156305aa92a3ea235980c3c77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Thu, 18 May 2023 01:07:32 -0400
Subject: [PATCH] logger: remove OPENDHT_LOG, rename to logger.h

---
 CMakeLists.txt                             |  8 +--
 configure.ac                               |  6 --
 include/opendht/dht_interface.h            |  2 +-
 include/opendht/dhtrunner.h                |  2 +-
 include/opendht/log.h                      |  2 +-
 include/opendht/{log_enable.h => logger.h} | 73 +++++-----------------
 include/opendht/network_engine.h           |  2 +-
 include/opendht/network_utils.h            |  2 +-
 include/opendht/peer_discovery.h           |  2 +-
 src/Makefile.am                            |  2 +-
 src/compat/os_cert.cpp                     |  2 +-
 src/compat/os_cert.h                       |  2 +-
 src/dht.cpp                                |  4 +-
 src/http.cpp                               |  6 +-
 src/network_engine.cpp                     |  2 +-
 15 files changed, 33 insertions(+), 84 deletions(-)
 rename include/opendht/{log_enable.h => logger.h} (76%)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5ab668ad..fc3f47f4 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -23,7 +23,6 @@ set (VERSION "${opendht_VERSION}")
 # Options
 option (BUILD_SHARED_LIBS "Build shared library" ON)
 CMAKE_DEPENDENT_OPTION (OPENDHT_STATIC "Build static library" OFF BUILD_SHARED_LIBS ON)
-option (OPENDHT_LOG "Build with logs" ON)
 option (OPENDHT_PYTHON "Build Python bindings" OFF)
 option (OPENDHT_TOOLS "Build DHT tools" ON)
 option (OPENDHT_SYSTEMD "Install systemd module" OFF)
@@ -155,11 +154,6 @@ endif ()
 set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DMSGPACK_NO_BOOST -DMSGPACK_DISABLE_LEGACY_NIL -DMSGPACK_DISABLE_LEGACY_CONVERT")
 
 add_definitions(-DPACKAGE_VERSION="${opendht_VERSION}")
-if (OPENDHT_LOG)
-    add_definitions(-DOPENDHT_LOG=true)
-else ()
-    add_definitions(-DOPENDHT_LOG=false)
-endif()
 
 if (ASIO_INCLUDE_DIR)
     include_directories (SYSTEM "${ASIO_INCLUDE_DIR}")
@@ -232,7 +226,7 @@ list (APPEND opendht_HEADERS
     include/opendht/rate_limiter.h
     include/opendht/securedht.h
     include/opendht/log.h
-    include/opendht/log_enable.h
+    include/opendht/logger.h
     include/opendht/thread_pool.h
     include/opendht/network_utils.h
     include/opendht.h
diff --git a/configure.ac b/configure.ac
index 7fecac15..35cc267d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -34,12 +34,6 @@ AC_LANG(C++)
 AC_PROG_CXX
 AX_CXX_COMPILE_STDCXX(17,[noext],[mandatory])
 
-dnl Check for logs
-AC_ARG_ENABLE([logs], [AS_HELP_STRING([--disable-logs], [Disable DHT logs])])
-AS_IF([test "x$enable_logs" != "xno"], [], [
-	AC_DEFINE([OPENDHT_LOG], [false], [Define if DHT logs are disabled])
-])
-
 dnl Check for C binding
 AC_ARG_ENABLE([c], [AS_HELP_STRING([--disable-c], [Disable DHT C binding])])
 AM_CONDITIONAL(ENABLE_C, test x$enable_c != "xno")
diff --git a/include/opendht/dht_interface.h b/include/opendht/dht_interface.h
index 9a9bfcb5..25faaa8c 100644
--- a/include/opendht/dht_interface.h
+++ b/include/opendht/dht_interface.h
@@ -19,7 +19,7 @@
 #pragma once
 
 #include "infohash.h"
-#include "log_enable.h"
+#include "logger.h"
 #include "node_export.h"
 
 #include <queue>
diff --git a/include/opendht/dhtrunner.h b/include/opendht/dhtrunner.h
index 9f840931..948cef34 100644
--- a/include/opendht/dhtrunner.h
+++ b/include/opendht/dhtrunner.h
@@ -25,7 +25,7 @@
 #include "value.h"
 #include "callbacks.h"
 #include "sockaddr.h"
-#include "log_enable.h"
+#include "logger.h"
 #include "network_utils.h"
 #include "node_export.h"
 
diff --git a/include/opendht/log.h b/include/opendht/log.h
index af7165f2..170190bf 100644
--- a/include/opendht/log.h
+++ b/include/opendht/log.h
@@ -20,7 +20,7 @@
 #pragma once
 
 #include "def.h"
-#include "log_enable.h"
+#include "logger.h"
 
 #include <iostream>
 #include <memory>
diff --git a/include/opendht/log_enable.h b/include/opendht/logger.h
similarity index 76%
rename from include/opendht/log_enable.h
rename to include/opendht/logger.h
index d3f5334b..09af78c7 100644
--- a/include/opendht/log_enable.h
+++ b/include/opendht/logger.h
@@ -18,23 +18,15 @@
 
 #pragma once
 
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
 #include "infohash.h"
 
+#include <fmt/format.h>
+#include <fmt/printf.h>
+
 #include <functional>
 #include <string_view>
 #include <cstdarg>
 
-#ifndef OPENDHT_LOG
-#define OPENDHT_LOG true
-#endif
-
-#include <fmt/format.h>
-#include <fmt/printf.h>
-
 namespace dht {
 namespace log {
 
@@ -51,104 +43,73 @@ struct Logger {
 
     Logger() = default;
     Logger(LogMethodFmt&& logger, LogMethodPrintf&& loggerf)
-        : logger(std::move(logger)), loggerf(std::move(loggerf)) {}
+        : logger(std::move(logger)), loggerf(std::move(loggerf)) {
+            if (!logger or !loggerf)
+                throw std::invalid_argument{"logger and loggerf must be set"};
+        }
     void setFilter(const InfoHash& f) {
         filter_ = f;
         filterEnable_ = static_cast<bool>(filter_);
     }
-    inline void logFmt(LogLevel level, fmt::string_view format, fmt::format_args args) const {
-        if (logger and not filterEnable_)
-            logger(level, format, args);
-    }
     inline void log0(LogLevel level, fmt::string_view format, fmt::printf_args args) const {
-#if OPENDHT_LOG
-        if (loggerf and not filterEnable_)
+        if (not filterEnable_)
             loggerf(level, format, args);
-#endif
     }
     inline void log1(LogLevel level, const InfoHash& f, fmt::string_view format, fmt::printf_args args) const {
-#if OPENDHT_LOG
-        if (loggerf and (not filterEnable_ or f == filter_))
+        if (not filterEnable_ or f == filter_)
             loggerf(level, format, args);
-#endif
     }
     inline void log2(LogLevel level, const InfoHash& f1, const InfoHash& f2, fmt::string_view format, fmt::printf_args args) const {
-#if OPENDHT_LOG
-        if (loggerf and (not filterEnable_ or f1 == filter_ or f2 == filter_))
+        if (not filterEnable_ or f1 == filter_ or f2 == filter_)
             loggerf(level, format, args);
-#endif
     }
     template<typename S, typename... Args>
     inline void debug(S&& format, Args&&... args) const {
-#if OPENDHT_LOG
-        logFmt(LogLevel::debug, std::forward<S>(format), fmt::make_format_args(args...));
-#endif
+        logger(LogLevel::debug, std::forward<S>(format), fmt::make_format_args(args...));
     }
-    template <typename... T>
-    inline void warn(fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
-        logFmt(LogLevel::warning, format, fmt::make_format_args(args...));
-#endif
+    template<typename S, typename... Args>
+    inline void warn(S&& format, Args&&... args) const {
+        logger(LogLevel::warning, std::forward<S>(format), fmt::make_format_args(args...));
     }
-    template <typename... T>
-    inline void error(fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
-        logFmt(LogLevel::error, format, fmt::make_format_args(args...));
-#endif
+    template<typename S, typename... Args>
+    inline void error(S&& format, Args&&... args) const {
+        logger(LogLevel::error, std::forward<S>(format), fmt::make_format_args(args...));
     }
     template <typename... T>
     inline void d(fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log0(LogLevel::debug, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void d(const InfoHash& f, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log1(LogLevel::debug, f, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void d(const InfoHash& f1, const InfoHash& f2, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log2(LogLevel::debug, f1, f2, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void w(fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log0(LogLevel::warning, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void w(const InfoHash& f, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log1(LogLevel::warning, f, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void w(const InfoHash& f1, const InfoHash& f2, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log2(LogLevel::warning, f1, f2, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void e(fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log0(LogLevel::error, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void e(const InfoHash& f, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log1(LogLevel::error, f, format, fmt::make_printf_args(args...));
-#endif
     }
     template <typename... T>
     inline void e(const InfoHash& f1, const InfoHash& f2, fmt::format_string<T...> format, T&&... args) const {
-#if OPENDHT_LOG
         log2(LogLevel::error, f1, f2, format, fmt::make_printf_args(args...));
-#endif
     }
 private:
     bool filterEnable_ {false};
diff --git a/include/opendht/network_engine.h b/include/opendht/network_engine.h
index 94dbdb91..802a8d74 100644
--- a/include/opendht/network_engine.h
+++ b/include/opendht/network_engine.h
@@ -27,7 +27,7 @@
 #include "utils.h"
 #include "rng.h"
 #include "rate_limiter.h"
-#include "log_enable.h"
+#include "logger.h"
 #include "network_utils.h"
 
 #include <vector>
diff --git a/include/opendht/network_utils.h b/include/opendht/network_utils.h
index 238b876f..4ffbd066 100644
--- a/include/opendht/network_utils.h
+++ b/include/opendht/network_utils.h
@@ -21,7 +21,7 @@
 
 #include "sockaddr.h"
 #include "utils.h"
-#include "log_enable.h"
+#include "logger.h"
 
 #ifdef _WIN32
 #include <ws2tcpip.h>
diff --git a/include/opendht/peer_discovery.h b/include/opendht/peer_discovery.h
index 610e444f..0c63e144 100644
--- a/include/opendht/peer_discovery.h
+++ b/include/opendht/peer_discovery.h
@@ -22,7 +22,7 @@
 #include "def.h"
 #include "sockaddr.h"
 #include "infohash.h"
-#include "log_enable.h"
+#include "logger.h"
 
 #include <thread>
 
diff --git a/src/Makefile.am b/src/Makefile.am
index 7bfc015a..43afdde2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -54,7 +54,7 @@ nobase_include_HEADERS = \
         ../include/opendht/dhtrunner.h \
         ../include/opendht/default_types.h \
         ../include/opendht/log.h \
-        ../include/opendht/log_enable.h \
+        ../include/opendht/logger.h \
         ../include/opendht/network_utils.h \
         ../include/opendht/rng.h \
         ../include/opendht/thread_pool.h
diff --git a/src/compat/os_cert.cpp b/src/compat/os_cert.cpp
index ea89f12a..11163a2a 100644
--- a/src/compat/os_cert.cpp
+++ b/src/compat/os_cert.cpp
@@ -18,7 +18,7 @@
 
 #include "os_cert.h"
 
-#include "log_enable.h"
+#include "logger.h"
 
 #ifdef _WIN32
 #include <windows.h>
diff --git a/src/compat/os_cert.h b/src/compat/os_cert.h
index f20bd729..43d9d75d 100644
--- a/src/compat/os_cert.h
+++ b/src/compat/os_cert.h
@@ -18,7 +18,7 @@
 
 #pragma once
 
-#include "log_enable.h"
+#include "logger.h"
 
 #include <memory>
 
diff --git a/src/dht.cpp b/src/dht.cpp
index 9eb1d271..6c6c0c7a 100644
--- a/src/dht.cpp
+++ b/src/dht.cpp
@@ -1840,7 +1840,7 @@ Dht::Dht(std::unique_ptr<net::DatagramSocket>&& sock, const Config& config, cons
     expire();
 
     if (logger_)
-        logger_->d("DHT node initialised with ID %s", myid.toString().c_str());
+        logger_->debug("DHT node initialised with ID {:s}", myid);
 }
 
 bool
@@ -2232,7 +2232,7 @@ Dht::pingNode(SockAddr sa, DoneCallbackSimple&& cb)
 {
     scheduler.syncTime();
     if (logger_)
-        logger_->d("Sending ping to %s", sa.toString().c_str());
+        logger_->debug("Sending ping to {}", sa);
     auto& count = dht(sa.getFamily()).pending_pings;
     count++;
     network_engine.sendPing(std::move(sa), [&count,cb](const net::Request&, net::RequestAnswer&&) {
diff --git a/src/http.cpp b/src/http.cpp
index d7e33bb0..a34005d7 100644
--- a/src/http.cpp
+++ b/src/http.cpp
@@ -17,7 +17,7 @@
  */
 
 #include "http.h"
-#include "log_enable.h"
+#include "logger.h"
 #include "crypto.h"
 #include "base64.h"
 #include "compat/os_cert.h"
@@ -111,11 +111,11 @@ newTlsClientContext(const std::shared_ptr<dht::Logger>& logger)
 
     if (char* path = getenv("CA_ROOT_FILE")) {
         if (logger)
-            logger->d("Using CA file: %s", path);
+            logger->debug("Using CA file: {}", path);
         ctx->load_verify_file(path);
     } else if (char* path = getenv("CA_ROOT_PATH")) {
         if (logger)
-            logger->d("Using CA path: %s", path);
+            logger->debug("Using CA path: {}", path);
         ctx->add_verify_path(path);
     } else {
 #ifdef __ANDROID__
diff --git a/src/network_engine.cpp b/src/network_engine.cpp
index a92915f8..c641b5db 100644
--- a/src/network_engine.cpp
+++ b/src/network_engine.cpp
@@ -20,7 +20,7 @@
 #include "network_engine.h"
 #include "request.h"
 #include "default_types.h"
-#include "log_enable.h"
+#include "logger.h"
 #include "parsed_message.h"
 
 #include <msgpack.hpp>
-- 
GitLab