RDK-60308-[tr69hostif, RFC] RDK Coverity Defect Resolution for Device Management by Vismalskumar0 · Pull Request #143 · rdkcentral/rfc

Vismalskumar0 · 2026-01-09T10:43:33Z

RDK-60308-[tr69hostif, RFC] RDK Coverity Defect Resolution for Device Management

Fixed 17 COPY_INSTEAD_OF_MOVE warnings by using std::move() for string operations:
- clearDB(), rfcStashRetrieveParams(), clearDBEnd()
- updateHashInDB(), updateTimeInDB(), updateHashAndTimeInDB()
- GetValidAccountId(), GetValidPartnerId(), GetXconfSelect()
- processXconfResponseConfigDataPart(), ProcessXconfUrl()
Fixed DEADCODE warning in DownloadRuntimeFeatutres() by adding break statements to switch cases
Fixed RESOURCE_LEAK in DownloadRuntimeFeatutres() by properly freeing strdup'd hashvalue and hashtime strings
Fixed TOCTOU vulnerability in IsDirectBlocked() by removing race condition between stat() and remove()
Fixed UNINIT_CTOR warnings by initializing member variables:
- XconfHandler: _ebuild_type initialized to RDKB_DEV
- RuntimeFeatureControlProcessor: rfc_state initialized to Invalid

These changes improve code performance through move semantics, eliminate potential
memory leaks, and fix race conditions and uninitialized variable issues.

Merge

Fix multiple code quality issues in RFC xconf handler - Fixed 12 COPY_INSTEAD_OF_MOVE warnings using std::move(): * clearDB(): Move strings in set_RFCProperty calls * rfcStashRetrieveParams(): Move stashAccountId parameter * clearDBEnd(): Move key strings * updateHashInDB(): Move ConfigSetHash parameters * updateTimeInDB(): Move ConfigSetTime parameters * updateHashAndTimeInDB(): Move configSetHashValue and timestampString * GetValidAccountId(): Move value assignment * GetValidPartnerId(): Move value assignment * GetXconfSelect(): Move XconfUrl assignment * processXconfResponseConfigDataPart(): Move data string in push_back * ProcessXconfUrl(): Move FQDN assignment - Fixed DEADCODE warning by adding break statements to curl error code switch - Fixed RESOURCE_LEAK by properly freeing hashvalue and hashtime before freeing hashData - Fixed TOCTOU race condition by removing error check after remove() call

nhanasi · 2026-01-09T14:26:41Z

rfcMgr/xconf_handler.h

why this change needed?

nhanasi · 2026-01-09T14:26:54Z

