Skip to content

rebase#156

Open
Abhinavpv28 wants to merge 24 commits intofeature/rrdl2_backup_with_l2flagfrom
develop
Open

rebase#156
Abhinavpv28 wants to merge 24 commits intofeature/rrdl2_backup_with_l2flagfrom
develop

Conversation

@Abhinavpv28
Copy link
Copy Markdown
Contributor

No description provided.

rdkcmf-jenkins and others added 4 commits June 24, 2025 15:09
Create deepsleep_main.c

Update Makefile

Create power_controller.h

Update run_l2.sh

Update cov_build.sh

Create pwrMgr.h

Update cov_build.sh

Update cov_build.sh

Update Makefile

Rename deepsleep_main.c to main.c

Update Makefile

update Makefile

Delete test/functional-tests/tests/rrdIarmEvents.c

Update run_l2.sh

Update remote_debugger.json

Update remote_debugger.json

Create test_deepsleep_static.py

Update run_l2.sh

Update run_l2.sh

Create test_deepsleep_dynamic.py

Update run_l2.sh

Update rrdInterface.c

Update create_json.sh

Create test_append.py

Create test_category.py

Update test_rrd_dynamic_profile_missing_report.py

Create test_rrd_dynamic_with_download_harmful.py

Create test_rrd_negative.py

Update configure.ac

Update cov_build.sh

Update run_l2.sh

Update main.c

Rename main.c to deepsleep_main.c

Update Makefile

Create rrd_append_report.feature

Create rrd_deepsleep_static.feature

Create rrd_deepsleep_dynamic.feature

Create rrd_dynamic_with_download_harmful_report.feature

Update rrd_dynamic_profile_missing_report.feature

Update test_rrd_dynamic_profile_missing_report.py

Update cov_build.sh

Update test_rrd_single_instance.py

Update test_category.py

Update run_l2.sh

Update test_rrd_dynamic_profile_missing_report.py

Update rrdInterface.c

Update rrdInterface.c

Update rrdInterface.c

Update rrdIarmEvents.c

Update rrdIarmEvents.c

Delete test/functional-tests/tests/test_deepsleep_dynamic.py

Delete test/functional-tests/tests/test_rrd_negative.py

Update run_l2.sh

Update test_category.py

Update cov_build.sh

Update configure.ac

Update configure.ac

Delete test/functional-tests/features/rrd_deepsleep_dynamic.feature

Update rrd_dynamic_profile_missing_report.feature

Update rrd_dynamic_profile_missing_report.feature

Delete test/functional-tests/tests/test_rrd_dynamic_with_download_harmful.py

Delete test/functional-tests/tests/deepsleep_main.c

Delete test/functional-tests/tests/Makefile

Update run_l2.sh

Update run_l2.sh

Delete test/functional-tests/features/rrd_dynamic_with_download_harmful_report.feature

Create rrd_dynamic_profile_subcategory_report.feature

Update run_l2.sh

Rename test_append.py to test_rrd_append_report.py

Rename test_category.py to test_rrd_dynamic_subcategory_report.py

Rename test_deepsleep_static.py to test_rrd_deepsleep_static_report.py

Rename rrd_deepsleep_static.feature to rrd_deepsleep_static_report.feature

Update rrd_dynamic_profile_missing_report.feature

Update rrd_dynamic_profile_missing_report.feature

