refactor(liblogging): Split vlogf() to internal formatting function

liblogging/CMakeLists.txt

@@ -1,6 +1,16 @@
-list(APPEND SOURCES src/Logger.cpp)
+list(APPEND SOURCES src/Logger.cpp src/Formatting.cpp)
 set(TARGET logging)
 
 add_library(${TARGET} STATIC ${SOURCES})
 target_include_directories(${TARGET} PRIVATE src PUBLIC headers)
 target_link_libraries(${TARGET} spdlog::spdlog)
+
+if(BUILD_TESTS)
+	list(APPEND TEST_SOURCES test/FormattingTest.cpp)
+	set(TEST_TARGET test${TARGET})
+
+	add_executable(${TEST_TARGET} ${TEST_SOURCES})
+	target_link_libraries(${TEST_TARGET} GTest::gtest_main ${TARGET})
+	target_include_directories(${TEST_TARGET} PRIVATE src)
+	gtest_discover_tests(${TEST_TARGET})
+endif()

liblogging/headers/logging/Logger.h

@@ -112,7 +112,6 @@ class Logger {
 private:
 	void loggerSetDefaults();
 	void setLogger(spdlog::logger logger);
-	void logLine(enum LogLevel level, const char* str, size_t len);
 
 	spdlog::logger spdLogger;
 };

liblogging/src/Formatting.cpp

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include "Formatting.h"
+
+namespace logging {
+
+std::string_view trimNewline(std::string_view str) {
+	if (!str.empty() && str.back() == '\n') {
+		return std::string_view(str.data(), str.length() - 1);
+	} else {
+		return str;
+	}
+}
+
+std::string vaFormat(const char* format, va_list args) noexcept {
+	std::string result(256, '\0');
+
+	va_list argsCopy;
+	va_copy(argsCopy, args);
+
+	int len{std::vsnprintf(result.data(), result.size() + 1, format, args)};
+
+	if (len <= 0) {
+		return result;
+	}
+
+	if (len <= (int)result.size()) {
+		result.resize(len);
+		return result;
+	}
+
+	result.resize(len);
+	len = std::vsnprintf(result.data(), result.size() + 1, format, argsCopy);
+
+	if (len <= 0) {
+		return {};
+	}
+
+	return result;
+}
+
+std::string vaFormat(const char* format, ...) noexcept {
+	va_list args;
+	va_start(args, format);
+	auto result{vaFormat(format, args)};
+	va_end(args);
+	return result;
+}
+
+} // namespace logging

liblogging/src/Formatting.h

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: Apache-2.0
+#pragma once
+
+#include <cstdarg>
+#include <string>
+
+namespace logging {
+
+std::string_view trimNewline(std::string_view str);
+
+std::string vaFormat(const char* format, va_list args) noexcept;
+std::string vaFormat(const char* format, ...) noexcept;
+
+} // namespace logging

liblogging/src/Logger.cpp

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 #include "logging/Logger.h"
 
+#include "Formatting.h"
+
 #include <spdlog/common.h>
 #include <spdlog/sinks/rotating_file_sink.h>
 #include <spdlog/sinks/stdout_color_sinks.h>
@@ -86,22 +88,13 @@ void Logger::vlogf(enum LogLevel level, const char* format, va_list args) {
 		return;
 	}
 
-	const int staticBufSize{512};
-	static char staticBuf[staticBufSize]{};
-
-	static va_list argsCopy;
-	va_copy(argsCopy, args);
-
-	int resultSize{vsnprintf(staticBuf, staticBufSize, format, args)};
-	if (resultSize <= staticBufSize) {
-		logLine(level, staticBuf, resultSize);
+	auto formatted{logging::vaFormat(format, args)};
+	if (formatted.empty()) {
 		return;
 	}
 
-	char buf[resultSize]{};
-	vsnprintf(buf, resultSize, format, argsCopy);
-
-	logLine(level, buf, resultSize);
+	formatted = logging::trimNewline(formatted);
+	log(level, "{}", formatted);
 }
 
 void Logger::setLogger(spdlog::logger logger) {
@@ -114,19 +107,4 @@ void Logger::loggerSetDefaults() {
 	spdLogger.set_pattern("%Y-%m-%d %H:%M:%S.%e [%t] %^%l%$ [%n] %v");
 }
 
-void Logger::logLine(enum LogLevel level, const char* buf, size_t len) {
-	if (buf == nullptr || len == 0) {
-		return;
-	}
-
-	std::string str;
-	if (len > 1 && buf[len - 1] == '\n') {
-		str.assign(buf, len - 1);
-	} else {
-		str.assign(buf, len);
-	}
-
-	log(level, "{}", str);
-}
-
 } // namespace logging

liblogging/test/FormattingTest.cpp

@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include "Formatting.h"
+
+#include <gtest/gtest.h>
+
+#include <cstdarg>
+#include <string>
+
+using logging::trimNewline;
+using logging::vaFormat;
+
+TEST(VaFormatTest, testFormat) {
+	const std::string expected{"Hello, World!"};
+	const std::string actual{vaFormat("Hello, %s!", "World")};
+	ASSERT_EQ(actual, expected);
+}
+
+TEST(VaFormatTest, testMultipleFormat) {
+	const std::string expected{"Hello, World! 1 + 2 = 3"};
+	const std::string actual{vaFormat("Hello, %s! %d %c %d %c %d", "World", 1, '+', 2, '=', 3)};
+	ASSERT_EQ(actual, expected);
+}
+
+TEST(VaFormatTest, testLongFormat) {
+	const std::string longString(300, 'Z');
+	const std::string expected{longString + " Hello, World!"};
+	const std::string format{longString + " Hello, %s!"};
+	const std::string actual = vaFormat(format.c_str(), "World");
+	ASSERT_EQ(actual, expected);
+}
+
+TEST(VaFormatTest, test256Format) {
+	const std::string longString(254, 'Z');
+	const std::string expected{longString + "32"};
+	const std::string format{longString + "%d"};
+	const std::string actual = vaFormat(format.c_str(), 32);
+	ASSERT_EQ(actual, expected);
+}
+
+TEST(VaFormatTest, test255Format) {
+	const std::string longString(254, 'Z');
+	const std::string expected{longString + "3"};
+	const std::string format{longString + "%d"};
+	const std::string actual = vaFormat(format.c_str(), 3);
+	ASSERT_EQ(actual, expected);
+}
+
+TEST(FormattingTest, testTrimNewline) {
+	const std::string expected{"Hello, World!"};
+	const auto actual = trimNewline("Hello, World!\0");
+	ASSERT_EQ(actual, expected);
+}