rfcMgr/xconf_handler.h

    class XconfHandler {
        public :
-        XconfHandler(){ }
+        XconfHandler() : _ebuild_type(RDKB_DEV) { }


this is not needed

nhanasi · 2026-01-09T14:27:26Z

rfcMgr/rfc_xconf_handler.cpp

    _xconf_server_url = std::string(XconfUrl) + "/featureControl/getSettings"; 
    std::stringstream url = CreateXconfHTTPUrl();
-    _xconf_server_url = FQDN;
+    _xconf_server_url = std::move(FQDN);


why stb:move needed only for few params?

nhanasi · 2026-01-09T14:28:05Z

rfcMgr/rfc_xconf_handler.cpp

        else
        {
-            if (newValue.empty())
+            if (currentValue.empty())


rebase the code

nhanasi · 2026-01-09T14:28:47Z

rfcMgr/rfc_xconf_handler.cpp


-    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value: %s\n", __FUNCTION__,__LINE__,ClearDB.c_str());
-    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value: %s\n", __FUNCTION__,__LINE__,BootstrapClearDB.c_str());
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value\n", __FUNCTION__,__LINE__);


why we removing the log?

rdkcmf-jenkins · 2026-01-09T14:32:01Z

Coverity Issue - Using a moved object

"reloadCacheKey" is used after it has been already moved.

High Impact, CWE-457
USE_AFTER_MOVE

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
rfcMgr/rfc_xconf_handler.cpp:1380

rdkcmf-jenkins · 2026-01-09T14:32:01Z

rfcMgr/rfc_xconf_handler.cpp

+    set_RFCProperty(name, std::move(reloadCacheKey), clearValue);

    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DBEnd Key Value: %s\n", __FUNCTION__,__LINE__,ClearDBEndKey.c_str());
    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DBEnd Key Value: %s\n", __FUNCTION__,__LINE__,BootstrapClearDBEndKey.c_str());


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Using a moved object

"BootstrapClearDBEndKey" is used after it has been already moved.

High Impact, CWE-457
USE_AFTER_MOVE

rdkcmf-jenkins · 2026-01-09T14:32:01Z

rfcMgr/rfc_xconf_handler.cpp

+    set_RFCProperty(name, std::move(BootstrapClearDBEndKey), clearValue);
+    set_RFCProperty(name, std::move(reloadCacheKey), clearValue);

    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DBEnd Key Value: %s\n", __FUNCTION__,__LINE__,ClearDBEndKey.c_str());


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Using a moved object

"ClearDBEndKey" is used after it has been already moved.

High Impact, CWE-457
USE_AFTER_MOVE

rdkcmf-jenkins · 2026-01-09T14:32:01Z

rfcMgr/rfc_xconf_handler.cpp

-    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value: %s\n", __FUNCTION__,__LINE__,BootstrapClearDB.c_str());
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value\n", __FUNCTION__,__LINE__);
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value\n", __FUNCTION__,__LINE__);
    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] ConfigChangeTime: %s\n", __FUNCTION__,__LINE__,ConfigChangeTime.c_str());


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Using a moved object

"ConfigChangeTime" is used after it has been already moved.

High Impact, CWE-457
USE_AFTER_MOVE

Merge and rebase

rebase and merge

Remove std::move() from string variables that are used in subsequent log statements to fix 4 USE_AFTER_MOVE defects: - ConfigChangeTime at line 1300 - ClearDBEndKey at line 1374 - BootstrapClearDBEndKey at line 1375 - reloadCacheKey at line 1376 These const string literal assignments don't benefit from move semantics and accessing them after move causes undefined behavior.

Remove extra space in XconfHandler constructor to address code review feedback.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot · 2026-01-14T08:23:50Z

rfcMgr/rfc_xconf_handler.cpp

+            // Attempt to remove the file - if it fails (e.g., already removed), it's not critical
+            (void)remove(DIRECT_BLOCK_FILENAME);


The TOCTOU fix is correct in removing the race condition between stat() and remove(). However, silently ignoring all remove() failures (via void cast) may hide legitimate errors like permission issues. Consider logging at DEBUG level if remove() fails, checking errno to distinguish between "file not found" (expected race condition) and other errors like EACCES (permission denied).

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-01-14T08:23:50Z

rfcMgr/rfc_xconf_handler.cpp

+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value\n", __FUNCTION__,__LINE__);
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value\n", __FUNCTION__,__LINE__);


The logging statements were updated to remove the string values because the variables were moved on lines 1298-1299. However, these log statements are now less informative. Consider either: 1) keeping the original code without std::move() for these variables since the performance gain is minimal for short-lived local variables, or 2) logging the values before the move operations.

@copilot open a new pull request to apply changes based on this feedback

rfcMgr/rfc_xconf_handler.cpp

Copilot · 2026-01-14T08:23:51Z

rfcMgr/rfc_xconf_handler.cpp

+    set_RFCProperty(name, std::move(ConfigChangeTimeKey), std::move(ConfigChangeTime));

-    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value: %s\n", __FUNCTION__,__LINE__,ClearDB.c_str());
-    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value: %s\n", __FUNCTION__,__LINE__,BootstrapClearDB.c_str());
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value\n", __FUNCTION__,__LINE__);
+    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value\n", __FUNCTION__,__LINE__);
    RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] ConfigChangeTime: %s\n", __FUNCTION__,__LINE__,ConfigChangeTime.c_str());


