Repository navigation
RDKEMW-61972: Fix Coverity defects (3 files) - #548
Open
rdkdevpilot wants to merge 3 commits into
Open
rdkdevpilot wants to merge 3 commits into
rdkdevpilot wants to merge 3 commits into
Conversation
Defect ID: 448f8b44-8542-4705-a175-0d33a5413c23
Defect ID: f0cebec9-68aa-41bc-84a9-1100192c51c3
Defect ID: 739ee7d9-b8d2-4951-ad2d-27c4b2da335c
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new while loop around pthread_cond_timedwait() can busy-loop on timeout, and checkForUpdates() introduces a compile-time redeclaration risk when HAVE_VALUE_CHANGE_EVENT is enabled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR aims to address three Coverity-reported defects in the TR-069 host interface codebase, focusing on condition-variable usage, dead code reachability, and missing locking around shared state.
Changes:
libpd.cpp: Switches a condition-variable wait guard fromiftowhilein the Parodus receive thread.webpa_adapter.cpp: CallsgetnotifyparamList()so the initial notify list logic becomes reachable.hostIf_IPClient_ReqHandler.cpp: Adds a mutex guard incheckForUpdates()to protect shared counters used for add/remove event generation.
File summaries
| File | Description |
|---|---|
| src/hostif/parodusClient/pal/webpa_adapter.cpp | Populates notify parameter list so initialization logic can execute. |
| src/hostif/parodusClient/pal/libpd.cpp | Adjusts pthread condvar wait loop to satisfy Coverity’s wait-condition checking expectation. |
| src/hostif/handlers/src/hostIf_IPClient_ReqHandler.cpp | Adds locking around shared state used during update/event checks. |
Review details
Suppressed comments (1)
src/hostif/parodusClient/pal/libpd.cpp:165
- The new
while (!exit_parodus_recv.load())loop aroundpthread_cond_timedwaitwill spin indefinitely onETIMEDOUTbecausecurrTimeis not updated and the loop never breaks on timeout/error. After the first timeout, subsequent timedwait calls will immediately returnETIMEDOUT, causing a busy loop while holdingparodus_lockuntilstop_parodus_recv_wait()sets the exit flag.
while (!exit_parodus_recv.load())
{
int wait_ret = pthread_cond_timedwait(&parodus_cond, &parodus_lock,&currTime);
if(wait_ret == ETIMEDOUT)
{
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| memset(&msgData,0,sizeof(msgData)); | ||
|
|
||
| int interfaceNumberOfEntries = 0; | ||
| std::lock_guard<std::mutex> lg (m_mutex); |
Code Coverage Summary |
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
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.
Automated Fix for Coverity Defects
Triggered by: dev-user
Fixed Files
src/hostif/parodusClient/pal/libpd.cppsrc/hostif/parodusClient/pal/webpa_adapter.cppsrc/hostif/handlers/src/hostIf_IPClient_ReqHandler.cppFix Summaries
src/hostif/parodusClient/pal/libpd.cppThe fix correctly addresses the BAD_CHECK_OF_WAIT_COND defect by replacing
ifwithwhilearound thepthread_cond_timedwaitcall, ensuring the predicate is re-checked after spurious wakeups as required by POSIX. The change is minimal (single keyword), does not introduce new defects, and preserves all existing logic including the mutex lock/unlock pattern and the outer loop behavior.src/hostif/parodusClient/pal/webpa_adapter.cppThe fix correctly addresses the DEADCODE defect by adding a call to getnotifyparamList() before the NULL check on line 117. This populates the notifyparameters pointer, making the previously unreachable code block (lines 118-157) now reachable. The fix is minimal, adding only the necessary function call to resolve the defect without introducing new bugs or breaking existing logic.
src/hostif/handlers/src/hostIf_IPClient_ReqHandler.cppThe fix correctly adds std::lock_guardstd::mutex lg(m_mutex) in checkForUpdates() before accessing shared member variables (curNumOfIPInterface, curNumOfInterfaceIPv4Addresses, etc.), directly addressing the MISSING_LOCK data race defect. The change is minimal and does not break existing logic. The second 'lg' declaration at line 537 is inside a #ifdef HAVE_VALUE_CHANGE_EVENT block that is never compiled (the macro is commented out at line 34), so there is no duplicate variable name conflict.