From fcb594cb0ef062ce804091f4e152e8c16c819e59 Mon Sep 17 00:00:00 2001 From: Adam Honse Date: Fri, 8 May 2026 16:41:48 -0500 Subject: [PATCH] More LogManager cleanup, default to a log file count of 10, fix bug in log rotation --- LogManager.cpp | 61 +++++++++++++++++----------------- LogManager.h | 4 +-- ResourceManager.cpp | 79 +++++++++++++++++++++++---------------------- 3 files changed, 75 insertions(+), 69 deletions(-) diff --git a/LogManager.cpp b/LogManager.cpp index 235cb99a4..fcfa8c816 100644 --- a/LogManager.cpp +++ b/LogManager.cpp @@ -94,7 +94,7 @@ void LogManager::RegisterLogManagerCallback(LogManagerCallback new_callback, voi { LogManagerCallbackMutex.lock(); - for(size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) + for(std::size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) { if(LogManagerCallbacks[idx] == new_callback && LogManagerCallbackArgs[idx] == new_callback_arg) { @@ -118,7 +118,7 @@ void LogManager::UnregisterLogManagerCallback(LogManagerCallback callback, void { LogManagerCallbackMutex.lock(); - for(size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) + for(std::size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) { if(LogManagerCallbacks[idx] == callback && LogManagerCallbackArgs[idx] == callback_arg) { @@ -162,9 +162,9 @@ void LogManager::Configure(json config, const filesystem::path& config_dir) | current "logfile", starting with the oldest ones | | (according to the timestamp in their filename) | | i.e. with the lexicographically smallest filename | - | 0 or less equals no limit (default) | + | 0 or less equals no limit | \*-------------------------------------------------*/ - int loglimit = JsonUtils::JsonGetInt(config, "file_count_limit"); + int loglimit = JsonUtils::JsonGetInt(config, "file_count_limit", 10); bool log_file_enabled = JsonUtils::JsonGetBool(config, "log_file", true); /*-------------------------------------------------*\ @@ -185,7 +185,7 @@ void LogManager::Configure(json config, const filesystem::path& config_dir) snprintf(time_string, 64, TimestampPattern, 1900 + tmp->tm_year, tmp->tm_mon + 1, tmp->tm_mday, tmp->tm_hour, tmp->tm_min, tmp->tm_sec); std::string logname = logtemp; - size_t oct = logname.find("#"); + std::size_t oct = logname.find("#"); if(oct != logname.npos) { logname.replace(oct, 1, time_string); @@ -205,7 +205,7 @@ void LogManager::Configure(json config, const filesystem::path& config_dir) | "Log rotation": remove old log files | | exceeding the current configured limit | \*---------------------------------------------*/ - rotate_logs(p.parent_path(), filesystem::u8path(logtemp).filename(), loglimit); + LogRotate(p.parent_path(), filesystem::u8path(logtemp).filename(), loglimit); /*---------------------------------------------*\ | Open the logfile | @@ -236,7 +236,7 @@ void LogManager::Configure(json config, const filesystem::path& config_dir) /*-----------------------------------------------------*\ | Flush the log | \*-----------------------------------------------------*/ - flush(); + LogFlush(); } /*---------------------------------------------------------*\ @@ -351,14 +351,14 @@ void LogManager::LogEntry_message(PLogMessage message) \*-----------------------------------------------------*/ if(message->level == LL_DIALOG) { - for(size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) + for(std::size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) { LogManagerCallbacks[idx](LogManagerCallbackArgs[idx], LOGMANAGER_UPDATE_REASON_SHOW_DIALOG, message); } } else { - for(size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) + for(std::size_t idx = 0; idx < LogManagerCallbacks.size(); idx++) { LogManagerCallbacks[idx](LogManagerCallbackArgs[idx], LOGMANAGER_UPDATE_REASON_LOG_ENTRY, message); } @@ -400,7 +400,7 @@ void LogManager::LogEntry_message(PLogMessage message) /*-----------------------------------------------------*\ | Flush the queues | \*-----------------------------------------------------*/ - flush(); + LogFlush(); } void LogManager::LogEntry_va(const char* filename, int line, unsigned int level, const char* fmt, va_list va) @@ -455,7 +455,7 @@ void LogManager::LogEntry_va(const char* filename, int line, unsigned int level, /*---------------------------------------------------------*\ | Private Functions | \*---------------------------------------------------------*/ -void LogManager::flush() +void LogManager::LogFlush() { /*-----------------------------------------------------*\ | Lock the entry mutex while flushing | @@ -494,7 +494,7 @@ void LogManager::flush() } } -void LogManager::rotate_logs(const filesystem::path& folder, const filesystem::path& templ, int max_count) +void LogManager::LogRotate(const filesystem::path& folder, const filesystem::path& templ, std::size_t max_count) { if(max_count < 1) { @@ -510,7 +510,7 @@ void LogManager::rotate_logs(const filesystem::path& folder, const filesystem::p | backslash | \*-----------------------------------------------------*/ std::string regex_templ = "^"; - for(size_t i = 0; i < templ2.size(); ++i) + for(std::size_t i = 0; i < templ2.size(); ++i) { switch(templ2[i]) { @@ -577,26 +577,29 @@ void LogManager::rotate_logs(const filesystem::path& folder, const filesystem::p | the one we're about to create for max_count <= 0 and | | to prevent any possible errors in the above logic | \*-----------------------------------------------------*/ - size_t remove_count = valid_paths.size() - max_count + 1; - if(remove_count > valid_paths.size()) + if(valid_paths.size() > (max_count - 1)) { - remove_count = valid_paths.size(); - } - - for(size_t i = 0; i < remove_count; ++i) - { - /*-------------------------------------------------*\ - | Uses error code to force the `remove` call to be | - | `noexcept` | - \*-------------------------------------------------*/ - std::error_code ec; - if(filesystem::remove(valid_paths[i], ec)) + std::size_t remove_count = valid_paths.size() - max_count + 1; + if(remove_count > valid_paths.size()) { - LOG_VERBOSE("[LogManager] Removed log file [%s] during rotation", valid_paths[i].u8string().c_str()); + remove_count = valid_paths.size(); } - else + + for(std::size_t i = 0; i < remove_count; ++i) { - LOG_WARNING("[LogManager] Failed to remove log file [%s] during rotation: %s", valid_paths[i].u8string().c_str(), ec.message().c_str()); + /*-------------------------------------------------*\ + | Uses error code to force the `remove` call to be | + | `noexcept` | + \*-------------------------------------------------*/ + std::error_code ec; + if(filesystem::remove(valid_paths[i], ec)) + { + LOG_VERBOSE("[LogManager] Removed log file [%s] during rotation", valid_paths[i].u8string().c_str()); + } + else + { + LOG_WARNING("[LogManager] Failed to remove log file [%s] during rotation: %s", valid_paths[i].u8string().c_str(), ec.message().c_str()); + } } } } diff --git a/LogManager.h b/LogManager.h index 9583c81b1..a359be1dc 100644 --- a/LogManager.h +++ b/LogManager.h @@ -166,8 +166,8 @@ private: /*-----------------------------------------------------*\ | Private Functions | \*-----------------------------------------------------*/ - void flush(); - void rotate_logs(const filesystem::path& folder, const filesystem::path& templ, int max_count); + void LogFlush(); + void LogRotate(const filesystem::path& folder, const filesystem::path& templ, std::size_t max_count); }; /*---------------------------------------------------------*\ diff --git a/ResourceManager.cpp b/ResourceManager.cpp index 451047c62..60b95191d 100644 --- a/ResourceManager.cpp +++ b/ResourceManager.cpp @@ -168,33 +168,36 @@ ResourceManager::ResourceManager() \*-----------------------------------------------------*/ json logmanager_settings_schema; - logmanager_settings_schema["log_console"]["title"] = QT_TRANSLATE_NOOP("Settings", "Enable Log Console"); - logmanager_settings_schema["log_console"]["type"] = "bool"; + logmanager_settings_schema["log_console"]["title"] = QT_TRANSLATE_NOOP("Settings", "Enable Log Console"); + logmanager_settings_schema["log_console"]["type"] = "bool"; - logmanager_settings_schema["log_file"]["title"] = QT_TRANSLATE_NOOP("Settings", "Enable Log File"); - logmanager_settings_schema["log_file"]["type"] = "bool"; - logmanager_settings_schema["log_file"]["default"] = true; + logmanager_settings_schema["log_file"]["title"] = QT_TRANSLATE_NOOP("Settings", "Enable Log File"); + logmanager_settings_schema["log_file"]["type"] = "bool"; + logmanager_settings_schema["log_file"]["default"] = true; - logmanager_settings_schema["loglevel"]["title"] = QT_TRANSLATE_NOOP("Settings", "Log Level"); - logmanager_settings_schema["loglevel"]["type"] = "integer"; - logmanager_settings_schema["loglevel"]["enum"][0] = 0; - logmanager_settings_schema["loglevel"]["enumNames"][0] = "Fatal"; - logmanager_settings_schema["loglevel"]["enum"][1] = 1; - logmanager_settings_schema["loglevel"]["enumNames"][1] = "Error"; - logmanager_settings_schema["loglevel"]["enum"][2] = 2; - logmanager_settings_schema["loglevel"]["enumNames"][2] = "Warning"; - logmanager_settings_schema["loglevel"]["enum"][3] = 3; - logmanager_settings_schema["loglevel"]["enumNames"][3] = "Info"; - logmanager_settings_schema["loglevel"]["enum"][4] = 4; - logmanager_settings_schema["loglevel"]["enumNames"][4] = "Verbose"; - logmanager_settings_schema["loglevel"]["enum"][5] = 5; - logmanager_settings_schema["loglevel"]["enumNames"][5] = "Debug"; - logmanager_settings_schema["loglevel"]["enum"][6] = 6; - logmanager_settings_schema["loglevel"]["enumNames"][6] = "Trace"; + logmanager_settings_schema["loglevel"]["title"] = QT_TRANSLATE_NOOP("Settings", "Log Level"); + logmanager_settings_schema["loglevel"]["type"] = "integer"; + logmanager_settings_schema["loglevel"]["default"] = 3; + logmanager_settings_schema["loglevel"]["enum"][0] = 0; + logmanager_settings_schema["loglevel"]["enumNames"][0] = "Fatal"; + logmanager_settings_schema["loglevel"]["enum"][1] = 1; + logmanager_settings_schema["loglevel"]["enumNames"][1] = "Error"; + logmanager_settings_schema["loglevel"]["enum"][2] = 2; + logmanager_settings_schema["loglevel"]["enumNames"][2] = "Warning"; + logmanager_settings_schema["loglevel"]["enum"][3] = 3; + logmanager_settings_schema["loglevel"]["enumNames"][3] = "Info"; + logmanager_settings_schema["loglevel"]["enum"][4] = 4; + logmanager_settings_schema["loglevel"]["enumNames"][4] = "Verbose"; + logmanager_settings_schema["loglevel"]["enum"][5] = 5; + logmanager_settings_schema["loglevel"]["enumNames"][5] = "Debug"; + logmanager_settings_schema["loglevel"]["enum"][6] = 6; + logmanager_settings_schema["loglevel"]["enumNames"][6] = "Trace"; - logmanager_settings_schema["file_count_limit"]["title"] = QT_TRANSLATE_NOOP("Settings", "Log File Count Limit"); - logmanager_settings_schema["file_count_limit"]["type"] = "integer"; - logmanager_settings_schema["file_count_limit"]["minimum"] = 0; + logmanager_settings_schema["file_count_limit"]["title"] = QT_TRANSLATE_NOOP("Settings", "Log File Count Limit"); + logmanager_settings_schema["file_count_limit"]["type"] = "integer"; + logmanager_settings_schema["file_count_limit"]["description"] = QT_TRANSLATE_NOOP("Settings", "Maximum number of log files to keep, 0 for no limit"); + logmanager_settings_schema["file_count_limit"]["default"] = 10; + logmanager_settings_schema["file_count_limit"]["minimum"] = 0; settings_manager->RegisterSettingsSchema("LogManager", "Log Manager", logmanager_settings_schema); @@ -203,23 +206,23 @@ ResourceManager::ResourceManager() \*-----------------------------------------------------*/ json server_settings_schema; - server_settings_schema["all_controllers"]["title"] = QT_TRANSLATE_NOOP("Settings", "Serve All Controllers"); - server_settings_schema["all_controllers"]["type"] = "bool"; - server_settings_schema["all_controllers"]["description"] = QT_TRANSLATE_NOOP("Settings", "Include controllers provided by client connections and plugins"); + server_settings_schema["all_controllers"]["title"] = QT_TRANSLATE_NOOP("Settings", "Serve All Controllers"); + server_settings_schema["all_controllers"]["type"] = "bool"; + server_settings_schema["all_controllers"]["description"] = QT_TRANSLATE_NOOP("Settings", "Include controllers provided by client connections and plugins"); - server_settings_schema["default_host"]["title"] = QT_TRANSLATE_NOOP("Settings", "Default Host"); - server_settings_schema["default_host"]["type"] = "string"; - server_settings_schema["default_host"]["default"] = OPENRGB_SDK_HOST; + server_settings_schema["default_host"]["title"] = QT_TRANSLATE_NOOP("Settings", "Default Host"); + server_settings_schema["default_host"]["type"] = "string"; + server_settings_schema["default_host"]["default"] = OPENRGB_SDK_HOST; - server_settings_schema["default_port"]["title"] = QT_TRANSLATE_NOOP("Settings", "Default Port"); - server_settings_schema["default_port"]["type"] = "integer"; - server_settings_schema["default_port"]["default"] = OPENRGB_SDK_PORT; - server_settings_schema["default_port"]["minimum"] = 0; - server_settings_schema["default_port"]["maximum"] = 65535; + server_settings_schema["default_port"]["title"] = QT_TRANSLATE_NOOP("Settings", "Default Port"); + server_settings_schema["default_port"]["type"] = "integer"; + server_settings_schema["default_port"]["default"] = OPENRGB_SDK_PORT; + server_settings_schema["default_port"]["minimum"] = 0; + server_settings_schema["default_port"]["maximum"] = 65535; - server_settings_schema["legacy_workaround"]["title"] = QT_TRANSLATE_NOOP("Settings", "Legacy Workaround"); - server_settings_schema["legacy_workaround"]["type"] = "bool"; - server_settings_schema["legacy_workaround"]["description"] = QT_TRANSLATE_NOOP("Settings", "Workaround for some older SDK implementations that sent incorrect packet size for certain packets"); + server_settings_schema["legacy_workaround"]["title"] = QT_TRANSLATE_NOOP("Settings", "Legacy Workaround"); + server_settings_schema["legacy_workaround"]["type"] = "bool"; + server_settings_schema["legacy_workaround"]["description"] = QT_TRANSLATE_NOOP("Settings", "Workaround for some older SDK implementations that sent incorrect packet size for certain packets"); settings_manager->RegisterSettingsSchema("Server", "Server", server_settings_schema);