-
Notifications
You must be signed in to change notification settings - Fork 5
RDKB-60656 : Available memory check for firmware downloads #23
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
Changes from 5 commits
7e5c843
a86f968
f6505dd
abca33e
bb2a7d4
3a15aaf
7c4e0b9
85113dc
d8b4d95
3d92313
df7dba2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||
| /* | ||||||||||
| * If not stated otherwise in this file or this component's LICENSE file the | ||||||||||
| * following copyright and licenses apply: | ||||||||||
|
Check failure on line 3 in source/TR-181/integration_src.shared/cosa_device_info_apis.c
|
||||||||||
| * | ||||||||||
| * Copyright 2015 RDK Management | ||||||||||
| * | ||||||||||
|
|
@@ -77,6 +77,7 @@ | |||||||||
| #include "safec_lib_common.h" | ||||||||||
| #include <syscfg/syscfg.h> | ||||||||||
| #include "secure_wrapper.h" | ||||||||||
| #include "fw_download_check.h" | ||||||||||
|
||||||||||
|
|
||||||||||
| #define CM_HTTPURL_LEN 512 | ||||||||||
| #define VALID_fW_LEN 128 | ||||||||||
|
|
@@ -379,7 +380,6 @@ | |||||||||
| } | ||||||||||
| #endif | ||||||||||
|
|
||||||||||
|
|
||||||||||
| ANSC_STATUS CosaDmlDIDownloadNow(ANSC_HANDLE hContext) | ||||||||||
| { | ||||||||||
| PCOSA_DATAMODEL_DEVICEINFO pMyObject = (PCOSA_DATAMODEL_DEVICEINFO)hContext; | ||||||||||
|
|
@@ -491,6 +491,12 @@ | |||||||||
| } | ||||||||||
|
|
||||||||||
| } */ | ||||||||||
|
|
||||||||||
| if(can_proceed_fw_download() == 0){ | ||||||||||
|
||||||||||
| if(can_proceed_fw_download() == 0){ | |
| /* can_proceed_fw_download() returns non-zero when it is safe to proceed, | |
| * and 0 on failure (e.g. not enough memory). */ | |
| if (!can_proceed_fw_download()){ |
Outdated
Copilot
AI
Jan 21, 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 memory check is performed after several other validation steps and resource allocations. If the intent is to prevent firmware download when memory is insufficient, this check should be moved earlier in the function (before cm_hal_Set_HTTP_Download_Url and cm_hal_Set_HTTP_Download_Interface calls) to avoid unnecessary operations when the download will fail anyway.
Copilot
AI
Jan 21, 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 lacks specific information about how much memory is available versus how much is required. Consider enhancing the error message to include actual memory values, which would be more helpful for debugging and troubleshooting in production environments.
Outdated
Copilot
AI
Jan 21, 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.
Tests exist for CosaDmlDIDownloadNow() but there is no test coverage for the new memory check failure path. Add tests to verify the behavior when can_proceed_fw_download() returns 0, ensuring the function properly returns ANSC_STATUS_FAILURE and logs the appropriate error message.
Outdated
Copilot
AI
Jan 27, 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 new memory check for firmware downloads lacks test coverage. The existing test cases for CosaDmlDIDownloadNow (lines 785 and 852 in CcspCMAgentDeviceInfoDmlTest.cpp) do not mock or test the can_proceed_fw_download() function. Add test cases to verify that the function correctly fails when can_proceed_fw_download() returns 0, and succeeds when it returns non-zero.
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 library "-lfw_download_chk" is added as a dependency but there's no verification that this library exists or is available in the build environment. Consider adding appropriate configuration checks (e.g., PKG_CHECK_MODULES or AC_CHECK_LIB in configure.ac) to ensure the library is available before attempting to link against it.