RDKB-64347: Fixing coverity issues#1062
RDKB-64347: Fixing coverity issues#1062bharathivelp wants to merge 2 commits intordkcentral:developfrom
Conversation
Reason for change: Fixing high medium priority coverity issues. Test Procedure: Build should be successful and the regression test should also succeed. Risks: Low Priority: P1 Signed-off-by: Velpula_Bharathi@comcast.com
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size_t current_len = strlen(assoc_maclist); | ||
| if (current_len + strlen(mac_str) + 2 <= 2048) { | ||
| strncat(assoc_maclist, mac_str, 2048 - current_len - 1); | ||
| strncat(assoc_maclist, ",", 2048 - strlen(assoc_maclist) - 1); | ||
| has_successful_operations = true; |
There was a problem hiding this comment.
The buffer size 2048 is hard-coded in multiple related operations (allocation, memset, bounds check, and strncat limits). To reduce the risk of future mismatches/bugs, define a single named constant (or reuse an existing one) for the assoc MAC list buffer size and use it consistently in these calculations.
| ovsh_where_num++; | ||
| ovsh_where_expr = (char**)realloc(ovsh_where_expr, sizeof(char*) * ovsh_where_num); | ||
| ovsh_where_expr = (char**)realloc(ovsh_where_expr, sizeof(*ovsh_where_expr) * ovsh_where_num); | ||
| assert(ovsh_where_expr); | ||
| ovsh_where_expr[ovsh_where_num-1] = strdup(_str); |
There was a problem hiding this comment.
realloc() is assigned directly to ovsh_where_expr and then only checked via assert(). If realloc() fails, the original pointer is leaked (because it gets overwritten with NULL), and in release builds with NDEBUG the assert() may be compiled out leading to a NULL dereference on the next line. Use a temporary pointer for realloc(), check for NULL, and gracefully return/goto error (also consider rolling back ovsh_where_num on failure).
| @@ -1217,8 +1217,6 @@ int main(int argc, char *argv[]) | |||
| csi_sub_index++; | |||
| g_csi_levl_sub = true; | |||
| break; | |||
There was a problem hiding this comment.
This switch statement no longer includes a default case. The project style guide explicitly requires always including a default statement in switches (see CODE_STYLE.md “Switch statement” section). Please re-add a default: branch (even if it just breaks).
| break; | |
| break; | |
| default: | |
| break; |
Reason for change: Fixing high medium priority coverity issues.
Test Procedure: Build should be successful and the regression test should also succeed.
Risks: Low
Priority: P1
Signed-off-by: Velpula_Bharathi@comcast.com