Conversation
…orms (#88) * Update rrdDynamic.c * Update rrdDynamic.c * Update rrdEventProcess.c * Update rrdInterface.c
|
Saranya seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
RDKECMF-219 Remove redundant variable
…102) * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdCommandSanity.c * Update rrdJsonParser.c * Update rrdMain.c * Update rrdMain.c * Update rrdMain.c * Update rrdEventProcess.c * Update rrdInterface.c * Update rrdEventProcess.c
* RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin with retry Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com * RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com * RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com * RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com * RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com * RDK-55702: Update the MW clients to use Power Manager Plugin Reason for change: Update the MW clients to use Power Manager Plugin Test Procedure: Refer RDK-55702 Risks: Low Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com --------- Co-authored-by: nhanasi <navihansi@gmail.com>
Merge changes related to L1, L2 workflow and Iarmbus plugin changes
…ntral/remote_debugger into feature/RDK-56115-coverity
It should not be needed with registry being opensourced
RDKECMF-219 Enable component build workflow on Pull Request and remove unnecessary token
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>
RRD 1.2.9 release
… 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
There was a problem hiding this comment.
Pull request overview
This PR appears to be a rebase that includes:
- Enhanced functional test coverage for dynamic profile handling, deep sleep scenarios, and append operations
- Improvements to PID handling in single instance tests
- New test infrastructure for dynamic profiles with subcategory and harmful command detection
- Unit test updates to support RBus API integration and PowerManager plugin support
- Documentation additions including requirements, design documents, and sequence diagrams for uploadRRDLogs
- Build system updates to support L2 testing and remove deprecated dependencies
- Source code improvements including memory leak fixes, null pointer checks, and CPU-aware priority management
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| test/functional-tests/tests/test_rrd_single_instance.py | Improved PID handling to support multiple PIDs and better validation |
| test/functional-tests/tests/test_rrd_dynamic_subcategory_report.py | New test for dynamic profile subcategory handling |
| test/functional-tests/tests/test_rrd_dynamic_profile_report.py | New test for complete dynamic profile processing |
| test/functional-tests/tests/test_rrd_dynamic_profile_missing_report.py | New test for missing dynamic profile scenarios |
| test/functional-tests/tests/test_rrd_dynamic_profile_harmful_report.py | New test for harmful command detection |
| test/functional-tests/tests/test_rrd_deepsleep_static_report.py | New test for deep sleep static report scenarios |
| test/functional-tests/tests/test_rrd_append_report.py | New test for append request handling |
| test/functional-tests/tests/test_rrd_append_dynamic_profile_static_notfound.py | New test for append with missing static profile |
| test/functional-tests/tests/power_controller.h | New header for PowerController API definitions |
| test/functional-tests/tests/helper_functions.py | Added constants for dynamic testing |
| test/functional-tests/tests/deepsleep_main.c | New test executable for deep sleep event handling |
| test/functional-tests/tests/create_json.sh | New script for dynamic JSON profile creation |
| test/functional-tests/tests/Makefile | New makefile for test executable compilation |
| src/unittest/rrdUnitTestRunner.cpp | Updated unit tests for RBus API and power manager integration |
| src/unittest/mocks/pwrMgr.h | New mock header for power manager API |
| src/unittest/mocks/Client_Mock.h | Updated mocks to support RBus get operation |
| src/unittest/mocks/Client_Mock.cpp | Implemented RBus get mock function |
| src/rrdRunCmdThread.c | Added RemainAfterExit flag and conditional compilation guards |
| src/rrdMain.c | Minor formatting change |
| src/rrdJsonParser.c | Fixed resource leak and added null checks |
| src/rrdInterface.h | Updated for PowerManager plugin support |
| src/rrdInterface.c | Added RBus API integration and improved event handling |
| src/rrdIarmEvents.c | Added PowerManager plugin support with threading |
| src/rrdEventProcess.c | Added memory safety improvements and null checks |
| src/rrdDynamic.c | Fixed string copy buffer overflow |
| src/rrdCommon.h | Added conditional power_controller.h include |
| src/rrdCommandSanity.c | Improved command sanitization with quote and whitespace handling |
| remote_debugger.json | Added DeepSleep configuration |
| docs/uploadRRDLogs_*.md | New comprehensive documentation for uploadRRDLogs migration |
| run_l2.sh | Added new test cases and directory setup |
| cov_build.sh | Updated build flags and removed deprecated include |
| configure.ac | Added L2 support configuration option |
| .github/workflows/*.yml | Updated CI/CD workflows |
💡 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" |
There was a problem hiding this comment.
Variable issue_string is not used.
| DYNAMIC_JSONFILE = "Reading json config file /media/apps/RDK-RRD-Test/etc/rrd/remote_debugger.json" | ||
| assert DYNAMIC_JSONFILE in grep_rrdlogs(DYNAMIC_JSONFILE) | ||
|
|
||
| PARSE_SUCCESS = "Dynamic Profile Parse And Read Success..." |
There was a problem hiding this comment.
Variable PARSE_SUCCESS is not used.
| ########################################################################## | ||
|
|
||
| import json | ||
| from helper_functions import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.
| @@ -0,0 +1,134 @@ | |||
| import json | |||
| from helper_functions import * | |||
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| from helper_functions import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
* 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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 74 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if '.' in ISSUE_STRING: | ||
| ISSUE_NODE, ISSUE_SUBNODE = ISSUE_STRING.split('.') | ||
| else: | ||
| node = ISSUE_STRING |
There was a problem hiding this comment.
Variable node is not used.
| ISSUE_NODE, ISSUE_SUBNODE = ISSUE_STRING.split('.') | ||
| else: | ||
| node = ISSUE_STRING | ||
| subnode = None |
There was a problem hiding this comment.
Variable subnode is not used.
| 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" |
There was a problem hiding this comment.
Variable ISSUE_FOUND is not used.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| from helper_functions import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module helper_functions does not define 'all'.
RRD release 1.3.1
…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>
| j--; | ||
| } | ||
| RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Checking for \"%s\" string in Issue commands... \n",__FUNCTION__,__LINE__,checkcmd); | ||
| sanitystr = strstr(issuecmd,checkcmd); |
There was a problem hiding this comment.
Memory leak: The 'checkcmd' pointer is modified to skip the opening quote (line 139), but the original pointer allocated by cJSON_Print() is lost. When this memory needs to be freed later, you won't have the original pointer. Store the original pointer before modifying it, or use a different variable for the adjusted pointer.
| } else if (name_len <= 255) { | ||
| /* split into prefix and name */ | ||
| size_t prefix_len = name_len - 100 - 1; /* leave room for null */ | ||
| if (prefix_len > sizeof(hdr.prefix)) { | ||
| RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s] File name too long: %s (length %zu)\n", | ||
| __FUNCTION__, name, name_len); | ||
| return -1; | ||
| } | ||
| strncpy(hdr.prefix, name, prefix_len); | ||
| hdr.prefix[prefix_len < sizeof(hdr.prefix) ? prefix_len : sizeof(hdr.prefix) - 1] = '\0'; | ||
| strncpy(hdr.name, name + prefix_len + 1, sizeof(hdr.name) - 1); | ||
| hdr.name[sizeof(hdr.name) - 1] = '\0'; |
There was a problem hiding this comment.
Potential buffer overflow: When splitting the name into prefix and name, the calculation 'prefix_len = name_len - 100 - 1' may not correctly handle cases where the name needs to be split. This could lead to incorrect string operations. Additionally, after setting the prefix, copying to hdr.name uses 'name + prefix_len + 1' which might skip a character unintentionally.
| 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") | ||
| else: | ||
| print("Script execution not found in logs") |
There was a problem hiding this comment.
Logic error in test: The variable names SCRIPT_SUCCESS and SCRIPT_FAILURE are swapped. The check for "Debug Information Report upload Failed" is stored in SCRIPT_SUCCESS, and "Debug Information Report upload Success" is in SCRIPT_FAILURE. This will cause incorrect test results.
| 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") | ||
| else: | ||
| print("Script execution not found in logs") |
There was a problem hiding this comment.
Same logic error: SCRIPT_SUCCESS and SCRIPT_FAILURE variable names are swapped, with "Failed" assigned to SUCCESS and "Success" assigned to FAILURE. This duplicates the error from test_rrd_dynamic_profile_report.py.
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
| issue =rbusValue_GetString(value, NULL); | ||
| char *dot_position = strchr(issue, '.'); // Find the first occurrence of '.' | ||
| if (dot_position != NULL) | ||
| { | ||
| *dot_position = '\0'; // Replace '.' with null terminator | ||
| } |
There was a problem hiding this comment.
issue is a const char* returned by rbusValue_GetString(), but the code modifies it in-place by replacing '.' with '\0'. This risks corrupting RBUS-managed memory and can crash/produce incorrect values. Copy the string into a writable buffer (e.g., strdup/snprintf into a local array), modify the copy, and keep the RBUS value immutable.
No description provided.