RDKB-64347: Fixing coverity issues#1074
RDKB-64347: Fixing coverity issues#1074bharathivelp wants to merge 1 commit 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
Fixes several Coverity findings across passpoint, webconfig analytics logging, and blaster measurement code paths.
Changes:
- Replaced unsafe
strcpy()with boundedsnprintf()for HESSID copies in passpoint interworking config. - Refactored interworking element handling under
ENABLE_FEATURE_MESHWIFI(now heap-allocateselem). - Removed an unused return-status variable and simplified analytics status formatting; removed redundant
buff = NULLafterfree().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
source/core/wifi_passpoint.c |
Hardens HESSID copies and refactors interworking element initialization logic for mesh builds. |
source/core/wifi_ctrl_webconfig.c |
Removes unused variable and simplifies analytics “success” formatting on successful apply. |
source/apps/blaster/wifi_single_client_msmt.c |
Removes redundant nulling of a freed buffer pointer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elem = (wifi_interworking_t *)malloc(sizeof(wifi_interworking_t)); | ||
| if (elem == NULL) { | ||
| wifi_util_dbg_print(WIFI_PASSPOINT, "Failed to allocate memory for elem\n"); | ||
| return RETURN_ERR; | ||
| } |
There was a problem hiding this comment.
WiFi_InitInterworkingElement() now heap-allocates elem and returns RETURN_ERR on allocation failure. Previously this path couldn’t fail, and the function’s contract/comment suggests it should fall back to defaults when it can’t initialize from DB. Consider either keeping elem on the stack (no allocation needed) or, if allocation is required, falling back to WiFi_DefaultInterworkingConfig(vapIndex) when malloc fails instead of returning RETURN_ERR.
| memset(elem, 0, sizeof(wifi_interworking_t)); | ||
| //Update OVS DB | ||
| if(-1 == get_wifidb_obj()->desc.update_wifi_interworking_cfg_fn(getVAPName(vapIndex - 1), &elem)) { | ||
| if(-1 == get_wifidb_obj()->desc.update_wifi_interworking_cfg_fn(getVAPName(vapIndex - 1), elem)) { | ||
| wifi_util_dbg_print(WIFI_PASSPOINT,"Failed to Initialize Interwokring Configuration from DB for AP: %d. Setting Default\n",InstanceNumber); | ||
| free(elem); | ||
| return WiFi_DefaultInterworkingConfig(vapIndex);//ONE_WIFI |
There was a problem hiding this comment.
In this ENABLE_FEATURE_MESHWIFI branch, elem is memset to 0 and then passed to update_wifi_interworking_cfg_fn(...), but that function pointer is wired to update_wifi_interworking_config() (source/db/wifi_db_apis.c), which writes config->interworking to OVSDB and does not populate the passed-in struct. As a result, this call will upsert zeros into the DB and pCfg will be populated from zeros rather than DB state. If the intent is to read existing config, use a DB "get" API (e.g., wifidb_get_interworking_config / get_wifi_interworking_config) and only upsert defaults when missing.
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