diff options
author | Ed Tanous <ed@tanous.net> | 2024-04-07 18:38:44 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2024-04-09 01:46:24 +0300 |
commit | 6ea9076048028a4adfdbdd2606698bffc81203d3 (patch) | |
tree | 23ba9504c963534c9354e66cdd61b19a5608eafd | |
parent | 8cb2c024c4625e2fe2f0b107a865faffcd4bb770 (diff) | |
download | bmcweb-6ea9076048028a4adfdbdd2606698bffc81203d3.tar.xz |
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>
-rw-r--r-- | .clang-tidy | 1 | ||||
-rw-r--r-- | http/http_client.hpp | 3 | ||||
-rw-r--r-- | http/logging.hpp | 111 | ||||
-rw-r--r-- | http/routing.hpp | 16 |
4 files changed, 80 insertions, 51 deletions
diff --git a/.clang-tidy b/.clang-tidy index 5b530c3791..0027ca00b2 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -355,6 +355,7 @@ CheckOptions: - { key: readability-identifier-naming.NamespaceCase, value: lower_case } - { key: readability-identifier-naming.StructCase, value: CamelCase } - { key: readability-identifier-naming.FunctionIgnoredRegexp, value: (BMCWEB_LOG_DEBUG|BMCWEB_LOG_INFO|BMCWEB_LOG_WARNING|BMCWEB_LOG_ERROR|BMCWEB_LOG_CRITICAL) } + - { key: readability-identifier-naming.StructIgnoredRegexp, value: (BMCWEB_LOG_DEBUG|BMCWEB_LOG_INFO|BMCWEB_LOG_WARNING|BMCWEB_LOG_ERROR|BMCWEB_LOG_CRITICAL) } - { key: cppcoreguidelines-macro-usage.AllowedRegexp, value: DEBUG*|NLOHMANN_JSON_SERIALIZE_ENUM } - { key: performance-unnecessary-value-param.AllowedTypes, value: ((segments_view)|(url_view)) } - { key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true } 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); } } |