Merge pull request #3533 from daniellimws/fmtlib-macros

Logging: Add fmtlib-based macros

src/citra/citra.cpp

@@ -25,6 +25,7 @@
 
 #include "citra/config.h"
 #include "citra/emu_window/emu_window_sdl2.h"
+#include "common/common_paths.h"
 #include "common/file_util.h"
 #include "common/logging/backend.h"
 #include "common/logging/filter.h"
@@ -67,39 +68,39 @@ static void PrintVersion() {
 static void OnStateChanged(const Network::RoomMember::State& state) {
     switch (state) {
     case Network::RoomMember::State::Idle:
-        LOG_DEBUG(Network, "Network is idle");
+        NGLOG_DEBUG(Network, "Network is idle");
         break;
     case Network::RoomMember::State::Joining:
-        LOG_DEBUG(Network, "Connection sequence to room started");
+        NGLOG_DEBUG(Network, "Connection sequence to room started");
         break;
     case Network::RoomMember::State::Joined:
-        LOG_DEBUG(Network, "Successfully joined to the room");
+        NGLOG_DEBUG(Network, "Successfully joined to the room");
         break;
     case Network::RoomMember::State::LostConnection:
-        LOG_DEBUG(Network, "Lost connection to the room");
+        NGLOG_DEBUG(Network, "Lost connection to the room");
         break;
     case Network::RoomMember::State::CouldNotConnect:
-        LOG_ERROR(Network, "State: CouldNotConnect");
+        NGLOG_ERROR(Network, "State: CouldNotConnect");
         exit(1);
         break;
     case Network::RoomMember::State::NameCollision:
-        LOG_ERROR(
+        NGLOG_ERROR(
             Network,
             "You tried to use the same nickname then another user that is connected to the Room");
         exit(1);
         break;
     case Network::RoomMember::State::MacCollision:
-        LOG_ERROR(Network, "You tried to use the same MAC-Address then another user that is "
+        NGLOG_ERROR(Network, "You tried to use the same MAC-Address then another user that is "
-                           "connected to the Room");
+                             "connected to the Room");
         exit(1);
         break;
     case Network::RoomMember::State::WrongPassword:
-        LOG_ERROR(Network, "Room replied with: Wrong password");
+        NGLOG_ERROR(Network, "Room replied with: Wrong password");
         exit(1);
         break;
     case Network::RoomMember::State::WrongVersion:
-        LOG_ERROR(Network,
+        NGLOG_ERROR(Network,
-                  "You are using a different version then the room you are trying to connect to");
+                    "You are using a different version then the room you are trying to connect to");
         exit(1);
         break;
     default:
@@ -126,7 +127,7 @@ int main(int argc, char** argv) {
     auto argv_w = CommandLineToArgvW(GetCommandLineW(), &argc_w);
 
     if (argv_w == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to get command line arguments");
+        NGLOG_CRITICAL(Frontend, "Failed to get command line arguments");
         return -1;
     }
 #endif
@@ -162,7 +163,7 @@ int main(int argc, char** argv) {
                 break;
             case 'i': {
                 const auto cia_progress = [](size_t written, size_t total) {
-                    LOG_INFO(Frontend, "%02zu%%", (written * 100 / total));
+                    NGLOG_INFO(Frontend, "{:02d}%", (written * 100 / total));
                 };
                 if (Service::AM::InstallCIA(std::string(optarg), cia_progress) !=
                     Service::AM::InstallStatus::Success)
@@ -230,23 +231,22 @@ int main(int argc, char** argv) {
     LocalFree(argv_w);
 #endif
 
-    Log::Filter log_filter(Log::Level::Debug);
-    Log::SetFilter(&log_filter);
-
     MicroProfileOnThreadCreate("EmuThread");
     SCOPE_EXIT({ MicroProfileShutdown(); });
 
     if (filepath.empty()) {
-        LOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified");
+        NGLOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified");
         return -1;
     }
 
     if (!movie_record.empty() && !movie_play.empty()) {
-        LOG_CRITICAL(Frontend, "Cannot both play and record a movie");
+        NGLOG_CRITICAL(Frontend, "Cannot both play and record a movie");
         return -1;
     }
 
+    Log::Filter log_filter;
     log_filter.ParseFilterString(Settings::values.log_filter);
+    Log::SetFilter(&log_filter);
 
     // Apply the command line arguments
     Settings::values.gdbstub_port = gdb_port;
@@ -265,28 +265,28 @@ int main(int argc, char** argv) {
 
     switch (load_result) {
     case Core::System::ResultStatus::ErrorGetLoader:
-        LOG_CRITICAL(Frontend, "Failed to obtain loader for %s!", filepath.c_str());
+        NGLOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filepath);
         return -1;
     case Core::System::ResultStatus::ErrorLoader:
-        LOG_CRITICAL(Frontend, "Failed to load ROM!");
+        NGLOG_CRITICAL(Frontend, "Failed to load ROM!");
         return -1;
     case Core::System::ResultStatus::ErrorLoader_ErrorEncrypted:
-        LOG_CRITICAL(Frontend, "The game that you are trying to load must be decrypted before "
+        NGLOG_CRITICAL(Frontend, "The game that you are trying to load must be decrypted before "
-                               "being used with Citra. \n\n For more information on dumping and "
+                                 "being used with Citra. \n\n For more information on dumping and "
-                               "decrypting games, please refer to: "
+                                 "decrypting games, please refer to: "
-                               "https://citra-emu.org/wiki/dumping-game-cartridges/");
+                                 "https://citra-emu.org/wiki/dumping-game-cartridges/");
         return -1;
     case Core::System::ResultStatus::ErrorLoader_ErrorInvalidFormat:
-        LOG_CRITICAL(Frontend, "Error while loading ROM: The ROM format is not supported.");
+        NGLOG_CRITICAL(Frontend, "Error while loading ROM: The ROM format is not supported.");
         return -1;
     case Core::System::ResultStatus::ErrorNotInitialized:
-        LOG_CRITICAL(Frontend, "CPUCore not initialized");
+        NGLOG_CRITICAL(Frontend, "CPUCore not initialized");
         return -1;
     case Core::System::ResultStatus::ErrorSystemMode:
-        LOG_CRITICAL(Frontend, "Failed to determine system mode!");
+        NGLOG_CRITICAL(Frontend, "Failed to determine system mode!");
         return -1;
     case Core::System::ResultStatus::ErrorVideoCore:
-        LOG_CRITICAL(Frontend, "VideoCore not initialized");
+        NGLOG_CRITICAL(Frontend, "VideoCore not initialized");
         return -1;
     case Core::System::ResultStatus::Success:
         break; // Expected case
@@ -298,11 +298,11 @@ int main(int argc, char** argv) {
         if (auto member = Network::GetRoomMember().lock()) {
             member->BindOnChatMessageRecieved(OnMessageReceived);
             member->BindOnStateChanged(OnStateChanged);
-            LOG_DEBUG(Network, "Start connection to %s:%u with nickname %s", address.c_str(), port,
+            NGLOG_DEBUG(Network, "Start connection to {}:{} with nickname {}", address, port,
-                      nickname.c_str());
+                        nickname);
             member->Join(nickname, address.c_str(), port, 0, Network::NoPreferredMac, password);
         } else {
-            LOG_ERROR(Network, "Could not access RoomMember");
+            NGLOG_ERROR(Network, "Could not access RoomMember");
             return 0;
         }
     }

src/citra/config.cpp

@@ -27,17 +27,17 @@ bool Config::LoadINI(const std::string& default_contents, bool retry) {
     const char* location = this->sdl2_config_loc.c_str();
     if (sdl2_config->ParseError() < 0) {
         if (retry) {
-            LOG_WARNING(Config, "Failed to load %s. Creating file from defaults...", location);
+            NGLOG_WARNING(Config, "Failed to load {}. Creating file from defaults...", location);
             FileUtil::CreateFullPath(location);
             FileUtil::WriteStringToFile(true, default_contents, location);
             sdl2_config = std::make_unique<INIReader>(location); // Reopen file
 
             return LoadINI(default_contents, false);
         }
-        LOG_ERROR(Config, "Failed.");
+        NGLOG_ERROR(Config, "Failed.");
         return false;
     }
-    LOG_INFO(Config, "Successfully loaded %s", location);
+    NGLOG_INFO(Config, "Successfully loaded {}", location);
     return true;
 }
 

src/citra/emu_window/emu_window_sdl2.cpp

@@ -66,7 +66,7 @@ EmuWindow_SDL2::EmuWindow_SDL2() {
 
     // Initialize the window
     if (SDL_Init(SDL_INIT_VIDEO) < 0) {
-        LOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting...");
+        NGLOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting...");
         exit(1);
     }
 
@@ -89,19 +89,19 @@ EmuWindow_SDL2::EmuWindow_SDL2() {
                          SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI);
 
     if (render_window == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to create SDL2 window: %s", SDL_GetError());
+        NGLOG_CRITICAL(Frontend, "Failed to create SDL2 window: {}", SDL_GetError());
         exit(1);
     }
 
     gl_context = SDL_GL_CreateContext(render_window);
 
     if (gl_context == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to create SDL2 GL context: %s", SDL_GetError());
+        NGLOG_CRITICAL(Frontend, "Failed to create SDL2 GL context: {}", SDL_GetError());
         exit(1);
     }
 
     if (!gladLoadGLLoader(static_cast<GLADloadproc>(SDL_GL_GetProcAddress))) {
-        LOG_CRITICAL(Frontend, "Failed to initialize GL functions: %s", SDL_GetError());
+        NGLOG_CRITICAL(Frontend, "Failed to initialize GL functions: {}", SDL_GetError());
         exit(1);
     }
 

src/common/CMakeLists.txt

@@ -92,7 +92,7 @@ endif()
 
 create_target_directory_groups(common)
 
-target_link_libraries(common PUBLIC Boost::boost microprofile)
+target_link_libraries(common PUBLIC Boost::boost fmt microprofile)
 if (ARCHITECTURE_x86_64)
     target_link_libraries(common PRIVATE xbyak)
 endif()

src/common/logging/backend.cpp

@@ -11,6 +11,7 @@
 #include "common/logging/filter.h"
 #include "common/logging/log.h"
 #include "common/logging/text_formatter.h"
+#include "common/string_util.h"
 
 namespace Log {
 
@@ -113,25 +114,20 @@ const char* GetLevelName(Level log_level) {
 }
 
 Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr,
-                  const char* function, const char* format, va_list args) {
+                  const char* function, std::string message) {
     using std::chrono::duration_cast;
     using std::chrono::steady_clock;
 
     static steady_clock::time_point time_origin = steady_clock::now();
 
-    std::array<char, 4 * 1024> formatting_buffer;
-
     Entry entry;
     entry.timestamp = duration_cast<std::chrono::microseconds>(steady_clock::now() - time_origin);
     entry.log_class = log_class;
     entry.log_level = log_level;
-
+    entry.filename = Common::TrimSourcePath(filename);
-    snprintf(formatting_buffer.data(), formatting_buffer.size(), "%s:%s:%u", filename, function,
+    entry.line_num = line_nr;
-             line_nr);
+    entry.function = function;
-    entry.location = std::string(formatting_buffer.data());
+    entry.message = std::move(message);
-
-    vsnprintf(formatting_buffer.data(), formatting_buffer.size(), format, args);
-    entry.message = std::string(formatting_buffer.data());
 
     return entry;
 }
@@ -142,15 +138,27 @@ void SetFilter(Filter* new_filter) {
     filter = new_filter;
 }
 
-void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr,
+void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num,
                 const char* function, const char* format, ...) {
-    if (filter != nullptr && !filter->CheckMessage(log_class, log_level))
+    if (filter && !filter->CheckMessage(log_class, log_level))
         return;
-
+    std::array<char, 4 * 1024> formatting_buffer;
     va_list args;
     va_start(args, format);
-    Entry entry = CreateEntry(log_class, log_level, filename, line_nr, function, format, args);
+    vsnprintf(formatting_buffer.data(), formatting_buffer.size(), format, args);
     va_end(args);
+    Entry entry = CreateEntry(log_class, log_level, filename, line_num, function,
+                              std::string(formatting_buffer.data()));
+
+    PrintColoredMessage(entry);
+}
+
+void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num,
+                const char* function, const char* format, const fmt::format_args& args) {
+    if (filter && !filter->CheckMessage(log_class, log_level))
+        return;
+    Entry entry =
+        CreateEntry(log_class, log_level, filename, line_num, function, fmt::vformat(format, args));
 
     PrintColoredMessage(entry);
 }

src/common/logging/backend.h

@@ -22,13 +22,16 @@ struct Entry {
     std::chrono::microseconds timestamp;
     Class log_class;
     Level log_level;
-    std::string location;
+    std::string filename;
+    unsigned int line_num;
+    std::string function;
     std::string message;
 
     Entry() = default;
     Entry(Entry&& o) = default;
 
     Entry& operator=(Entry&& o) = default;
+    Entry& operator=(const Entry& o) = default;
 };
 
 /**
@@ -44,7 +47,7 @@ const char* GetLevelName(Level log_level);
 
 /// Creates a log entry by formatting the given source location, and message.
 Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr,
-                  const char* function, const char* format, va_list args);
+                  const char* function, std::string message);
 
 void SetFilter(Filter* filter);
 } // namespace Log

src/common/logging/filter.h

@@ -19,7 +19,7 @@ namespace Log {
 class Filter {
 public:
     /// Initializes the filter with all classes having `default_level` as the minimum level.
-    Filter(Level default_level);
+    Filter(Level default_level = Level::Info);
 
     /// Resets the filter so that all classes have `level` as the minimum displayed level.
     void ResetAll(Level level);

src/common/logging/log.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <fmt/format.h>
 #include "common/common_types.h"
 
 namespace Log {
@@ -98,7 +99,7 @@ enum class Class : ClassType {
 };
 
 /// Logs a message to the global logger.
-void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr,
+void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num,
                 const char* function,
 #ifdef _MSC_VER
                 _Printf_format_string_
@@ -110,6 +111,16 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned
 #endif
     ;
 
+/// Logs a message to the global logger, using fmt
+void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num,
+                const char* function, const char* format, const fmt::format_args& args);
+
+template <typename... Args>
+void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num,
+                   const char* function, const char* format, const Args&... args) {
+    LogMessage(log_class, log_level, filename, line_num, function, format, fmt::make_args(args...));
+}
+
 } // namespace Log
 
 #define LOG_GENERIC(log_class, log_level, ...)                                                     \
@@ -132,3 +143,28 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned
     LOG_GENERIC(::Log::Class::log_class, ::Log::Level::Error, __VA_ARGS__)
 #define LOG_CRITICAL(log_class, ...)                                                               \
     LOG_GENERIC(::Log::Class::log_class, ::Log::Level::Critical, __VA_ARGS__)
+
+// Define the fmt lib macros
+#ifdef _DEBUG
+#define NGLOG_TRACE(log_class, ...)                                                                \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__,         \
+                         __func__, __VA_ARGS__)
+#else
+#define NGLOG_TRACE(log_class, fmt, ...) (void(0))
+#endif
+
+#define NGLOG_DEBUG(log_class, ...)                                                                \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Debug, __FILE__, __LINE__,         \
+                         __func__, __VA_ARGS__)
+#define NGLOG_INFO(log_class, ...)                                                                 \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Info, __FILE__, __LINE__,          \
+                         __func__, __VA_ARGS__)
+#define NGLOG_WARNING(log_class, ...)                                                              \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Warning, __FILE__, __LINE__,       \
+                         __func__, __VA_ARGS__)
+#define NGLOG_ERROR(log_class, ...)                                                                \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Error, __FILE__, __LINE__,         \
+                         __func__, __VA_ARGS__)
+#define NGLOG_CRITICAL(log_class, ...)                                                             \
+    ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__,      \
+                         __func__, __VA_ARGS__)

src/common/logging/text_formatter.cpp

@@ -18,50 +18,29 @@
 
 namespace Log {
 
-// TODO(bunnei): This should be moved to a generic path manipulation library
+std::string FormatLogMessage(const Entry& entry) {
-const char* TrimSourcePath(const char* path, const char* root) {
-    const char* p = path;
-
-    while (*p != '\0') {
-        const char* next_slash = p;
-        while (*next_slash != '\0' && *next_slash != '/' && *next_slash != '\\') {
-            ++next_slash;
-        }
-
-        bool is_src = Common::ComparePartialString(p, next_slash, root);
-        p = next_slash;
-
-        if (*p != '\0') {
-            ++p;
-        }
-        if (is_src) {
-            path = p;
-        }
-    }
-    return path;
-}
-
-void FormatLogMessage(const Entry& entry, char* out_text, size_t text_len) {
     unsigned int time_seconds = static_cast<unsigned int>(entry.timestamp.count() / 1000000);
     unsigned int time_fractional = static_cast<unsigned int>(entry.timestamp.count() % 1000000);
 
     const char* class_name = GetLogClassName(entry.log_class);
     const char* level_name = GetLevelName(entry.log_level);
 
-    snprintf(out_text, text_len, "[%4u.%06u] %s <%s> %s: %s", time_seconds, time_fractional,
+    return fmt::format("[{:4d}.{:06d}] {} <{}> {}:{}:{}: {}", time_seconds, time_fractional,
-             class_name, level_name, TrimSourcePath(entry.location.c_str()), entry.message.c_str());
+                       class_name, level_name, entry.filename, entry.function, entry.line_num,
+                       entry.message);
 }
 
 void PrintMessage(const Entry& entry) {
-    std::array<char, 4 * 1024> format_buffer;
+    auto str = FormatLogMessage(entry) + '\n';
-    FormatLogMessage(entry, format_buffer.data(), format_buffer.size());
+    fputs(str.c_str(), stderr);
-    fputs(format_buffer.data(), stderr);
-    fputc('\n', stderr);
 }
 
 void PrintColoredMessage(const Entry& entry) {
 #ifdef _WIN32
-    static HANDLE console_handle = GetStdHandle(STD_ERROR_HANDLE);
+    HANDLE console_handle = GetStdHandle(STD_ERROR_HANDLE);
+    if (console_handle == INVALID_HANDLE_VALUE) {
+        return;
+    }
 
     CONSOLE_SCREEN_BUFFER_INFO original_info = {0};
     GetConsoleScreenBufferInfo(console_handle, &original_info);

src/common/logging/text_formatter.h

@@ -10,20 +10,8 @@ namespace Log {
 
 struct Entry;
 
-/**
- * Attempts to trim an arbitrary prefix from `path`, leaving only the part starting at `root`. It's
- * intended to be used to strip a system-specific build directory from the `__FILE__` macro,
- * leaving only the path relative to the sources root.
- *
- * @param path The input file path as a null-terminated string
- * @param root The name of the root source directory as a null-terminated string. Path up to and
- *             including the last occurrence of this name will be stripped
- * @return A pointer to the same string passed as `path`, but starting at the trimmed portion
- */
-const char* TrimSourcePath(const char* path, const char* root = "src");
-
 /// Formats a log entry into the provided text buffer.
-void FormatLogMessage(const Entry& entry, char* out_text, size_t text_len);
+std::string FormatLogMessage(const Entry& entry);
 /// Formats and prints a log entry to stderr.
 void PrintMessage(const Entry& entry);
 /// Prints the same message as `PrintMessage`, but colored acoording to the severity level.

src/common/string_util.cpp

@@ -462,4 +462,27 @@ std::string StringFromFixedZeroTerminatedBuffer(const char* buffer, size_t max_l
 
     return std::string(buffer, len);
 }
+
+const char* TrimSourcePath(const char* path, const char* root) {
+    const char* p = path;
+
+    while (*p != '\0') {
+        const char* next_slash = p;
+        while (*next_slash != '\0' && *next_slash != '/' && *next_slash != '\\') {
+            ++next_slash;
+        }
+
+        bool is_src = Common::ComparePartialString(p, next_slash, root);
+        p = next_slash;
+
+        if (*p != '\0') {
+            ++p;
+        }
+        if (is_src) {
+            path = p;
+        }
+    }
+    return path;
+}
+
 } // namespace Common

src/common/string_util.h

@@ -134,4 +134,17 @@ bool ComparePartialString(InIt begin, InIt end, const char* other) {
  * NUL-terminated then the string ends at max_len characters.
  */
 std::string StringFromFixedZeroTerminatedBuffer(const char* buffer, size_t max_len);
+
+/**
+ * Attempts to trim an arbitrary prefix from `path`, leaving only the part starting at `root`. It's
+ * intended to be used to strip a system-specific build directory from the `__FILE__` macro,
+ * leaving only the path relative to the sources root.
+ *
+ * @param path The input file path as a null-terminated string
+ * @param root The name of the root source directory as a null-terminated string. Path up to and
+ *             including the last occurrence of this name will be stripped
+ * @return A pointer to the same string passed as `path`, but starting at the trimmed portion
+ */
+const char* TrimSourcePath(const char* path, const char* root = "src");
+
 } // namespace Common