From a86fb9b4758e893b3ca121cabb8f066e27f06c1c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 11:33:29 -0700 Subject: [PATCH 1/7] Core/WGInput: Use fmt logging and include HRESULT in messages --- .../ControllerInterface/WGInput/WGInput.cpp | 74 +++++++++++-------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/WGInput/WGInput.cpp b/Source/Core/InputCommon/ControllerInterface/WGInput/WGInput.cpp index 46beb409cd..b934e86e71 100644 --- a/Source/Core/InputCommon/ControllerInterface/WGInput/WGInput.cpp +++ b/Source/Core/InputCommon/ControllerInterface/WGInput/WGInput.cpp @@ -13,6 +13,7 @@ #include #include "Common/DynamicLibrary.h" +#include "Common/HRWrap.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" @@ -481,8 +482,8 @@ private: if (!waveform_name.data()) { - WARN_LOG(CONTROLLERINTERFACE, "WGInput: Unknown haptics feedback waveform: %d.", - waveform); + WARN_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Unknown haptics feedback waveform: 0x{:04x}.", + waveform); continue; } @@ -502,23 +503,31 @@ private: const auto switch_count = UINT32(m_switches.size()); const auto axis_count = UINT32(m_axes.size()); UINT64 timestamp = 0; - if (FAILED(m_raw_controller->GetCurrentReading(button_count, m_buttons.data(), switch_count, - m_switches.data(), axis_count, m_axes.data(), - ×tamp))) + const HRESULT raw_result = m_raw_controller->GetCurrentReading( + button_count, m_buttons.data(), switch_count, m_switches.data(), axis_count, m_axes.data(), + ×tamp); + if (FAILED(raw_result)) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: IRawGameController::GetCurrentReading failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, + "WGInput: IRawGameController::GetCurrentReading failed: {}", + Common::HRWrap(raw_result)); } // IGamepad: - if (m_gamepad && FAILED(m_gamepad->GetCurrentReading(&m_gamepad_reading))) + if (m_gamepad) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: IGamepad::GetCurrentReading failed."); + const HRESULT gamepad_result = m_gamepad->GetCurrentReading(&m_gamepad_reading); + if (FAILED(gamepad_result)) + { + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: IGamepad::GetCurrentReading failed: {}", + Common::HRWrap(gamepad_result)); + } } // IGameControllerBatteryInfo: if (m_controller_battery && !UpdateBatteryLevel()) { - DEBUG_LOG(CONTROLLERINTERFACE, "WGInput: UpdateBatteryLevel failed."); + DEBUG_LOG_FMT(CONTROLLERINTERFACE, "WGInput: UpdateBatteryLevel failed."); } } @@ -648,7 +657,7 @@ void Init() if (!LoadFunctionPointers()) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: System lacks support, skipping init."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: System lacks support, skipping init."); return; } @@ -666,7 +675,7 @@ void Init() break; default: - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: RoInitialize failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: RoInitialize failed: {}", Common::HRWrap(hr)); break; } } @@ -712,43 +721,49 @@ void PopulateDevices() HSTRING_HEADER header_raw_game_controller; HSTRING hstr_raw_game_controller; - if (FAILED(WindowsCreateStringReferenceAutoSizeWrapper(L"Windows.Gaming.Input.RawGameController", - &header_raw_game_controller, - &hstr_raw_game_controller))) + HRESULT hr; + if (FAILED(hr = WindowsCreateStringReferenceAutoSizeWrapper( + L"Windows.Gaming.Input.RawGameController", &header_raw_game_controller, + &hstr_raw_game_controller))) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: Failed to create string reference."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Failed to create string reference: {}", + Common::HRWrap(hr)); return; } HSTRING_HEADER header_gamepad; HSTRING hstr_gamepad; - if (FAILED(WindowsCreateStringReferenceAutoSizeWrapper(L"Windows.Gaming.Input.Gamepad", - &header_gamepad, &hstr_gamepad))) + if (FAILED(hr = WindowsCreateStringReferenceAutoSizeWrapper(L"Windows.Gaming.Input.Gamepad", + &header_gamepad, &hstr_gamepad))) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: Failed to create string reference."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Failed to create string reference: {}", + Common::HRWrap(hr)); return; } ComPtr raw_stats; - if (FAILED(g_RoGetActivationFactory_address( - hstr_raw_game_controller, __uuidof(WGI::IRawGameControllerStatics), &raw_stats))) + if (FAILED(hr = g_RoGetActivationFactory_address( + hstr_raw_game_controller, __uuidof(WGI::IRawGameControllerStatics), &raw_stats))) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: Failed to get IRawGameControllerStatics."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Failed to get IRawGameControllerStatics: {}", + Common::HRWrap(hr)); return; } ComPtr gamepad_stats; - if (FAILED(g_RoGetActivationFactory_address(hstr_gamepad, __uuidof(WGI::IGamepadStatics2), - &gamepad_stats))) + if (FAILED(hr = g_RoGetActivationFactory_address(hstr_gamepad, __uuidof(WGI::IGamepadStatics2), + &gamepad_stats))) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: Failed to get IGamepadStatics2."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Failed to get IGamepadStatics2: {}", + Common::HRWrap(hr)); return; } IVectorView* raw_controllers; - if (FAILED(raw_stats->get_RawGameControllers(&raw_controllers))) + if (FAILED(hr = raw_stats->get_RawGameControllers(&raw_controllers))) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: get_RawGameControllers failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: get_RawGameControllers failed: {}", + Common::HRWrap(hr)); return; } @@ -763,10 +778,10 @@ void PopulateDevices() // Attempt to get the controller's name. WGI::IRawGameController2* rgc2 = nullptr; - if (SUCCEEDED(raw_controller->QueryInterface(&rgc2)) && rgc2) + if (SUCCEEDED(hr = raw_controller->QueryInterface(&rgc2)) && rgc2) { HSTRING hstr = {}; - if (SUCCEEDED(rgc2->get_DisplayName(&hstr)) && hstr) + if (SUCCEEDED(hr = rgc2->get_DisplayName(&hstr)) && hstr) { device_name = StripSpaces(WStringToUTF8(g_WindowsGetStringRawBuffer_address(hstr, nullptr))); @@ -775,7 +790,8 @@ void PopulateDevices() if (device_name.empty()) { - ERROR_LOG(CONTROLLERINTERFACE, "WGInput: Failed to get device name."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "WGInput: Failed to get device name: {}", + Common::HRWrap(hr)); // Set a default name if we couldn't query the name or it was empty. device_name = "Device"; } From 0faf0b80ad705ed5556c264db330f8de6988a1e8 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 12:41:45 -0700 Subject: [PATCH 2/7] TAPServer_Apple: Use fmt logging --- .../Core/Core/HW/EXI/BBA/TAPServer_Apple.cpp | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/HW/EXI/BBA/TAPServer_Apple.cpp b/Source/Core/Core/HW/EXI/BBA/TAPServer_Apple.cpp index ad87b5ee84..77a8f74a9c 100644 --- a/Source/Core/Core/HW/EXI/BBA/TAPServer_Apple.cpp +++ b/Source/Core/Core/HW/EXI/BBA/TAPServer_Apple.cpp @@ -8,6 +8,7 @@ #include #include +#include "Common/CommonFuncs.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "Core/HW/EXI/EXI_Device.h" @@ -29,7 +30,7 @@ bool CEXIETHERNET::TAPServerNetworkInterface::Activate() sockaddr_un sun = {}; if (sizeof(socket_path) > sizeof(sun.sun_path)) { - ERROR_LOG(SP1, "Socket path is too long, unable to init BBA"); + ERROR_LOG_FMT(SP1, "Socket path is too long, unable to init BBA"); return false; } sun.sun_family = AF_UNIX; @@ -38,19 +39,19 @@ bool CEXIETHERNET::TAPServerNetworkInterface::Activate() fd = socket(AF_UNIX, SOCK_STREAM, 0); if (fd == -1) { - ERROR_LOG(SP1, "Couldn't create socket, unable to init BBA"); + ERROR_LOG_FMT(SP1, "Couldn't create socket, unable to init BBA"); return false; } if (connect(fd, reinterpret_cast(&sun), sizeof(sun)) == -1) { - ERROR_LOG(SP1, "Couldn't connect socket (%d), unable to init BBA", errno); + ERROR_LOG_FMT(SP1, "Couldn't connect socket ({}), unable to init BBA", LastStrerrorString()); close(fd); fd = -1; return false; } - INFO_LOG(SP1, "BBA initialized.\n"); + INFO_LOG_FMT(SP1, "BBA initialized."); return RecvInit(); } @@ -58,20 +59,20 @@ bool CEXIETHERNET::TAPServerNetworkInterface::SendFrame(const u8* frame, u32 siz { { const std::string s = ArrayToString(frame, size, 0x10); - INFO_LOG(SP1, "SendFrame %x\n%s\n", size, s.c_str()); + INFO_LOG_FMT(SP1, "SendFrame {}\n{}", size, s); } auto size16 = u16(size); if (write(fd, &size16, 2) != 2) { - ERROR_LOG(SP1, "SendFrame(): could not write size field\n"); + ERROR_LOG_FMT(SP1, "SendFrame(): could not write size field"); return false; } int written_bytes = write(fd, frame, size); if (u32(written_bytes) != size) { - ERROR_LOG(SP1, "SendFrame(): expected to write %d bytes, instead wrote %d", size, - written_bytes); + ERROR_LOG_FMT(SP1, "SendFrame(): expected to write {} bytes, instead wrote {}", size, + written_bytes); return false; } else @@ -98,19 +99,19 @@ void CEXIETHERNET::TAPServerNetworkInterface::ReadThreadHandler() u16 size; if (read(fd, &size, 2) != 2) { - ERROR_LOG(SP1, "Failed to read size field from BBA, err=%d", errno); + ERROR_LOG_FMT(SP1, "Failed to read size field from BBA: {}", LastStrerrorString()); } else { int read_bytes = read(fd, m_eth_ref->mRecvBuffer.get(), size); if (read_bytes < 0) { - ERROR_LOG(SP1, "Failed to read packet data from BBA, err=%d", errno); + ERROR_LOG_FMT(SP1, "Failed to read packet data from BBA: {}", LastStrerrorString()); } else if (readEnabled.IsSet()) { std::string data_string = ArrayToString(m_eth_ref->mRecvBuffer.get(), read_bytes, 0x10); - INFO_LOG(SP1, "Read data: %s", data_string.c_str()); + INFO_LOG_FMT(SP1, "Read data: {}", data_string); m_eth_ref->mRecvBufferLength = read_bytes; m_eth_ref->RecvHandlePacket(); } From 642a1c1348bd85802f9f945598fb83fd43cbbd9a Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 11:40:46 -0700 Subject: [PATCH 3/7] MainWindow: Replace WARN_LOG with WARN_LOG_FMT --- Source/Core/DolphinQt/MainWindow.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp index 5099ffa0e2..4cd1ca0d1a 100644 --- a/Source/Core/DolphinQt/MainWindow.cpp +++ b/Source/Core/DolphinQt/MainWindow.cpp @@ -325,9 +325,9 @@ void MainWindow::InitControllers() if (!g_controller_interface.HasDefaultDevice()) { // Note that the CI default device could be still temporarily removed at any time - WARN_LOG(CONTROLLERINTERFACE, - "No default device has been added in time. EmulatedController(s) defaulting adds" - " input mappings made for a specific default device depending on the platform"); + WARN_LOG_FMT(CONTROLLERINTERFACE, + "No default device has been added in time. EmulatedController(s) defaulting adds" + " input mappings made for a specific default device depending on the platform"); } GCAdapter::Init(); Pad::Initialize(); From 2f5e69a94e523b5380e20e8b592406c2e48f4ba6 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 12:11:07 -0700 Subject: [PATCH 4/7] JitArm64: Replace WARN_LOG with WARN_LOG_FMT --- Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index 3be81d2c71..6f8aeaceec 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -709,7 +709,7 @@ void JitArm64::Jit(u32 em_address, bool clear_cache_and_retry_on_failure) { // Code generation failed due to not enough free space in either the near or far code regions. // Clear the entire JIT cache and retry. - WARN_LOG(POWERPC, "flushing code caches, please report if this happens a lot"); + WARN_LOG_FMT(POWERPC, "flushing code caches, please report if this happens a lot"); ClearCache(); Jit(em_address, false); return; @@ -728,7 +728,7 @@ bool JitArm64::SetEmitterStateToFreeCodeRegion() auto free_near = m_free_ranges_near.by_size_begin(); if (free_near == m_free_ranges_near.by_size_end()) { - WARN_LOG(POWERPC, "Failed to find free memory region in near code region."); + WARN_LOG_FMT(POWERPC, "Failed to find free memory region in near code region."); return false; } SetCodePtr(free_near.from(), free_near.to()); @@ -736,7 +736,7 @@ bool JitArm64::SetEmitterStateToFreeCodeRegion() auto free_far = m_free_ranges_far.by_size_begin(); if (free_far == m_free_ranges_far.by_size_end()) { - WARN_LOG(POWERPC, "Failed to find free memory region in far code region."); + WARN_LOG_FMT(POWERPC, "Failed to find free memory region in far code region."); return false; } m_far_code.SetCodePtr(free_far.from(), free_far.to()); @@ -995,9 +995,9 @@ bool JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC) if (HasWriteFailed() || m_far_code.HasWriteFailed()) { if (HasWriteFailed()) - WARN_LOG(POWERPC, "JIT ran out of space in near code region during code generation."); + WARN_LOG_FMT(POWERPC, "JIT ran out of space in near code region during code generation."); if (m_far_code.HasWriteFailed()) - WARN_LOG(POWERPC, "JIT ran out of space in far code region during code generation."); + WARN_LOG_FMT(POWERPC, "JIT ran out of space in far code region during code generation."); return false; } From 35cb5c8f7580c3ca805134b013846357aa33dbf5 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 12:24:35 -0700 Subject: [PATCH 5/7] EXI_DeviceEthernet: Replace INFO_LOG with INFO_LOG_FMT --- Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp index e6c95391a9..2151b8f013 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.cpp @@ -50,7 +50,7 @@ CEXIETHERNET::CEXIETHERNET(BBADeviceType type) #if defined(__APPLE__) case BBADeviceType::TAPSERVER: m_network_interface = std::make_unique(this); - INFO_LOG(SP1, "Created tapserver physical network interface."); + INFO_LOG_FMT(SP1, "Created tapserver physical network interface."); break; #endif case BBADeviceType::XLINK: From 0637c17b599b26ee39164ac4144e1714eecaa574 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 11:55:27 -0700 Subject: [PATCH 6/7] VideoCommon/FrameDump: printf-format messages before logging them --- Source/Core/VideoCommon/FrameDump.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/FrameDump.cpp b/Source/Core/VideoCommon/FrameDump.cpp index 6ae6c21c37..92296e8794 100644 --- a/Source/Core/VideoCommon/FrameDump.cpp +++ b/Source/Core/VideoCommon/FrameDump.cpp @@ -28,7 +28,9 @@ extern "C" { #include "Common/ChunkFile.h" #include "Common/FileUtil.h" #include "Common/Logging/Log.h" +#include "Common/Logging/LogManager.h" #include "Common/MsgHandler.h" +#include "Common/StringUtil.h" #include "Core/Config/MainSettings.h" #include "Core/ConfigManager.h" @@ -95,7 +97,17 @@ void InitAVCodec() // keep libav debug messages visible in release build of dolphin log_level = Common::Log::LogLevel::LINFO; - GENERIC_LOG_V(Common::Log::LogType::FRAMEDUMP, log_level, fmt, vl); + // Don't perform this formatting if the log level is disabled + auto* log_manager = Common::Log::LogManager::GetInstance(); + if (log_manager != nullptr && + log_manager->IsEnabled(Common::Log::LogType::FRAMEDUMP, log_level)) + { + constexpr size_t MAX_MSGLEN = 1024; + char message[MAX_MSGLEN]; + CharArrayFromFormatV(message, MAX_MSGLEN, fmt, vl); + + GENERIC_LOG_FMT(Common::Log::LogType::FRAMEDUMP, log_level, "{}", message); + } }); // TODO: We never call avformat_network_deinit. From 5f9212dd8479887da15659c7f277839d6115a46d Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Thu, 19 May 2022 11:59:54 -0700 Subject: [PATCH 7/7] Common/LogManager: Remove old printf-style logging functions --- Source/Core/Common/Logging/Log.h | 50 ----------------------- Source/Core/Common/Logging/LogManager.cpp | 34 --------------- 2 files changed, 84 deletions(-) diff --git a/Source/Core/Common/Logging/Log.h b/Source/Core/Common/Logging/Log.h index 98d420bda2..5a0d2c6e24 100644 --- a/Source/Core/Common/Logging/Log.h +++ b/Source/Core/Common/Logging/Log.h @@ -96,58 +96,8 @@ void GenericLogFmt(LogLevel level, LogType type, const char* file, int line, con "too many arguments?"); GenericLogFmtImpl(level, type, file, line, format, fmt::make_format_args(args...)); } - -void GenericLog(LogLevel level, LogType type, const char* file, int line, const char* fmt, ...) -#ifdef __GNUC__ - __attribute__((format(printf, 5, 6))) -#endif - ; - -void GenericLogV(LogLevel level, LogType type, const char* file, int line, const char* fmt, - va_list args); } // namespace Common::Log -// Let the compiler optimize this out -#define GENERIC_LOG(t, v, ...) \ - do \ - { \ - if (v <= Common::Log::MAX_LOGLEVEL) \ - Common::Log::GenericLog(v, t, __FILE__, __LINE__, __VA_ARGS__); \ - } while (0) - -#define ERROR_LOG(t, ...) \ - do \ - { \ - GENERIC_LOG(Common::Log::LogType::t, Common::Log::LogLevel::LERROR, __VA_ARGS__); \ - } while (0) -#define WARN_LOG(t, ...) \ - do \ - { \ - GENERIC_LOG(Common::Log::LogType::t, Common::Log::LogLevel::LWARNING, __VA_ARGS__); \ - } while (0) -#define NOTICE_LOG(t, ...) \ - do \ - { \ - GENERIC_LOG(Common::Log::LogType::t, Common::Log::LogLevel::LNOTICE, __VA_ARGS__); \ - } while (0) -#define INFO_LOG(t, ...) \ - do \ - { \ - GENERIC_LOG(Common::Log::LogType::t, Common::Log::LogLevel::LINFO, __VA_ARGS__); \ - } while (0) -#define DEBUG_LOG(t, ...) \ - do \ - { \ - GENERIC_LOG(Common::Log::LogType::t, Common::Log::LogLevel::LDEBUG, __VA_ARGS__); \ - } while (0) - -#define GENERIC_LOG_V(t, v, fmt, args) \ - do \ - { \ - if (v <= Common::Log::MAX_LOGLEVEL) \ - Common::Log::GenericLogV(v, t, __FILE__, __LINE__, fmt, args); \ - } while (0) - // fmtlib capable API #define GENERIC_LOG_FMT(t, v, format, ...) \ diff --git a/Source/Core/Common/Logging/LogManager.cpp b/Source/Core/Common/Logging/LogManager.cpp index bed3a183e4..ab795d372a 100644 --- a/Source/Core/Common/Logging/LogManager.cpp +++ b/Source/Core/Common/Logging/LogManager.cpp @@ -62,40 +62,6 @@ private: bool m_enable; }; -void GenericLog(LogLevel level, LogType type, const char* file, int line, const char* fmt, ...) -{ - auto* instance = LogManager::GetInstance(); - if (instance == nullptr) - return; - - if (!instance->IsEnabled(type, level)) - return; - - va_list args; - va_start(args, fmt); - char message[MAX_MSGLEN]; - CharArrayFromFormatV(message, MAX_MSGLEN, fmt, args); - va_end(args); - - instance->Log(level, type, file, line, message); -} - -void GenericLogV(LogLevel level, LogType type, const char* file, int line, const char* fmt, - va_list args) -{ - auto* instance = LogManager::GetInstance(); - if (instance == nullptr) - return; - - if (!instance->IsEnabled(type, level)) - return; - - char message[MAX_MSGLEN]; - CharArrayFromFormatV(message, MAX_MSGLEN, fmt, args); - - instance->Log(level, type, file, line, message); -} - void GenericLogFmtImpl(LogLevel level, LogType type, const char* file, int line, fmt::string_view format, const fmt::format_args& args) {