RDKEMW-13117: [DONOT MERGE] Use IARM for PowerManager clients which uses PowerController ThunderClientLibrary #346
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request reverts changes from PR #47 that introduced the PowerController Thunder plugin, returning to using IARM Bus for power management in the tr69hostif middleware. The PR is explicitly marked "[DONOT MERGE]" in the title, indicating it should not be merged.
Changes:
- Reverts PowerController ThunderClientLibrary integration back to IARM-based power management
- Removes PowerController stub files and header
- Restores IARM Bus calls for power state retrieval and event handling
- Removes threading code for PowerController initialization
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unittest/stubs/powerctrl_stubs.cpp | Deleted PowerController stub implementation |
| src/unittest/stubs/power_controller.h | Deleted PowerController header file |
| src/unittest/stubs/dm_stubs.cpp | Removed PowerController stub functions and header include |
| src/hostif/profiles/DeviceInfo/Device_DeviceInfo.h | Removed setPowerConInterface method declaration |
| src/hostif/profiles/DeviceInfo/Device_DeviceInfo.cpp | Reverted power status retrieval to IARM Bus API |
| src/hostif/handlers/src/hostIf_IARM_ReqHandler.cpp | Removed PowerController threading, restored IARM event handler |
| src/hostif/profiles/DeviceInfo/gtest/Makefile.am | Removed extra blank line |
| src/Makefile.am | Removed WPEFrameworkPowerController library dependency |
| cov_build.sh | Removed PowerController library build steps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in power controller thunder cleint call for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, pwr_ret); | ||
| strncpy((char *)stMsgData->paramValue, pwrState, str_len); | ||
| stMsgData->paramValue[str_len+1] = '\0'; |
There was a problem hiding this comment.
Potential buffer overflow: The code writes a null terminator at position str_len+1, but when using strncpy with str_len bytes, the string is already str_len characters long. The null terminator should be placed at position str_len, not str_len+1. This could write one byte beyond the intended boundary when the string length equals str_len.
| stMsgData->paramValue[str_len+1] = '\0'; | |
| stMsgData->paramValue[str_len] = '\0'; |
| else | ||
| { | ||
| RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Powercontroller Interface failed : %d. Try after sometime. \n", bPowerControllerEnable); | ||
| RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, ret); |
There was a problem hiding this comment.
Incorrect error code in log message: The error log uses ret variable which is always NOK at this point. The actual IARM error code should be logged using err instead of ret to provide meaningful diagnostics information.
| RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, ret); | |
| RDK_LOG(RDK_LOG_ERROR,LOG_TR69HOSTIF,"Failed in IARM_Bus_Call() for parameter : %s [param.type:%s with error code:%d]\n",stMsgData->paramName, pwrState, err); |
| std::string event_time_string = std::to_string(std::time(nullptr)); | ||
| event_time = event_time_string.c_str(); | ||
| NotificationHandler::getInstance()->push_device_deepsleep_notifications("device-enter-deepsleep-state", event_time); |
There was a problem hiding this comment.
Dangling pointer issue: The event_time pointer is set to point to the c_str() of a local std::string object (event_time_string) which goes out of scope at the end of each if block. The pointer then becomes invalid before being used in the push_device_deepsleep_notifications call. The string should be kept alive until after the notification is pushed, or the notification function should accept a std::string instead.
| std::string event_time_string = std::to_string(std::time(nullptr)); | ||
| event_time = event_time_string.c_str(); | ||
| NotificationHandler::getInstance()->push_device_deepsleep_notifications("device-exit-deepsleep-state", event_time); |
There was a problem hiding this comment.
Dangling pointer issue: The event_time pointer is set to point to the c_str() of a local std::string object (event_time_string) which goes out of scope at the end of this if block. The pointer then becomes invalid before being used in the push_device_deepsleep_notifications call. The string should be kept alive until after the notification is pushed, or the notification function should accept a std::string instead.
1 participant
Revert "Merge pull request #47 from rdkcentral/feature/RDK-55702-use-pwr-plugin_retry"
This reverts commit 09d4d8c, reversing changes made to 6f7faef.