RDKEMW-14594: Fix coverity identified issues #427
Merged
Merged
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
RDKEMW-12282: Fix Coverity identified issues - dobby
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple Coverity-reported issues across the Dobby daemon, tooling, and plugins (primarily around thread-safety, resource handling, and safer system-call usage).
Changes:
- Add/adjust locking and atomics to fix data races in several components.
- Harden file/socket handling (TOCTOU reductions, bounds checks, overflow checks, safer cleanup).
- Minor robustness/cleanup fixes in tools and plugins (error handling, initialization, formatting).
Reviewed changes
Copilot reviewed 49 out of 51 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/source/DobbyUtils.cpp | Adds metadata map locking for thread-safe reads. |
| utils/include/DobbyUtils.h | Makes metadata mutex mutable to allow locking in const accessors. |
| settings/source/Settings.cpp | Fixes resource cleanup on path expansion failure; minor formatting cleanup. |
| rdkPlugins/Storage/source/RefCountFile.cpp | Adds refcount overflow/return-value handling and includes <climits>. |
| rdkPlugins/Storage/source/LoopMountDetails.cpp | Prevents potential double-close by resetting fd after close. |
| rdkPlugins/Storage/source/DynamicMountDetails.cpp | Refactors mount prep logic; improves mkdir/file creation and cleanup behavior. |
| rdkPlugins/OOMCrash/source/OOMCrashPlugin.cpp | Simplifies crash-file removal and guards fclose on successful open. |
| rdkPlugins/Networking/source/NetworkSetup.cpp | Simplifies ruleset access via operator[] instead of iterators. |
| rdkPlugins/Networking/source/NetworkingPlugin.cpp | Initializes mPluginData to nullptr. |
| rdkPlugins/Networking/source/IPAllocator.cpp | Reworks directory existence/creation flow using opendir(); improves error handling. |
| rdkPlugins/Minidump/source/AnonymousFile.cpp | Improves file size validation and ensures fclose() in more error paths. |
| rdkPlugins/Logging/source/FileSink.cpp | Initializes mFileSizeLimit to a defined value. |
| rdkPlugins/IONMemory/source/IonMemoryPlugin.cpp | Initializes mPluginData to nullptr. |
| rdkPlugins/HttpProxy/source/HttpProxyPlugin.cpp | Initializes mPluginData to nullptr; minor formatting. |
| rdkPlugins/AppServices/source/AppServicesRdkPlugin.cpp | Initializes mPluginConfig to nullptr; minor formatting. |
| plugins/OpenCDM/source/OpenCDMPlugin.cpp | Reduces TOCTOU checks; tightens chmod/chown/mount sequencing; improves addMount error handling. |
| plugins/MulticastSockets/source/MulticastSocketsPlugin.cpp | Improves socket creation/dup error checks; fixes fd formatting; uses move when pushing structs. |
| plugins/EthanLog/source/EthanLogLoop.cpp | Adds mutex protection when clearing client list. |
| plugins/EthanLog/source/EthanLogClient.cpp | Minor whitespace-only change. |
| plugins/EthanLog/client/cat/ethanlog-cat.cpp | Hardens pipe read bounds; switches buffer offset type; fixes switch fallthrough behavior. |
| plugins/Common/source/ServiceMonitor.cpp | Avoids calling handlers under lock; improves lock scoping and timer behavior. |
| pluginLauncher/tool/source/Main.cpp | Fixes option parsing switch flow (adds missing break). |
| pluginLauncher/lib/source/DobbyRdkPluginUtils.cpp | Initializes exitStatus to -1 in constructors. |
| pluginLauncher/lib/source/DobbyRdkPluginManager.cpp | Improves scandir failure handling; removes redundant close(dirFd). |
| pluginLauncher/lib/include/DobbyRdkPluginUtils.h | Adds locking to getAnnotations() accessor. |
| ipcUtils/source/DobbyIpcBus.cpp | Tightens lock scope in destructor before notifying/joining. |
| daemon/process/source/Main.cpp | Wraps daemon main in try/catch; fixes option parsing switch flow. |
| daemon/lib/source/include/DobbyWorkQueue.h | Converts counters/flags to atomics to address race conditions. |
| daemon/lib/source/DobbyWorkQueue.cpp | Adds missing lock when posting work from running thread. |
| daemon/lib/source/DobbyStats.cpp | Fixes PID log formatting for int64_t values. |
| daemon/lib/source/DobbyManager.cpp | Makes shutdown cleanup safer with lock/unlock sequencing; fixes hibernation lambda captures; adds Coverity suppression. |
| daemon/lib/source/DobbyLogRelay.cpp | Adds socket-path bounds checks and initializes relay members. |
| daemon/lib/source/DobbyLogger.cpp | Adds bounds checks for socket paths and wraps destructor in try/catch. |
| daemon/lib/source/DobbyContainer.cpp | Initializes restart counter. |
| daemon/lib/source/Dobby.cpp | Improves work-queue posting error handling; cleans up formatting and reply send checks. |
| client/tool/source/Main.cpp | Adds locking around promise fulfillment; refactors bundle/spec path detection to use opendir(); fixes option parsing switch flow. |
| client/lib/source/DobbyProxy.cpp | Tightens lock scope in destructor before notifying/joining. |
| bundle/tool/source/Main.cpp | Wraps tool main in try/catch; fixes option parsing switch flow. |
| bundle/lib/source/DobbyTemplate.cpp | Avoids returning instance pointer after releasing rwlock without local copy. |
| bundle/lib/source/DobbySpecConfig.cpp | Initializes rtPriority variables; adds locking around spec-version access. |
| bundle/lib/source/DobbyRootfs.cpp | Replaces access() check with open(O_DIRECTORY) to avoid TOCTOU and improve error reporting. |
| bundle/lib/source/DobbyConfig.cpp | Adds locking in printCommand; formatting cleanup. |
| bundle/lib/source/DobbyBundleConfig.cpp | Adds locking around several getters; formatting cleanup. |
| bundle/lib/include/DobbyBundleConfig.h | Minor indentation change. |
| AppInfrastructure/ReadLine/source/ReadLine.cpp | Fixes malformed fprintf format string. |
| AppInfrastructure/Public/Common/Notifier.h | Adjusts lock/unlock flow; adds Coverity suppression for dispatcher sync. |
| AppInfrastructure/IpcService/source/sdbus/SDBusIpcService.cpp | Prevents timeout multiplication overflow; formatting cleanup. |
| AppInfrastructure/Common/source/Timer.cpp | Wraps destructor cancel in try/catch. |
| AppInfrastructure/Common/source/ThreadedDispatcher.cpp | Refactors lock scope and notification logic; adds Coverity suppression. |
| AppInfrastructure/Common/include/IDGenerator.h | Switches seeding away from rand() to std::random_device. |
| AppInfrastructure/Common/include/ConditionVariable.h | Improves error handling/logging on unexpected pthread condvar errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
madanagopalt
approved these changes
Apr 8, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
Fix coverity identified issues
Test Procedure
Perform stability run and check for no regressions
Type of Change
Requires Bitbake Recipe changes?
meta-rdk-ext/recipes-containers/dobby/dobby.bb) must be modified to support the changes in this PR (beyond updatingSRC_REV)