Use-after-move bug: The variable ConfigChangeTime is moved on line 1300, but then used again in the logging statement on line 1304 via .c_str(). After a string is moved, its contents are in an undefined state and accessing it leads to undefined behavior. Either remove std::move() from this variable or remove the logging statement that uses it.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-01-22T13:13:31Z

@Vismalskumar0 I've opened a new pull request, #153, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-01-22T13:14:28Z

@Vismalskumar0 I've opened a new pull request, #154, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-22T13:14:35Z

@Vismalskumar0 I've opened a new pull request, #155, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-01-22T13:14:54Z

@Vismalskumar0 I've opened a new pull request, #156, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T13:17:48Z

rfcMgr/rfc_xconf_handler.cpp

    {
        _xconf_server_url.clear();
-        _xconf_server_url = XconfUrl;
+        _xconf_server_url = std::move(XconfUrl);


Using std::move on a local variable (XconfUrl) to assign to a member variable (_xconf_server_url) prevents copy elision and doesn't provide any benefit here since the local variable is going out of scope anyway. Consider removing std::move and letting the compiler optimize the assignment.

Suggested change

_xconf_server_url = std::move(XconfUrl);

_xconf_server_url = XconfUrl;

Copilot · 2026-01-22T13:17:49Z

rfcMgr/rfc_xconf_handler.h

@@ -1,3 +1,4 @@
+


An unnecessary blank line was added at the beginning of the file. This appears to be unintentional and should be removed to maintain code consistency.

Suggested change

Copilot · 2026-01-22T13:17:49Z

rfcMgr/rfc_xconf_handler.cpp

        std::string name = "rfc";

-        WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, stashAccountId);
+        WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, std::move(stashAccountId));


After moving stashAccountId into the set_RFCProperty function at line 1345, the variable is in a valid but unspecified state. The assignment at line 1356 (_accountId = stashAccountId) will assign this unspecified value to _accountId, which is incorrect. The std::move should be removed from line 1345, or the assignment at line 1356 should use the value before it was moved.

Suggested change

WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, std::move(stashAccountId));

WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, stashAccountId);

Copilot · 2026-01-22T13:17:49Z