Update rrd_dynamic_profile_missing_report.feature
RDK-56291 - [RDKE] Increase L2 Test Coverage For Remote Debugger : Target 80% [ Phase 2 ]
@Abhinavpv28 Abhinavpv28 requested a review from a team as a code owner September 24, 2025 15:38
rdkcmf-jenkins and others added 9 commits September 25, 2025 18:24
Deploy fossid_integration_stateless_diffscan_target_repo action
…rget 80% [ Phase 2 ] (#157)

* Update rrdInterface.c

* Update rrdIarmEvents.c

* Update configure.ac

* Update cov_build.sh

* Update remote_debugger.json

* Update create_json.sh

* Create Makefile

* Create deepsleep_main.c

---------

Co-authored-by: nhanasi <navihansi@gmail.com>
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 1, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ Alan-Ryan
✅ Abhinavpv28
✅ venkat0557
❌ naveenkumarhanasi
You have signed the CLA already but the status is still pending? Let us recheck it.

… C (#167)

* Add comprehensive HLD documentation for uploadRRDLogs.sh migration to C

- Requirements document with functional specs, I/O definitions, and constraints
- High-Level Design with architecture, module breakdown, and data flows
- Detailed flowcharts for all major operations (Mermaid + text-based)
- Sequence diagrams showing component interactions
- Low-Level Design with complete C code, data structures, and implementations

Documentation covers migration of uploadRRDLogs.sh to embedded C program
optimized for low-memory, low-CPU embedded systems with full error handling,
security considerations, and platform portability.

- Replace custom MAC retrieval logic with TR-181 parameters via RBus
  - Device.DeviceInfo.X_COMCAST-COM_STB_MAC (Video)
  - Device.DeviceInfo.X_COMCAST-COM_CM_MAC (Broadband)
  - Device.X_CISCO_COM_MACAddress (Alternative)
- Add cache file fallback (/tmp/device_details.cache, /tmp/estb_mac)
- Add early exit optimization in config parser when all required properties found
- Mark libarchive as mandatory dependency, remove tar command fallback
Copilot AI review requested due to automatic review settings December 4, 2025 23:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR appears to be a rebase that includes significant additions for L2 test coverage improvements and new documentation for an uploadRRDLogs C implementation. The changes include:

  • Enhanced test coverage with new test files and improved existing tests
  • Comprehensive documentation for uploadRRDLogs migration from shell script to C
  • New power management integration with deepsleep functionality
  • Build system improvements with L2 support configuration
  • Updated CI/CD workflows and project maintenance files

Reviewed changes

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

Show a summary per file
File Description
test/functional-tests/tests/test_rrd_single_instance.py Improved PID handling with proper list operations
test/functional-tests/tests/test_rrd_dynamic_subcategory_report.py New comprehensive test for dynamic subcategory reporting
test/functional-tests/tests/test_rrd_deepsleep_static_report.py New test for deepsleep static report functionality
test/functional-tests/tests/test_rrd_append_report.py New test for report append functionality
test/functional-tests/tests/power_controller.h New power controller API header with comprehensive interface
test/functional-tests/tests/deepsleep_main.c New C test implementation for deepsleep event handling
test/functional-tests/tests/create_json.sh Updated JSON creation with additional test configurations
test/functional-tests/tests/Makefile New Makefile for compiling C test binaries
test/functional-tests/features/*.feature New Gherkin feature files for BDD test scenarios
src/unittest/mocks/pwrMgr.h New mock header for power manager IARM interface
src/rrdInterface.c Updated RBus subscription logic with L2 support conditionals
src/rrdIarmEvents.c Updated WebCfg force sync error handling with L2 support
run_l2.sh Added new test executions and configuration copy
remote_debugger.json Added DeepSleep configuration profiles
docs/uploadRRDLogs_*.md New comprehensive documentation for uploadRRDLogs C migration
cov_build.sh Updated build script with L2 support flags
configure.ac Added L2 support configuration option
CHANGELOG.md Updated with 1.2.9 release notes
.github/workflows/*.yml Updated CI/CD workflows with improved security
.github/CODEOWNERS Simplified code ownership

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

DEBUG_FILE = "Adding Details of Debug commands to Output File"
assert DEBUG_FILE in grep_rrdlogs(DEBUG_FILE)

issue_string = "DEEPSLEEP"
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Variable issue_string is not used.

Suggested change
issue_string = "DEEPSLEEP"

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,134 @@
import json
from helper_functions import *
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Suggested change
from helper_functions import *
from helper_functions import (
check_file_exists,
check_directory_exists,
kill_rrd,
remove_logfile,
run_shell_silent,
run_shell_command,
grep_rrdlogs,
JSON_FILE,
OUTPUT_DIR,
APPEND_JSON_FILE,
DYNAMIC_DIR,
)

Copilot uses AI. Check for mistakes.
# limitations under the License.
##########################################################################

from helper_functions import *
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Suggested change
from helper_functions import *
from helper_functions import (
JSON_FILE,
check_file_exists,
OUTPUT_DIR,
check_directory_exists,
kill_rrd,
remove_logfile,
run_shell_silent,
run_shell_command,
grep_rrdlogs,
remove_outdir_contents
)

Copilot uses AI. Check for mistakes.

import json
import subprocess
from helper_functions import *
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Suggested change
from helper_functions import *
from helper_functions import (
JSON_FILE,
check_file_exists,
OUTPUT_DIR,
check_directory_exists,
kill_rrd,
remove_logfile,
run_shell_silent,
run_shell_command,
sleep,
grep_rrdlogs,
)

Copilot uses AI. Check for mistakes.
# limitations under the License.
##########################################################################

import json
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Import of 'json' is not used.

Suggested change
import json

Copilot uses AI. Check for mistakes.
Vismalskumar0 and others added 2 commits January 12, 2026 13:55
* RDK-58172: integrate dcm-agent uploadstblogs and cleanup logging; update Makefile and upload flow

* Add unit tests for RRD upload orchestration

This file contains unit tests for the RRD upload orchestration, covering various aspects such as configuration loading, system information retrieval, log directory validation, and the entire upload workflow.

* Update Makefile.am

* Remove uploadRRDLogs program from Makefile

* Refactor uploadDebugoutput to use upload API

Updated the uploadDebugoutput function to call the upload API instead of executing a script. Enhanced logging for upload orchestration success and failure.

* Refactor upload orchestration into rrd_upload_orchestrate

Refactor upload orchestration logic into a separate function and remove deprecated main function.

* Update rrd_config.h

* Include common_device_api.h in rrd_sysinfo.h

Added inclusion of common_device_api.h for device API access.

* Add comprehensive functional tests for C API upload orchestration

- Add BDD feature file with 20 test scenarios for rrd_upload_orchestrate
- Add Python test implementation with 14 test functions
- Test coverage includes:
  * Valid and invalid parameter handling
  * Configuration loading from multiple sources (properties, RFC, DCM)
  * MAC address retrieval and timestamp generation
  * Issue type sanitization and normalization
  * Archive creation and format validation
  * Upload execution and lock handling
  * Cleanup operations
  * Error code propagation (codes 1-11)
  * Comprehensive logging verification
  * Integration with uploadDebugoutput wrapper

- Tests validate the complete flow from event trigger through C API
  execution to upload completion
- Helper class provides utilities for test setup, archive validation,
  and log verification

Refactor source validation and log handling. Improve error handling for directory operations and add functionality to move live logs.

* Refactor log upload handling and cleanup process
Updated log upload process to handle LOGUPLOAD_ENABLE case and adjusted archive creation and upload paths.
* RDKEMW-12334-The log entries in remote-debugger.log 

RDKEMW-12334-The log entries in remote-debugger.log differ between C binary files and script files when compared

* Change the log from Debug to Info
* Apply suggestions from code review

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

---------

Co-authored-by: Abhinavpv28 <162570454+Abhinavpv28@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 21, 2026 19:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated 9 comments.


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

Comment on lines +149 to +150
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The variable names SCRIPT_SUCCESS and SCRIPT_FAILURE appear to be swapped. SCRIPT_SUCCESS is assigned "Debug Information Report upload Failed" but should be "Debug Information Report upload Success". Similarly, SCRIPT_FAILURE is assigned "Debug Information Report upload Success" but should be "Debug Information Report upload Failed".

Copilot uses AI. Check for mistakes.
if '.' in ISSUE_STRING:
ISSUE_NODE, ISSUE_SUBNODE = ISSUE_STRING.split('.')
else:
node = ISSUE_STRING
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Variable node is not used.

Copilot uses AI. Check for mistakes.
ISSUE_NODE, ISSUE_SUBNODE = ISSUE_STRING.split('.')
else:
node = ISSUE_STRING
subnode = None
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Variable subnode is not used.

Copilot uses AI. Check for mistakes.
node = ISSUE_STRING
subnode = None

ISSUE_FOUND = f"Issue Data Node: {ISSUE_NODE} and Sub-Node: {ISSUE_SUBNODE} found in Static JSON File /etc/rrd/remote_debugger.json"
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Variable ISSUE_FOUND is not used.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,134 @@
import json
from helper_functions import *
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Copilot uses AI. Check for mistakes.
# limitations under the License.
##########################################################################

from helper_functions import *
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Copilot uses AI. Check for mistakes.
# limitations under the License.
##########################################################################

from helper_functions import *
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Copilot uses AI. Check for mistakes.

import json
import subprocess
from helper_functions import *
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.

Copilot uses AI. Check for mistakes.
# limitations under the License.
##########################################################################

import json
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Import of 'json' is not used.

Copilot uses AI. Check for mistakes.
nhanasi and others added 4 commits January 22, 2026 19:45
…ode (#178)

* Update Makefile.am

* Update rrdExecuteScript.c

* Update rrd_upload.h

* Update Makefile.am

* Update Makefile.am

* Update Makefile.am

* Update Makefile.am

---------

Co-authored-by: nhanasi <navihansi@gmail.com>
Copilot AI review requested due to automatic review settings February 17, 2026 16:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated 9 comments.

mkdir -p /media/apps/RDK-RRD-Test/etc/rrd

touch /media/apps/RDK-RRD-Test/etc/rrd/remote_debugger.json
echo "AA:BB:CC:DD:EE:FF" >> /tmp/.estb_mac
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

This appends the mock MAC to /tmp/.estb_mac on every run (>>), which can leave multiple lines and break any consumer expecting a single MAC value. Use overwrite (>) and/or ensure the file is truncated/rewritten deterministically.

Suggested change
echo "AA:BB:CC:DD:EE:FF" >> /tmp/.estb_mac
echo "AA:BB:CC:DD:EE:FF" > /tmp/.estb_mac

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +76
AC_ARG_ENABLE([L2support],
AS_HELP_STRING([--enable-L2support],[enable L2support (default is no)]),
[
case "${enableval}" in
yes) L2_SUPPORT_ENABLE=true
L2_SUPPORT_FLAG="-DUSE_L2_SUPPORT"
m4_if(m4_sysval,[0],[SUBDIRS_L2_SUPPORT="src"]) ;;
no) L2_SUPPORT_ENABLE=false AC_MSG_ERROR([L2_SUPPORT is disabled]) ;;
*) AC_MSG_ERROR([bad value ${enableval} for --enable-L2support]) ;;
esac
],
[echo "L2support is disabled"])
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The --enable-L2support option sets L2_SUPPORT_FLAG/L2_SUPPORT_ENABLE, but these variables are never AC_SUBST’d or wired into any AM_CONDITIONAL/Makefile logic, so the flag has no effect on the build. Either plumb this through (e.g., AC_SUBST(L2_SUPPORT_FLAG) and append it in src/Makefile.am, or add an AM_CONDITIONAL) or remove the option to avoid misleading configure behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +42
/* Get the device MAC address.
* @param mac_addr Buffer to store MAC address (min 18 bytes)
* @param size Size of buffer
* @return 0 on success, -1 on error
*/
int rrd_sysinfo_get_mac_address(char *mac_addr, size_t size);
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The header comment says the MAC buffer needs at least 18 bytes, but the implementation accepts 13 bytes (12 hex chars + NUL) after stripping colons. Update the comment (or the implementation) so the contract is consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +26
# Path to the existing JSON file
file_path = "/etc/rrd/remote_debugger.json"

# Read the existing JSON data
with open(file_path, "r") as json_file:
data = json.load(json_file)

# New entry to add
new_entry = {
"Test": {
"TestRun4": {
"Commands": "cat /version.txt;cat /tmp/.deviceDetails.cache",
"Timeout": 10
}
}
}

# Update the JSON data with the new entry
data.update(new_entry)

# Write the updated data back to the JSON file
with open(file_path, "w") as json_file:
json.dump(data, json_file, indent=4)
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

This test modifies /etc/rrd/remote_debugger.json at import/collection time (top-level file I/O). If the file is missing or not writable, pytest will fail before running any tests, and it also creates global side effects that can leak into other tests. Move the JSON read/update/write into a fixture or a dedicated test function with proper setup/teardown (and preferably write to a temporary copy instead of editing the system file in place).

Copilot uses AI. Check for mistakes.
Comment on lines +162 to +167
# Verify archive was created
ARCHIVE_CREATED = "Archive created:"
if ARCHIVE_CREATED in grep_rrdlogs(ARCHIVE_CREATED):
print("Archive creation confirmed")
else:
print("Archive creation not found in logs")
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The test looks for the literal string "Archive created:" in logs, but the new archive implementation logs "Archive created successfully:" (see rrd_archive_create). With the current search string (including the colon immediately after "created"), this will not match and will produce false negatives. Update the expected log substring to match the new message (e.g., "Archive created successfully" or "Archive created successfully:").

Copilot uses AI. Check for mistakes.
Comment on lines +188 to +204
int rrd_upload_cleanup_source_dir(const char *dir_path) {
if (!dir_path) return -1;

RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "%s: Removing source directory: %s\n", __FUNCTION__, dir_path);

char cmd[1024];
snprintf(cmd, sizeof(cmd), "rm -rf %s", dir_path);

int ret = system(cmd);
if (ret == 0) {
RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "%s: Successfully removed source directory: %s\n",
__FUNCTION__, dir_path);
return 0;
} else {
RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "%s: Failed to remove source directory: %s (ret: %d)\n",
__FUNCTION__, dir_path, ret);
return -1;
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

rrd_upload_cleanup_source_dir shells out to rm -rf %s using system() with an unquoted path. Since dir_path ultimately comes from runtime inputs (upload directory), this is command-injection prone and can also mis-handle paths with spaces/shell metacharacters. Replace this with a safe recursive delete implementation (e.g., nftw()/FTS, or explicit opendir/readdir/unlink/rmdir) and avoid invoking a shell.

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +55
typedef enum PowerController_ThermalTemperature {
THERMAL_TEMPERATURE_UNKNOWN = 0 /* UNKNOWN Thermal Temperature */,
THERMAL_TEMPERATURE_NORMAL = 1 /* Normal Thermal Temperature */,
THERMAL_TEMPERATURE_HIGH = 2 /* High Thermal Temperature */,
THERMAL_TEMPERATURE_CRITICAL = 4 /* Critial Thermal Temperature */
} PowerController_ThermalTemperature_t;
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Spelling: "Critial" should be "Critical" in the enum comment/value description to keep the public header clean and searchable.

Copilot uses AI. Check for mistakes.
Comment on lines +149 to +154
SCRIPT_SUCCESS = "Debug Information Report upload Failed"
SCRIPT_FAILURE = "Debug Information Report upload Success"
if SCRIPT_SUCCESS in grep_rrdlogs(SCRIPT_SUCCESS):
print("Script execution success")
elif SCRIPT_FAILURE in grep_rrdlogs(SCRIPT_FAILURE):
print("Script execution failed")
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

SCRIPT_SUCCESS/SCRIPT_FAILURE are assigned to the opposite log messages ("upload Failed" vs "upload Success"), and the subsequent prints also invert success/failure. This makes the test output misleading and can hide real failures; swap the constants/branches so success corresponds to the success log line.

Copilot uses AI. Check for mistakes.
Comment on lines +79 to +86
if '.' in ISSUE_STRING:
ISSUE_NODE, ISSUE_SUBNODE = ISSUE_STRING.split('.')
else:
node = ISSUE_STRING
subnode = None

ISSUE_FOUND = f"Issue Data Node: {ISSUE_NODE} and Sub-Node: {ISSUE_SUBNODE} found in Static JSON File /etc/rrd/remote_debugger.json"

Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

In the else branch, ISSUE_NODE/ISSUE_SUBNODE are never set, but they are still used to build ISSUE_FOUND below. This will raise UnboundLocalError when ISSUE_STRING has no '.' (and currently the default ISSUE_STRING in helper_functions is Device.Info, but this test should be correct for all inputs). Initialize ISSUE_NODE/ISSUE_SUBNODE in both branches (or build the expected log message conditionally).

Copilot uses AI. Check for mistakes.
madhubabutt and others added 4 commits February 25, 2026 15:11
This pull request addresses Coverity defects in the RDK Remote Debugger Device Management code, focusing on fixing memory leaks, buffer overflows, and resource handling issues.

Changes:

Added NULL checks for memory allocations (malloc, strdup, fread) with proper error handling and resource cleanup
Fixed buffer overflow vulnerabilities by using correct size parameters in strncpy/strncat operations
Corrected file pointer closure to only occur when pointers are valid (v_secure_pclose moved inside if blocks)
Fixed memory management issues with rfcbuf allocations and ownership





Co-authored-by: mtirum011 <madhubabu_tirumala@comcast.com>
Remote Debugger 1.3.2 release
Copilot AI review requested due to automatic review settings March 20, 2026 19:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 11 comments.

Comment on lines +65 to +73
AC_ARG_ENABLE([L2support],
AS_HELP_STRING([--enable-L2support],[enable L2support (default is no)]),
[
case "${enableval}" in
yes) L2_SUPPORT_ENABLE=true
L2_SUPPORT_FLAG="-DUSE_L2_SUPPORT"
m4_if(m4_sysval,[0],[SUBDIRS_L2_SUPPORT="src"]) ;;
no) L2_SUPPORT_ENABLE=false AC_MSG_ERROR([L2_SUPPORT is disabled]) ;;
*) AC_MSG_ERROR([bad value ${enableval} for --enable-L2support]) ;;
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The new --enable-L2support option fails the configure step when explicitly set to "no" (AC_MSG_ERROR), and the computed L2_SUPPORT_FLAG/SUBDIRS_L2_SUPPORT variables are never AC_SUBST’d or used anywhere else. This makes the option either unusable or a no-op; consider treating "no" as a normal value and wiring the flag via AC_SUBST/AM_CONDITIONAL so it actually affects compilation.

