Rework logger to create compile time errors

The logger changes to move to std::format incidentally caused format
errors to no longer be flagged at compile time.

The example here[1] shows that the latest gcc/c++ gave us a way to solve
this, using std::format_string.

This incidentally shows two places where we got the format arguments
wrong, so fix them.

[1] https://stackoverflow.com/questions/72795189/how-can-i-wrap-stdformat-with-my-own-template-function

Change-Id: Id884200e2c98eeaf5ef8db6c1d6362ede2ffb858
Signed-off-by: Ed Tanous <ed@tanous.net>

3 files changed, 79 insertions, 51 deletions

diff --git a/http/http_client.hpp b/http/http_client.hpp
index 2d03487e84..59804789ee 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -790,8 +790,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool>
         {
             // If we can't buffer the request then we should let the
             // callback handle a 429 Too Many Requests dummy response
-            BMCWEB_LOG_ERROR("{}:{} request queue full.  Dropping request.",
-                             id);
+            BMCWEB_LOG_ERROR("{} request queue full.  Dropping request.", id);
             Response dummyRes;
             dummyRes.result(boost::beast::http::status::too_many_requests);
             resHandler(dummyRes);
diff --git a/http/logging.hpp b/http/logging.hpp
index 49a9cbaaf0..2f88c90b14 100644
--- a/http/logging.hpp
+++ b/http/logging.hpp
@@ -160,19 +160,6 @@ constexpr crow::LogLevel getLogLevelFromName(std::string_view name)
 constexpr crow::LogLevel bmcwebCurrentLoggingLevel =
     getLogLevelFromName(bmcwebLoggingLevel);
 
-struct FormatString
-{
-    std::string_view str;
-    std::source_location loc;
-
-    // NOLINTNEXTLINE(google-explicit-constructor)
-    FormatString(const char* stringIn, const std::source_location& locIn =
-                                           std::source_location::current()) :
-        str(stringIn),
-        loc(locIn)
-    {}
-};
-
 template <typename T>
 const void* logPtr(T p)
 {
@@ -181,8 +168,9 @@ const void* logPtr(T p)
     return std::bit_cast<const void*>(p);
 }
 
-template <LogLevel level>
-inline void vlog(const FormatString& format, const std::format_args& args)
+template <LogLevel level, typename... Args>
+inline void vlog(std::format_string<Args...> format, Args... args,
+                 const std::source_location& loc) noexcept
 {
     if constexpr (bmcwebCurrentLoggingLevel < level)
     {
@@ -192,47 +180,90 @@ inline void vlog(const FormatString& format, const std::format_args& args)
     static_assert(stringIndex < mapLogLevelFromName.size(),
                   "Missing string for level");
     constexpr std::string_view levelString = mapLogLevelFromName[stringIndex];
-    std::string_view filename = format.loc.file_name();
+    std::string_view filename = loc.file_name();
     filename = filename.substr(filename.rfind('/') + 1);
-    std::cout << std::format("[{} {}:{}] ", levelString, filename,
-                             format.loc.line())
-              << std::vformat(format.str, args) << '\n'
+    std::cout << std::format("[{} {}:{}] ", levelString, filename, loc.line())
+              << std::format(format, std::forward<Args>(args)...) << '\n'
               << std::flush;
 }
 } // namespace crow
 
 template <typename... Args>
-inline void BMCWEB_LOG_CRITICAL(const crow::FormatString& format,
-                                Args&&... args)
+struct BMCWEB_LOG_CRITICAL
 {
-    crow::vlog<crow::LogLevel::Critical>(
-        format, std::make_format_args(std::forward<Args>(args)...));
-}
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    BMCWEB_LOG_CRITICAL(std::format_string<Args...> format, Args&&... args,
+                        const std::source_location& loc =
+                            std::source_location::current()) noexcept
+    {
+        crow::vlog<crow::LogLevel::Critical, Args...>(format, args..., loc);
+    }
+};
 
 template <typename... Args>
-inline void BMCWEB_LOG_ERROR(const crow::FormatString& format, Args&&... args)
+struct BMCWEB_LOG_ERROR
 {
-    crow::vlog<crow::LogLevel::Error>(
-        format, std::make_format_args(std::forward<Args>(args)...));
-}
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    BMCWEB_LOG_ERROR(std::format_string<Args...> format, Args&&... args,
+                     const std::source_location& loc =
+                         std::source_location::current()) noexcept
+    {
+        crow::vlog<crow::LogLevel::Error, Args...>(format, args..., loc);
+    }
+};
 
 template <typename... Args>