rfcMgr/rfc_xconf_handler.cpp

    else
    {
-        _valid_accountId = value;
+        _valid_accountId = std::move(value);


Using std::move on a local variable (value) to assign to a member variable (_valid_accountId) prevents copy elision and doesn't provide any benefit here since the local variable is going out of scope anyway. Additionally, if value is moved and later accessed in the logging or comparisons that follow, it would be in an undefined state. In this case, the move appears safe since value is not used after the assignment, but it's unnecessary and could be confusing. Consider removing std::move and letting the compiler optimize the assignment.

Suggested change

_valid_accountId = std::move(value);

_valid_accountId = value;

Copilot · 2026-01-22T13:17:49Z

rfcMgr/rfc_xconf_handler.cpp

       else
       {
-            _valid_partnerId = value;
+            _valid_partnerId = std::move(value);


Using std::move on a local variable (value) to assign to a member variable (_valid_partnerId) prevents copy elision and doesn't provide any benefit here since the local variable is going out of scope anyway. Additionally, if value is moved and later accessed in logging or comparisons that follow, it would be in an undefined state. In this case, the move appears safe since value is not used after the assignment, but it's unnecessary and could be confusing. Consider removing std::move and letting the compiler optimize the assignment.

Suggested change

_valid_partnerId = std::move(value);

_valid_partnerId = value;

rdkcmf-jenkins · 2026-01-22T13:25:31Z

Coverity Issue - Using a moved object

"configSetHash" is used after it has been already moved.

High Impact, CWE-457
USE_AFTER_MOVE

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
rfcMgr/rfc_xconf_handler.cpp:1406

Vismalskumar0 added 4 commits January 9, 2026 16:07

Merge pull request #142 from rdkcentral/develop

a33fd4b

Merge

Update rfc_xconf_handler.h

2baaf23

Update xconf_handler.h

e2360e6

Vismalskumar0 requested a review from a team as a code owner January 9, 2026 10:43

Vismalskumar0 added 2 commits January 9, 2026 19:50

Update rfc_xconf_handler.h

e03568e

Update xconf_handler.h

7bf89e2

nhanasi reviewed Jan 9, 2026

View reviewed changes

rfcMgr/xconf_handler.h

Copy link

Contributor

nhanasi Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change needed?

nhanasi reviewed Jan 9, 2026

View reviewed changes

rfcMgr/rfc_xconf_handler.cpp

else

{

if (newValue.empty())

if (currentValue.empty())

Copy link

Contributor

nhanasi Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase the code

nhanasi reviewed Jan 9, 2026

View reviewed changes

rdkcmf-jenkins reviewed Jan 9, 2026

View reviewed changes

Vismalskumar0 added 2 commits January 12, 2026 13:19

Merge pull request #144 from rdkcentral/develop

ad72760

Merge and rebase

Merge pull request #151 from rdkcentral/develop

aa456bd

rebase and merge

Copilot AI review requested due to automatic review settings January 14, 2026 08:18

Copilot started reviewing on behalf of Vismalskumar0 January 14, 2026 08:19 View session

Vismalskumar0 added 2 commits January 14, 2026 13:58

Remove unnecessary whitespace in xconf_handler.h

83950ae

Remove extra space in XconfHandler constructor to address code review feedback.

Copilot AI reviewed Jan 14, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 22, 2026

Remove std::move() from local variables with subsequent usage in clearDB() #153

Draft

Update rfcMgr/rfc_xconf_handler.cpp

cb05e91

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings January 22, 2026 13:13

Copilot started reviewing on behalf of Vismalskumar0 January 22, 2026 13:14 View session

This was referenced Jan 22, 2026

Add errno checking and debug logging for remove() failures in TOCTOU fix #154

Draft

Remove std::move() and enhance logging in clearDB() #155

Draft

Copilot AI mentioned this pull request Jan 22, 2026

Confirm use-after-move fix for ConfigChangeTime is already applied #156

Draft

Copilot AI reviewed Jan 22, 2026

View reviewed changes

		// Attempt to remove the file - if it fails (e.g., already removed), it's not critical
		(void)remove(DIRECT_BLOCK_FILENAME);

		RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Clearing DB Value\n", __FUNCTION__,__LINE__);
		RDK_LOG(RDK_LOG_INFO, LOG_RFCMGR, "[%s][%d] Bootstrap Clearing DB Value\n", __FUNCTION__,__LINE__);

	_xconf_server_url = std::move(XconfUrl);
	_xconf_server_url = XconfUrl;

	WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, std::move(stashAccountId));
	WDMP_STATUS status = set_RFCProperty(name, RFC_ACCOUNT_ID_KEY_STR, stashAccountId);

	_valid_accountId = std::move(value);
	_valid_accountId = value;

	_valid_partnerId = std::move(value);
	_valid_partnerId = value;

Conversation

Vismalskumar0 commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins commented Jan 9, 2026

Coverity Issue - Using a moved object

Issue location

Uh oh!

rdkcmf-jenkins Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Using a moved object

Uh oh!

rdkcmf-jenkins Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Using a moved object

Uh oh!

rdkcmf-jenkins Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Using a moved object

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 22, 2026

Uh oh!

Copilot AI commented Jan 22, 2026

Uh oh!

Copilot AI commented Jan 22, 2026

Uh oh!

Copilot AI commented Jan 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins Jan 9, 2026 •

edited

Loading

rdkcmf-jenkins Jan 9, 2026 •

edited

Loading

rdkcmf-jenkins Jan 9, 2026 •

edited

Loading