-
Notifications
You must be signed in to change notification settings - Fork 4
RDK-60308 [tr69hostif, RFC] RDK Coverity Defect Resolution for Device #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 modernizes C++ string initialization and improves code quality in the RFC (Runtime Feature Control) handler by converting copy initialization to direct initialization and adding proper control flow in a switch statement.
Changes:
- Updated string variable initializations from copy initialization (
std::string var = "value") to direct initialization (std::string var("value")) across multiple functions - Added missing
breakstatements and adefaultcase to the curl error code switch statement for proper control flow - Optimized string assignments by using
std::move()semantics for accountId and partnerId to avoid unnecessary copies
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this 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 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this 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 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this 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 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please share the Dev Test results for RDKV and RDKB |
There was a problem hiding this 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 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rfcMgr/rfc_manager.cpp
Outdated
| rdk_logger_ext_config_t config = { | ||
| .pModuleName = "LOG.RDK.RFC", /* Module name */ | ||
| .loglevel = RDK_LOG_INFO, /* Default log level */ | ||
| .output = RDKLOG_OUTPUT_CONSOLE, /* Output to console (stdout/stderr) */ | ||
| .format = RDKLOG_FORMAT_WITH_TS, /* Timestamped format */ | ||
| .pFilePolicy = NULL /* Not using file output, so NULL */ | ||
| }; | ||
|
|
||
| if (rdk_logger_ext_init(&config) != RDK_SUCCESS) { | ||
| printf("RFC : ERROR - Extended logger init failed\n"); | ||
| } | ||
|
|
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed indentation detected: the rdk_logger_ext_config_t initialization and the if statement use spaces, but the indentation at lines 46 and 57 uses tabs. Please use consistent indentation (spaces) throughout this block to match the surrounding code style.
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME); | ||
| if (errno != ENOENT) | ||
| { | ||
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s,errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME,errno); |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message format is inconsistent with other log messages in the same function. It should have a space after [%d] before "Failed", and a space after the comma before "errno". Compare with lines 1505 and 1510 which follow the pattern "[%s][%d] ". The message should be: "[%s][%d] Failed to remove file %s, errno=%d\n"
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s,errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME,errno); | |
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d] Failed to remove file %s, errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME, errno); |
| if (errno != ENOENT) | ||
| { | ||
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s,errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME,errno); | ||
| } |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation here uses tabs, which is inconsistent with the surrounding code in this file that uses spaces (see lines 1484-1522). Please use consistent indentation (spaces) to match the surrounding code style.
| if (errno != ENOENT) | |
| { | |
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s,errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME,errno); | |
| } | |
| if (errno != ENOENT) | |
| { | |
| RDK_LOG(RDK_LOG_ERROR, LOG_RFCMGR,"[%s][%d]Failed to remove file %s,errno=%d\n", __FUNCTION__, __LINE__, DIRECT_BLOCK_FILENAME,errno); | |
| } |
rfcMgr/rfc_manager.cpp
Outdated
|
|
||
| rdk_logger_init(0 == access(OVERIDE_DEBUG_INI_FILE, R_OK) ? OVERIDE_DEBUG_INI_FILE : DEBUG_INI_FILE); |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initializing the extended logger with rdk_logger_ext_init and then immediately calling rdk_logger_init may cause conflicts or unexpected behavior. The rdk_logger_init call on line 58 may override the extended logger configuration. Consider whether both initializations are necessary, or if only one should be used. If both are needed, add a comment explaining why both logger initialization methods are required.
| rdk_logger_init(0 == access(OVERIDE_DEBUG_INI_FILE, R_OK) ? OVERIDE_DEBUG_INI_FILE : DEBUG_INI_FILE); |
There was a problem hiding this 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 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rfcMgr/rfc_manager.cpp
Outdated
| if (rdk_logger_ext_init(&config) != RDK_SUCCESS) { | ||
| printf("RFC : ERROR - Extended logger init failed\n"); | ||
| } | ||
|
|
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation uses tabs instead of spaces, which is inconsistent with the surrounding code. Lines 47-56 use spaces for indentation. This creates mixed indentation within the same function.
|
|
||
| static char RFCMGRLOG[] = "LOG.RDK.RFCMGR"; | ||
| rdk_logger_ext_config_t config = { | ||
| .pModuleName = RFCMGRLOG, /* Module name */ | ||
| .loglevel = RDK_LOG_INFO, /* Default log level */ | ||
| .output = RDKLOG_OUTPUT_CONSOLE, /* Output to console (stdout/stderr) */ | ||
| .format = RDKLOG_FORMAT_WITH_TS, /* Timestamped format */ | ||
| .pFilePolicy = NULL /* Not using file output, so NULL */ | ||
| }; | ||
|
|
||
| if (rdk_logger_ext_init(&config) != RDK_SUCCESS) { | ||
| printf("RFC : ERROR - Extended logger init failed\n"); | ||
| } | ||
|
|
||
| rdk_logger_init(0 == access(OVERIDE_DEBUG_INI_FILE, R_OK) ? OVERIDE_DEBUG_INI_FILE : DEBUG_INI_FILE); | ||
|
|
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two logger initialization calls are made in sequence: rdk_logger_ext_init with console output configuration, followed immediately by rdk_logger_init with file-based configuration. If these initializers have different behaviors or override each other's settings, this could lead to unexpected logging behavior. Please verify that both initializers are necessary and that their configurations are compatible.
| static char RFCMGRLOG[] = "LOG.RDK.RFCMGR"; | |
| rdk_logger_ext_config_t config = { | |
| .pModuleName = RFCMGRLOG, /* Module name */ | |
| .loglevel = RDK_LOG_INFO, /* Default log level */ | |
| .output = RDKLOG_OUTPUT_CONSOLE, /* Output to console (stdout/stderr) */ | |
| .format = RDKLOG_FORMAT_WITH_TS, /* Timestamped format */ | |
| .pFilePolicy = NULL /* Not using file output, so NULL */ | |
| }; | |
| if (rdk_logger_ext_init(&config) != RDK_SUCCESS) { | |
| printf("RFC : ERROR - Extended logger init failed\n"); | |
| } | |
| rdk_logger_init(0 == access(OVERIDE_DEBUG_INI_FILE, R_OK) ? OVERIDE_DEBUG_INI_FILE : DEBUG_INI_FILE); | |
| rdk_logger_init(0 == access(OVERIDE_DEBUG_INI_FILE, R_OK) ? OVERIDE_DEBUG_INI_FILE : DEBUG_INI_FILE); |
There was a problem hiding this 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 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
RDK-60308 [tr69hostif, RFC] RDK Coverity Defect Resolution for Device
Reason for change :Resolved the coverity issues reported by Cov scans.