-inline void BMCWEB_LOG_WARNING(const crow::FormatString& format, Args&&... args)
+struct BMCWEB_LOG_WARNING
 {
-    crow::vlog<crow::LogLevel::Warning>(
-        format, std::make_format_args(std::forward<Args>(args)...));
-}
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    BMCWEB_LOG_WARNING(std::format_string<Args...> format, Args&&... args,
+                       const std::source_location& loc =
+                           std::source_location::current()) noexcept
+    {
+        crow::vlog<crow::LogLevel::Warning, Args...>(format, args..., loc);
+    }
+};
 
 template <typename... Args>
-inline void BMCWEB_LOG_INFO(const crow::FormatString& format, Args&&... args)
+struct BMCWEB_LOG_INFO
 {
-    crow::vlog<crow::LogLevel::Info>(
-        format, std::make_format_args(std::forward<Args>(args)...));
-}
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    BMCWEB_LOG_INFO(std::format_string<Args...> format, Args&&... args,
+                    const std::source_location& loc =
+                        std::source_location::current()) noexcept
+    {
+        crow::vlog<crow::LogLevel::Info, Args...>(format, args..., loc);
+    }
+};
 
 template <typename... Args>
-inline void BMCWEB_LOG_DEBUG(const crow::FormatString& format, Args&&... args)
+struct BMCWEB_LOG_DEBUG
 {
-    crow::vlog<crow::LogLevel::Debug>(
-        format, std::make_format_args(std::forward<Args>(args)...));
-}
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    BMCWEB_LOG_DEBUG(std::format_string<Args...> format, Args&&... args,
+                     const std::source_location& loc =
+                         std::source_location::current()) noexcept
+    {
+        crow::vlog<crow::LogLevel::Debug, Args...>(format, args..., loc);
+    }
+};
+
+template <typename... Args>
+BMCWEB_LOG_CRITICAL(std::format_string<Args...>, Args&&...)
+    -> BMCWEB_LOG_CRITICAL<Args...>;
+
+template <typename... Args>
+BMCWEB_LOG_ERROR(std::format_string<Args...>, Args&&...)
+    -> BMCWEB_LOG_ERROR<Args...>;
+
+template <typename... Args>
+BMCWEB_LOG_WARNING(std::format_string<Args...>, Args&&...)
+    -> BMCWEB_LOG_WARNING<Args...>;
+
+template <typename... Args>
+BMCWEB_LOG_INFO(std::format_string<Args...>, Args&&...)
+    -> BMCWEB_LOG_INFO<Args...>;
+
+template <typename... Args>
+BMCWEB_LOG_DEBUG(std::format_string<Args...>, Args&&...)
+    -> BMCWEB_LOG_DEBUG<Args...>;
diff --git a/http/routing.hpp b/http/routing.hpp
index 7872b76cb5..a9884667d0 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -303,24 +303,24 @@ class Trie
   private:
     void debugNodePrint(Node* n, size_t level)
     {
+        std::string indent(2U * level, ' ');
         for (size_t i = 0; i < static_cast<size_t>(ParamType::MAX); i++)
         {
             if (n->paramChildrens[i] != 0U)
             {
-                BMCWEB_LOG_DEBUG(
-                    "{}({}{}",
-                    std::string(2U * level,
-                                ' ') /*, n->paramChildrens[i], ") "*/);
                 switch (static_cast<ParamType>(i))
                 {
                     case ParamType::STRING:
-                        BMCWEB_LOG_DEBUG("<str>");
+                        BMCWEB_LOG_DEBUG("{}({}) <str>", indent,
+                                         n->paramChildrens[i]);
                         break;
                     case ParamType::PATH:
+                        BMCWEB_LOG_DEBUG("{}({}) <path>", indent,
+                                         n->paramChildrens[i]);
                         BMCWEB_LOG_DEBUG("<path>");
                         break;
                     default:
-                        BMCWEB_LOG_DEBUG("<ERROR>");
+                        BMCWEB_LOG_DEBUG("{}<ERROR>", indent);
                         break;
                 }
 
@@ -329,9 +329,7 @@ class Trie
         }
         for (const Node::ChildMap::value_type& kv : n->children)
         {
-            BMCWEB_LOG_DEBUG("{}({}{}{}",
-                             std::string(2U * level, ' ') /*, kv.second, ") "*/,
-                             kv.first);
+            BMCWEB_LOG_DEBUG("{}({}{}) ", indent, kv.second, kv.first);
             debugNodePrint(&nodes[kv.second], level + 1);
         }
     }