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>
diff --git a/http/http_client.hpp b/http/http_client.hpp
index ac231b4..0bfb381 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 cf90877..4b9eab9 100644
--- a/http/logging.hpp
+++ b/http/logging.hpp
@@ -65,7 +65,7 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 5859d10..b96008d 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>
diff --git a/include/async_resolve.hpp b/include/async_resolve.hpp
index 2d9899d..dbc0cfc 100644
--- a/include/async_resolve.hpp
+++ b/include/async_resolve.hpp
@@ -8,7 +8,6 @@
#include <sdbusplus/message.hpp>
#include <charconv>
-#include <iostream>
#include <memory>
namespace async_resolve
diff --git a/include/ossl_random.hpp b/include/ossl_random.hpp
index 0e28944..4d4bc04 100644
--- a/include/ossl_random.hpp
+++ b/include/ossl_random.hpp
@@ -7,7 +7,6 @@
#include <openssl/rand.h>
}
-#include <iostream>
#include <limits>
#include <string>
diff --git a/redfish-core/include/registries.hpp b/redfish-core/include/registries.hpp
index bad31c4..b0ddb68 100644
--- a/redfish-core/include/registries.hpp
+++ b/redfish-core/include/registries.hpp
@@ -20,7 +20,6 @@
#include <array>
#include <charconv>
#include <cstddef>
-#include <iostream>
#include <numeric>
#include <span>
#include <string>