From 6ea9076048028a4adfdbdd2606698bffc81203d3 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Sun, 7 Apr 2024 08:38:44 -0700 Subject: 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 --- http/http_client.hpp | 3 +- http/logging.hpp | 111 ++++++++++++++++++++++++++++++++------------------- http/routing.hpp | 16 ++++---- 3 files changed, 79 insertions(+), 51 deletions(-) (limited to 'http') 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 { // 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 const void* logPtr(T p) { @@ -181,8 +168,9 @@ const void* logPtr(T p) return std::bit_cast(p); } -template -inline void vlog(const FormatString& format, const std::format_args& args) +template +inline void vlog(std::format_string 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)...) << '\n' << std::flush; } } // namespace crow template -inline void BMCWEB_LOG_CRITICAL(const crow::FormatString& format, - Args&&... args) +struct BMCWEB_LOG_CRITICAL { - crow::vlog( - format, std::make_format_args(std::forward(args)...)); -} + // NOLINTNEXTLINE(google-explicit-constructor) + BMCWEB_LOG_CRITICAL(std::format_string format, Args&&... args, + const std::source_location& loc = + std::source_location::current()) noexcept + { + crow::vlog(format, args..., loc); + } +}; template -inline void BMCWEB_LOG_ERROR(const crow::FormatString& format, Args&&... args) +struct BMCWEB_LOG_ERROR { - crow::vlog( - format, std::make_format_args(std::forward(args)...)); -} + // NOLINTNEXTLINE(google-explicit-constructor) + BMCWEB_LOG_ERROR(std::format_string format, Args&&... args, + const std::source_location& loc = + std::source_location::current()) noexcept + { + crow::vlog(format, args..., loc); + } +}; template -inline void BMCWEB_LOG_WARNING(const crow::FormatString& format, Args&&... args) +struct BMCWEB_LOG_WARNING { - crow::vlog( - format, std::make_format_args(std::forward(args)...)); -} + // NOLINTNEXTLINE(google-explicit-constructor) + BMCWEB_LOG_WARNING(std::format_string format, Args&&... args, + const std::source_location& loc = + std::source_location::current()) noexcept + { + crow::vlog(format, args..., loc); + } +}; template -inline void BMCWEB_LOG_INFO(const crow::FormatString& format, Args&&... args) +struct BMCWEB_LOG_INFO { - crow::vlog( - format, std::make_format_args(std::forward(args)...)); -} + // NOLINTNEXTLINE(google-explicit-constructor) + BMCWEB_LOG_INFO(std::format_string format, Args&&... args, + const std::source_location& loc = + std::source_location::current()) noexcept + { + crow::vlog(format, args..., loc); + } +}; template -inline void BMCWEB_LOG_DEBUG(const crow::FormatString& format, Args&&... args) +struct BMCWEB_LOG_DEBUG { - crow::vlog( - format, std::make_format_args(std::forward(args)...)); -} + // NOLINTNEXTLINE(google-explicit-constructor) + BMCWEB_LOG_DEBUG(std::format_string format, Args&&... args, + const std::source_location& loc = + std::source_location::current()) noexcept + { + crow::vlog(format, args..., loc); + } +}; + +template +BMCWEB_LOG_CRITICAL(std::format_string, Args&&...) + -> BMCWEB_LOG_CRITICAL; + +template +BMCWEB_LOG_ERROR(std::format_string, Args&&...) + -> BMCWEB_LOG_ERROR; + +template +BMCWEB_LOG_WARNING(std::format_string, Args&&...) + -> BMCWEB_LOG_WARNING; + +template +BMCWEB_LOG_INFO(std::format_string, Args&&...) + -> BMCWEB_LOG_INFO; + +template +BMCWEB_LOG_DEBUG(std::format_string, Args&&...) + -> BMCWEB_LOG_DEBUG; 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(ParamType::MAX); i++) { if (n->paramChildrens[i] != 0U) { - BMCWEB_LOG_DEBUG( - "{}({}{}", - std::string(2U * level, - ' ') /*, n->paramChildrens[i], ") "*/); switch (static_cast(i)) { case ParamType::STRING: - BMCWEB_LOG_DEBUG(""); + BMCWEB_LOG_DEBUG("{}({}) ", indent, + n->paramChildrens[i]); break; case ParamType::PATH: + BMCWEB_LOG_DEBUG("{}({}) ", indent, + n->paramChildrens[i]); BMCWEB_LOG_DEBUG(""); break; default: - BMCWEB_LOG_DEBUG(""); + BMCWEB_LOG_DEBUG("{}", 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); } } -- cgit v1.2.3