From 762415dfd0782e80c465366b875475d4f10ea8db Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Tue, 9 Jun 2026 18:10:50 -0400 Subject: [PATCH] refactor(sonar): fix cpp:S6197, cpp:S1066, and cpp:S1181 --- src/common/file_settings_persistence.cpp | 6 ++-- .../display_device/detail/json_converter.h | 29 +++++++++++++-- .../detail/json_serializer_details.h | 3 +- .../include/display_device/retry_scheduler.h | 16 +++++---- src/windows/settings_manager_apply.cpp | 36 ++++++++----------- src/windows/win_api_layer.cpp | 8 ++--- src/windows/win_api_utils.cpp | 6 ++-- src/windows/win_display_device_topology.cpp | 4 +-- .../test_file_settings_persistence.cpp | 5 +-- tests/unit/general/test_retry_scheduler.cpp | 6 ++-- .../test_win_display_device_general.cpp | 3 +- 11 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/common/file_settings_persistence.cpp b/src/common/file_settings_persistence.cpp index b436d3d8..96f1b6e2 100644 --- a/src/common/file_settings_persistence.cpp +++ b/src/common/file_settings_persistence.cpp @@ -30,9 +30,9 @@ namespace display_device { return false; } - std::copy(std::begin(data), std::end(data), std::ostreambuf_iterator {stream}); + std::ranges::copy(data, std::ostreambuf_iterator {stream}); return true; - } catch (const std::exception &error) { + } catch (const std::ios_base::failure &error) { DD_LOG(error) << "Failed to write to " << m_filepath << "! Error:\n" << error.what(); return false; @@ -58,7 +58,7 @@ namespace display_device { } return std::vector {std::istreambuf_iterator {stream}, std::istreambuf_iterator {}}; - } catch (const std::exception &error) { + } catch (const std::ios_base::failure &error) { DD_LOG(error) << "Failed to read " << m_filepath << "! Error:\n" << error.what(); return std::nullopt; diff --git a/src/common/include/display_device/detail/json_converter.h b/src/common/include/display_device/detail/json_converter.h index fa41c532..56fe38e8 100644 --- a/src/common/include/display_device/detail/json_converter.h +++ b/src/common/include/display_device/detail/json_converter.h @@ -7,6 +7,7 @@ #ifdef DD_JSON_DETAIL // system includes #include + #include namespace display_device { // A shared "toJson" implementation. Extracted here for UTs + coverage. @@ -19,7 +20,19 @@ namespace display_device { nlohmann::json json_obj = obj; return json_obj.dump(static_cast(indent.value_or(-1))); - } catch (const std::exception &err) { // GCOVR_EXCL_BR_LINE for fallthrough branch + } catch (const nlohmann::json::exception &err) { // GCOVR_EXCL_BR_LINE for fallthrough branch + if (success) { + *success = false; + } + + return err.what(); + } catch (const std::out_of_range &err) { // GCOVR_EXCL_BR_LINE for fallthrough branch + if (success) { + *success = false; + } + + return err.what(); + } catch (const std::invalid_argument &err) { // GCOVR_EXCL_BR_LINE for fallthrough branch if (success) { *success = false; } @@ -39,7 +52,19 @@ namespace display_device { Type parsed_obj = nlohmann::json::parse(string); obj = std::move(parsed_obj); return true; - } catch (const std::exception &err) { + } catch (const nlohmann::json::exception &err) { + if (error_message) { + *error_message = err.what(); + } + + return false; + } catch (const std::out_of_range &err) { + if (error_message) { + *error_message = err.what(); + } + + return false; + } catch (const std::invalid_argument &err) { if (error_message) { *error_message = err.what(); } diff --git a/src/common/include/display_device/detail/json_serializer_details.h b/src/common/include/display_device/detail/json_serializer_details.h index d0c5e319..6ef4b2f4 100644 --- a/src/common/include/display_device/detail/json_serializer_details.h +++ b/src/common/include/display_device/detail/json_serializer_details.h @@ -6,6 +6,7 @@ #ifdef DD_JSON_DETAIL // system includes + #include #include #include @@ -84,7 +85,7 @@ namespace display_device { template typename std::map::const_iterator findInEnumMap(const char *error_msg, Predicate predicate) { const auto &map {getEnumMap(T {})}; - auto it {std::find_if(std::begin(map), std::end(map), predicate)}; + auto it {std::ranges::find_if(map, predicate)}; if (it == std::end(map)) { // GCOVR_EXCL_BR_LINE for fallthrough branch throw std::out_of_range(error_msg); // GCOVR_EXCL_BR_LINE for fallthrough branch } diff --git a/src/common/include/display_device/retry_scheduler.h b/src/common/include/display_device/retry_scheduler.h index 85430c9d..8c3fc25f 100644 --- a/src/common/include/display_device/retry_scheduler.h +++ b/src/common/include/display_device/retry_scheduler.h @@ -7,6 +7,7 @@ // system includes #include #include +#include #include #include #include @@ -63,6 +64,11 @@ namespace display_device { }; namespace detail { + inline void logSchedulerException(const std::exception &exception, const char *message) { + DD_LOG(error) << message << " Error:\n" + << exception.what(); + } + /** * @brief Given that we know that we are dealing with a function, * check if it is an optional function (like std::function<...>) or other callable. @@ -176,9 +182,8 @@ namespace display_device { }}; m_retry_function(*m_iface, scheduler_stop_token); continue; - } catch (const std::exception &error) { - DD_LOG(error) << "Exception thrown in the RetryScheduler thread. Stopping scheduler. Error:\n" - << error.what(); + } catch (const std::exception &error) { // NOSONAR(cpp:S1181): Scheduler callback boundary must catch standard callback failures. + detail::logSchedulerException(error, "Exception thrown in the RetryScheduler thread. Stopping scheduler."); } clearThreadLoopUnlocked(); @@ -254,10 +259,9 @@ namespace display_device { m_sleep_durations = std::move(sleep_durations); syncThreadUnlocked(); } - } catch (const std::exception &error) { + } catch (const std::exception &error) { // NOSONAR(cpp:S1181): Scheduler callback boundary must catch standard callback failures. stop_token.requestStop(); - DD_LOG(error) << "Exception thrown in the RetryScheduler::schedule. Stopping scheduler. Error:\n" - << error.what(); + detail::logSchedulerException(error, "Exception thrown in the RetryScheduler::schedule. Stopping scheduler."); } } diff --git a/src/windows/settings_manager_apply.cpp b/src/windows/settings_manager_apply.cpp index ad1f19c0..51d56862 100644 --- a/src/windows/settings_manager_apply.cpp +++ b/src/windows/settings_manager_apply.cpp @@ -181,13 +181,11 @@ namespace display_device { // Non-stripped initial state MUST be checked here as the missing device could have its context captured! const bool switching_from_initial {m_dd_api->isTopologyTheSame(new_state.m_initial.m_topology, topology_before_changes)}; const bool new_topology_contains_all_current_topology_devices {std::ranges::includes(win_utils::flattenTopology(new_topology), win_utils::flattenTopology(topology_before_changes))}; - if (switching_from_initial && !new_topology_contains_all_current_topology_devices) { - // Only capture the context when switching from initial topology. All the other intermediate states, like non-existent - // capture state after system restart are to be avoided. - if (!m_audio_context_api->capture()) { - DD_LOG(error) << "Failed to capture audio context!"; - return std::nullopt; - } + // Only capture the context when switching from initial topology. All the other intermediate states, like non-existent + // capture state after system restart are to be avoided. + if (switching_from_initial && !new_topology_contains_all_current_topology_devices && !m_audio_context_api->capture()) { + DD_LOG(error) << "Failed to capture audio context!"; + return std::nullopt; } } @@ -251,11 +249,9 @@ namespace display_device { return true; } - if (might_need_to_restore) { - if (!try_change(cached_primary_device, "Changing primary display back to:\n", "Failed to restore original primary device!")) { - // Error already logged - return false; - } + if (might_need_to_restore && !try_change(cached_primary_device, "Changing primary display back to:\n", "Failed to restore original primary device!")) { + // Error already logged + return false; } return true; @@ -313,11 +309,9 @@ namespace display_device { return true; } - if (might_need_to_restore) { - if (!try_change(cached_display_modes, "Changing display modes back to:\n", "Failed to restore original display modes!")) { - // Error already logged - return false; - } + if (might_need_to_restore && !try_change(cached_display_modes, "Changing display modes back to:\n", "Failed to restore original display modes!")) { + // Error already logged + return false; } return true; @@ -370,11 +364,9 @@ namespace display_device { return true; } - if (might_need_to_restore) { - if (!try_change(cached_hdr_states, "Changing HDR states back to:\n", "Failed to restore original HDR states!")) { - // Error already logged - return false; - } + if (might_need_to_restore && !try_change(cached_hdr_states, "Changing HDR states back to:\n", "Failed to restore original HDR states!")) { + // Error already logged + return false; } return true; diff --git a/src/windows/win_api_layer.cpp b/src/windows/win_api_layer.cpp index 8b48a243..e803182b 100644 --- a/src/windows/win_api_layer.cpp +++ b/src/windows/win_api_layer.cpp @@ -656,11 +656,9 @@ namespace display_device { return FALSE; } - if (MONITORINFOEXA monitor_info {sizeof(MONITORINFOEXA)}; GetMonitorInfoA(monitor, &monitor_info)) { - if (data->m_display_name == monitor_info.szDevice) { - data->m_width = monitor_info.rcMonitor.right - monitor_info.rcMonitor.left; - return FALSE; - } + if (MONITORINFOEXA monitor_info {sizeof(MONITORINFOEXA)}; GetMonitorInfoA(monitor, &monitor_info) && data->m_display_name == monitor_info.szDevice) { + data->m_width = monitor_info.rcMonitor.right - monitor_info.rcMonitor.left; + return FALSE; } return TRUE; diff --git a/src/windows/win_api_utils.cpp b/src/windows/win_api_utils.cpp index 61e5228b..7711ae3f 100644 --- a/src/windows/win_api_utils.cpp +++ b/src/windows/win_api_utils.cpp @@ -183,10 +183,8 @@ namespace display_device::win_utils { return std::nullopt; } - if (type == ValidatedPathType::Active) { - if (!isActive(path)) { - return std::nullopt; - } + if (type == ValidatedPathType::Active && !isActive(path)) { + return std::nullopt; } const auto device_path {w_api.getMonitorDevicePath(path)}; diff --git a/src/windows/win_display_device_topology.cpp b/src/windows/win_display_device_topology.cpp index 1aaf3ce5..9d938f24 100644 --- a/src/windows/win_display_device_topology.cpp +++ b/src/windows/win_display_device_topology.cpp @@ -119,10 +119,10 @@ namespace display_device { bool WinDisplayDevice::isTopologyTheSame(const ActiveTopology &lhs, const ActiveTopology &rhs) const { const auto sort_topology = [](ActiveTopology &topology) { for (auto &group : topology) { - std::sort(std::begin(group), std::end(group)); + std::ranges::sort(group); } - std::sort(std::begin(topology), std::end(topology)); + std::ranges::sort(topology); }; auto lhs_copy {lhs}; diff --git a/tests/unit/general/test_file_settings_persistence.cpp b/tests/unit/general/test_file_settings_persistence.cpp index 241b170c..21e70446 100644 --- a/tests/unit/general/test_file_settings_persistence.cpp +++ b/tests/unit/general/test_file_settings_persistence.cpp @@ -1,4 +1,5 @@ // system includes +#include #include #include #include @@ -63,7 +64,7 @@ TEST_F_S(Store, FileOverwritten) { { std::ofstream file {filepath, std::ios_base::binary}; - std::copy(std::begin(data1), std::end(data1), std::ostreambuf_iterator {file}); + std::ranges::copy(data1, std::ostreambuf_iterator {file}); } EXPECT_TRUE(std::filesystem::exists(filepath)); @@ -94,7 +95,7 @@ TEST_F_S(Load, FileRead) { { std::ofstream file {filepath, std::ios_base::binary}; - std::copy(std::begin(data), std::end(data), std::ostreambuf_iterator {file}); + std::ranges::copy(data, std::ostreambuf_iterator {file}); } EXPECT_EQ(getImpl(filepath).load(), data); diff --git a/tests/unit/general/test_retry_scheduler.cpp b/tests/unit/general/test_retry_scheduler.cpp index 0bc96a6f..1c7702cc 100644 --- a/tests/unit/general/test_retry_scheduler.cpp +++ b/tests/unit/general/test_retry_scheduler.cpp @@ -24,10 +24,10 @@ namespace { void constMethod() const { /* noop */ } }; - class SchedulerStopTokenTestException final: public std::exception { + class SchedulerStopTokenTestException final: public std::runtime_error { public: - [[nodiscard]] const char *what() const noexcept override { - return "Get rekt!"; + SchedulerStopTokenTestException(): + std::runtime_error {"Get rekt!"} { } }; diff --git a/tests/unit/windows/test_win_display_device_general.cpp b/tests/unit/windows/test_win_display_device_general.cpp index 7509fb05..ab0c62bb 100644 --- a/tests/unit/windows/test_win_display_device_general.cpp +++ b/tests/unit/windows/test_win_display_device_general.cpp @@ -1,4 +1,5 @@ // system includes +#include #include #include @@ -87,7 +88,7 @@ TEST_F_S(EnumAvailableDevices) { const auto topology {display_device::win_utils::flattenTopology(m_win_dd.getCurrentTopology())}; for (const auto &device_id : *available_devices) { - auto enum_it {std::find_if(std::begin(enum_devices), std::end(enum_devices), [&device_id](const auto &entry) { + auto enum_it {std::ranges::find_if(enum_devices, [&device_id](const auto &entry) { return entry.m_device_id == device_id; })};