Copilot uses AI. Check for mistakes.
#endif
#if !defined(GTEST_ENABLE)
rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, 3);
ret = rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, 3);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

RRD_subscribe() may subscribe only 2 RBUS events (when IARMBUS_SUPPORT && !USE_L2_SUPPORT), but RRD_unsubscribe() always unsubscribes 3. This mismatch can cause RBUS unsubscribe to fail or access an unsubscribed slot; track the subscribed count (2 vs 3) and pass the same count into rbusEvent_UnsubscribeEx().

Suggested change
ret = rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, 3);
int subscriptionCount = 3;
#if defined(IARMBUS_SUPPORT) && !defined(USE_L2_SUPPORT)
subscriptionCount = 2;
#endif
ret = rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, subscriptionCount);

Copilot uses AI. Check for mistakes.
Comment on lines +191 to +197
RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "%s: Removing source directory: %s\n", __FUNCTION__, dir_path);

char cmd[1024];
snprintf(cmd, sizeof(cmd), "rm -rf %s", dir_path);

int ret = system(cmd);
if (ret == 0) {
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

rrd_upload_cleanup_source_dir() builds a shell command with unquoted dir_path and executes it via system("rm -rf %s"). If dir_path contains spaces or shell metacharacters, this can delete unintended paths or enable command injection. Prefer a safe recursive delete implementation (nftw/unlinkat) or at minimum avoid invoking a shell / properly escape and validate the path.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +40
// Trim leading
while (*start == ' ' || *start == '\t' || *start == '\n' || *start == '\r') start++;

// Trim trailing
end = start + strlen(start) - 1;
while (end > start && (*end == ' ' || *end == '\t' || *end == '\n' || *end == '\r')) *end-- = 0;

Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

trim() computes end = start + strlen(start) - 1 without handling the case where the string becomes empty after trimming leading whitespace. When strlen(start) == 0, this does pointer arithmetic to before the buffer (undefined behavior). Add an early return when *start == '\0' before calculating end.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +60
def reset_issuetype_rfc():
command = 'rbuscli set Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.RDKRemoteDebugger.IssueType string ""'
result = subprocess.run(command, shell=True, capture_output=True, text=True)
assert result.returncode == 0

Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

subprocess is used in reset_issuetype_rfc() (subprocess.run) but this file does not import subprocess. This will raise NameError at runtime; add the missing import or route through an existing helper that wraps command execution.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +33
def reset_issuetype_rfc():
command = 'rbuscli set Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.RDKRemoteDebugger.IssueType string ""'
result = subprocess.run(command, shell=True, capture_output=True, text=True)
assert result.returncode == 0
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

reset_issuetype_rfc() uses subprocess.run but subprocess is not imported in this file. This will raise NameError at runtime; add the missing import.

Copilot uses AI. Check for mistakes.
}
}
},
"DeepSleep": {
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The codebase constant for deep sleep handling is DEEP_SLEEP_STR="DEEPSLEEP" (src/rrdCommon.h). This JSON adds a category key "DeepSleep" which will not be found by the case-sensitive JSON lookup (cJSON_GetObjectItem). Consider renaming the key to "DEEPSLEEP" (or making lookups case-insensitive) so deep sleep events resolve correctly.

Suggested change
"DeepSleep": {
"DEEPSLEEP": {

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +41
And the directory should be created to store the executed output
And Sanity check to validate the commands should be executed
And Command output shopuld be added to the output file
And the issuetype systemd service should start successfully
And the journalctl service should start successfully
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Typo in step text: "shopuld" should be "should".

Copilot uses AI. Check for mistakes.
Comment on lines +47 to +52
And the directory should be created to store the executed output
And Sanity check to validate the commands should be executed
And Command output shopuld be added to the output file
And the issuetype systemd service should start successfully
And the journalctl service should start successfully
And the process should sleep with timeout
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Typo in step text: "shopuld" should be "should".

Copilot uses AI. Check for mistakes.
if [ ! -d tr69hostif ]; then
git clone https://github.com/rdkcentral/tr69hostif.git
fi
git clone https://github.com/rdkcentral/dcm-agent.git -b develop
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

dcm-agent is cloned unconditionally. If cov_build.sh is re-run in an environment where /usr/dcm-agent already exists, git clone will fail and stop the build. Consider guarding it like the other dependencies (clone only if the directory is missing) or doing a git -C dcm-agent fetch/reset if it already exists.

Suggested change
git clone https://github.com/rdkcentral/dcm-agent.git -b develop
if [ ! -d dcm-agent ]; then
git clone https://github.com/rdkcentral/dcm-agent.git -b develop
else
cd dcm-agent
git fetch origin develop
git reset --hard origin/develop
cd ..
fi

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.