diff options
author | Ed Tanous <ed@tanous.net> | 2024-04-09 03:18:12 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2024-05-07 03:37:41 +0300 |
commit | 17c472452c9a3518aeac636d01b8e2f01c4fa21f (patch) | |
tree | 39dcf835f3ea7836290b7f77c15f48abae46caff /http | |
parent | 8b901d78af166fa3017d930cf121b1dad6edbe0b (diff) | |
download | bmcweb-17c472452c9a3518aeac636d01b8e2f01c4fa21f.tar.xz |
Move logging args
Args captured by logging functions should be captured by rvalue, and use
std::forward to get perfect forwarding. In addition, separate out the
various std::out lines.
While we're here, also try to optimize a little. We should ideally be
writing each log line to the output once, and ideally not use iostreams,
which induce a lot of overhead.
Similar to spdlog[1] (which at one point this codebase used), construct
the string, then call fwrite and fflush once, rather than calling
std::cout repeatedly.
Now that we don't have a dependency on iostreams anymore, we can remove
it from the places where it has snuck in.
Tested:
Logging still functions as before. Logs present.
[1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13
Change-Id: I1dd4739e06eb506d68989a066d122109b71b92cd
Signed-off-by: Ed Tanous <ed@tanous.net>
Diffstat (limited to 'http')
-rw-r--r-- | http/http_client.hpp | 1 | ||||
-rw-r--r-- | http/logging.hpp | 41 | ||||
-rw-r--r-- | http/verb.hpp | 1 |
3 files changed, 32 insertions, 11 deletions
diff --git a/http/http_client.hpp b/http/http_client.hpp index ac231b42e7..0bfb3818df 100644 --- a/http/http_client.hpp +++ b/http/http_client.hpp @@ -44,7 +44,6 @@ #include <cstdlib> #include <functional> -#include <iostream> #include <memory> #include <queue> #include <string> diff --git a/http/logging.hpp b/http/logging.hpp index cf908771bb..4b9eab967c 100644 --- a/http/logging.hpp +++ b/http/logging.hpp @@ -65,7 +65,7 @@ const void* logPtr(T p) } template <LogLevel level, typename... Args> -inline void vlog(std::format_string<Args...> format, Args... args, +inline void vlog(std::format_string<Args...>&& format, Args&&... args, const std::source_location& loc) noexcept { if constexpr (bmcwebCurrentLoggingLevel < level) @@ -78,9 +78,27 @@ inline void vlog(std::format_string<Args...> format, Args... args, constexpr std::string_view levelString = mapLogLevelFromName[stringIndex]; std::string_view filename = loc.file_name(); filename = filename.substr(filename.rfind('/') + 1); - std::cout << std::format("[{} {}:{}] ", levelString, filename, loc.line()) - << std::format(format, std::forward<Args>(args)...) << '\n' - << std::flush; + std::string logLocation; + logLocation = std::format("[{} {}:{}] ", levelString, filename, loc.line()); + try + { + // TODO, multiple static analysis tools flag that this could potentially + // throw Based on the documentation, it shouldn't throw, so long as none + // of the formatters throw, so unclear at this point why this try/catch + // is required, but add it to silence the static analysis tools. + logLocation += std::format(std::move(format), + std::forward<Args>(args)...); + } + catch (const std::format_error& /*error*/) + { + logLocation += "Failed to format"; + // Nothing more we can do here if logging is broken. + } + logLocation += '\n'; + // Intentionally ignore error return. + fwrite(logLocation.data(), sizeof(std::string::value_type), + logLocation.size(), stdout); + fflush(stdout); } } // namespace crow @@ -92,7 +110,8 @@ struct BMCWEB_LOG_CRITICAL const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Critical, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Critical, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -104,7 +123,8 @@ struct BMCWEB_LOG_ERROR const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Error, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Error, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -116,7 +136,8 @@ struct BMCWEB_LOG_WARNING const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Warning, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Warning, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -128,7 +149,8 @@ struct BMCWEB_LOG_INFO const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Info, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Info, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -140,7 +162,8 @@ struct BMCWEB_LOG_DEBUG const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Debug, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Debug, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; diff --git a/http/verb.hpp b/http/verb.hpp index 5859d10b0d..b96008de18 100644 --- a/http/verb.hpp +++ b/http/verb.hpp @@ -2,7 +2,6 @@ #include <boost/beast/http/verb.hpp> -#include <iostream> #include <optional> #include <string_view> |