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>
diff --git a/.clang-tidy b/.clang-tidy
index 5b530c3..0027ca0 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -355,6 +355,7 @@
- { 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 2d03487..5980478 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -790,8 +790,7 @@
{
// 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 49a9cba..2f88c90 100644
--- a/http/logging.hpp
+++ b/http/logging.hpp
@@ -160,19 +160,6 @@
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 @@
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 @@
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 7872b76..a988466 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -303,24 +303,24 @@
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 @@
}
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);
}
}