Reliability Fixes#14
Merged
Merged
Conversation
Issue: Inverted logic in onInterfacesRemoved() method causes incorrect cleanup behavior.
if (!adapters_.contains(objectPath)) {
if (adapters_.contains(objectPath)) { // This will never execute!
adapters_[objectPath].reset();
adapters_.erase(objectPath);
}
}
Impact:
Memory leaks when adapters/devices are removed
Orphaned proxy objects continue to exist
Map continues to grow with removed entries
Potential use-after-free if removed devices are accessed
Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Programs use std::this_thread::sleep_for() with hardcoded durations (120000ms = 2 minutes) and no signal handling for graceful shutdown. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Race condition between destructor and run() thread accessing pipe_fds_. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: gatt_characteristics_ and gatt_descriptors_ accessed within gatt_services_mutex_ lock, but they may need separate synchronization for concurrent access. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Callback invoked without null check, though constructor allows null callbacks. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: No overflow protection when dividing by 1024. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Exception handling in async callback may not work correctly. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: String operations assume format without validation. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: enterEventLoopAsync() followed by sleep creates a fixed-duration program with no early exit capability. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Many D-Bus operations lack try-catch blocks. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Classes with detached threads lack proper move semantics. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Properties retrieved with .get<T>() without checking types or handling exceptions. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: Multiple mutexes (adapters_mutex_, devices_mutex_, gatt_services_mutex_, etc.) could deadlock if acquired in different orders. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: No limits on number of tracked devices/adapters/services. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: All sleep durations are hardcoded (120000ms, 30000ms, 5s). Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Issue: spdlog used without configuration - no control over log levels, output destinations. Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
Signed-off-by: Joel Winarske <joel.winarske@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| spdlog::info("\tSpiderDspFwVersion: 0x{:08X}", | ||
| version.Data.SpiderDspFwVersion); | ||
| LOG_INFO("\tFirmwareVersion: {}", firmware_version_str.str()); | ||
| // LOG_INFO("\tFirmwareVersion: 0x{:08X}", version.Data.FirmwareVersion); |
Check notice
Code scanning / CodeQL
Commented-out code Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, remove the commented-out logging statement or turn it into a non-code explanatory comment. Since the surrounding code already logs the firmware version in a human-readable dotted format, and we want to avoid commented-out executable code, the best fix is to delete the line entirely.
Concretely, in src/bluez/ps5_dual_sense/input_reader.cc, within InputReader::PrintControllerVersion, delete line 447:
// LOG_INFO("\tFirmwareVersion: 0x{:08X}", version.Data.FirmwareVersion);No new methods, imports, or definitions are needed. This change does not alter existing functionality; it only cleans up dead/commented-out code.
Suggested changeset
1
src/bluez/ps5_dual_sense/input_reader.cc
| @@ -444,7 +444,6 @@ | ||
| << std::setw(4) << std::setfill('0') | ||
| << (firmware_version & 0xFFFF); | ||
| LOG_INFO("\tFirmwareVersion: {}", firmware_version_str.str()); | ||
| // LOG_INFO("\tFirmwareVersion: 0x{:08X}", version.Data.FirmwareVersion); | ||
| LOG_INFO("\tDeviceInfo: {}", version.Data.DeviceInfo); | ||
| LOG_INFO("\tUpdateVersion: 0x{:04X}", version.Data.UpdateVersion); | ||
| LOG_INFO("\tUpdateImageInfo: 0x{:02}", |
Copilot is powered by AI and may make mistakes. Always verify output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.