RDKB-64347: Fixing coverity issues#1095
RDKB-64347: Fixing coverity issues#1095bharathivelp wants to merge 1 commit intordkcentral:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses several Coverity findings (primarily around safer file handling and safer string copying) in the WiFi sample app and TR-181 / DB utilities.
Changes:
- Harden CSI file rotation by switching from
fopen/stat/unlinkpatterns toopen/fstat/fdopenand adding read/rename error handling. - Make security-mode string formatting bounded by adding a buffer-size parameter and using
snprintf. - Make last-reboot-reason retrieval bounded and validate inputs by adding a length parameter and using
snprintf.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| source/sampleapps/wifievents_consumer_sample.c | Updates CSI file rotation to use fd-based APIs and adds error handling around read/rename. |
| source/dml/tr_181/sbapi/cosa_wifi_apis.c | Changes security string conversion to use bounded writes (snprintf) with an explicit buffer size. |
| source/dml/tr_181/ml/cosa_wifi_dml.c | Updates call site to pass buffer size to getSecurityStringFromInt. |
| source/db/wifi_db_apis.c | Adds length-aware API for last reboot reason and replaces strcpy with snprintf plus argument validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
source/sampleapps/wifievents_consumer_sample.c:595
- The return values from fwrite() are ignored when copying and appending CSI records. If the write fails (e.g., ENOSPC or I/O error), the function will still proceed to rename() the temp file, potentially replacing a good CSI.bin with a truncated/corrupted one. Please check fwrite() results and abort/cleanup (and avoid renaming) on write errors.
fwrite(&tmp_mac, sizeof(mac_address_t), 1, csifptr_tmp);
fwrite(&tmp_frame_info, sizeof(wifi_frame_info_t), 1, csifptr_tmp);
fwrite(&tmp_csi_matrix, sizeof(wifi_csi_matrix_t), 1, csifptr_tmp);
}
}
if (csifptr_tmp != NULL) {
fwrite(sta_mac, sizeof(mac_address_t), 1, csifptr_tmp);
fwrite(&(csi->frame_info), sizeof(wifi_frame_info_t), 1, csifptr_tmp);
fwrite(&(csi->csi_matrix), sizeof(wifi_csi_matrix_t), 1, csifptr_tmp);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
source/sampleapps/wifievents_consumer_sample.c:608
- The
fwrite()results are not checked. If the temp file fills up or an I/O error occurs, this will silently produce a truncated/corrupted CSI file and still proceed torename(). Please checkfwrite()return values (and/orferror(csifptr_tmp)) and on failure log + abort rotation (close, unlink temp, and skiprename()).
fwrite(&tmp_mac, sizeof(mac_address_t), 1, csifptr_tmp);
fwrite(&tmp_frame_info, sizeof(wifi_frame_info_t), 1, csifptr_tmp);
fwrite(&tmp_csi_matrix, sizeof(wifi_csi_matrix_t), 1, csifptr_tmp);
}
}
if (csifptr_tmp != NULL) {
fwrite(sta_mac, sizeof(mac_address_t), 1, csifptr_tmp);
fwrite(&(csi->frame_info), sizeof(wifi_frame_info_t), 1, csifptr_tmp);
fwrite(&(csi->csi_matrix), sizeof(wifi_csi_matrix_t), 1, csifptr_tmp);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f2bc22b to
1a65604
Compare
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.
Reason for change: Fixing Low 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
Reason for change: Fixing Low 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