RDKB-60656 : Available memory check for firmware downloads by bunnam988 · Pull Request #37 · rdkcentral/miscellaneous-broadband

bunnam988 · 2026-01-21T11:55:28Z

RDKB-60656 : Available memory check for firmware downloads

Reason for change: Before firmware download, we need to check if the device have enough memory
Test Procedure: 1. while firmware download, available memory check logs should be seen.
2. If available memory < required memory, firmware download should not start.
Risks: medium
Priority: P1

Gerrit meta-layer changes:
https://gerrit.teamccp.com/#/q/topic:RDKB-60656+(status:open+OR+status:merged)
List all dependent GitHub PRs:
rdkcentral/xconf-client#13
rdkcentral/provisioning-and-management#178
rdkcentral/utopia#182
rdkcentral/cable-modem-agent#23
https://github.com/rdk-gdcs/apparmor-profiles/pull/49

Copilot

Pull request overview

This PR adds a memory availability check mechanism before firmware downloads to prevent download attempts when insufficient memory is available. The implementation introduces a new library module FwDownloadChk that validates available system memory against firmware size and configurable thresholds.

Changes:

Added new FwDownloadChk library module with memory checking functionality
Integrated the new module into the build system via Makefile.am and configure.ac
Implemented can_proceed_fw_download() function that fetches firmware size via curl, reads system memory info, and compares against configurable thresholds

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
source/Makefile.am	Added FwDownloadChk to the list of subdirectories to build
source/FwDownloadChk/fw_download_check.h	Header file declaring the public API for memory check function
source/FwDownloadChk/fw_download_check.c	Implementation of memory check logic including syscfg queries, curl-based size fetching, and memory calculations
source/FwDownloadChk/Makefile.am	Build configuration for the new library with dependencies on syscfg and ccsp_common
configure.ac	Added FwDownloadChk Makefile to autoconf configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/FwDownloadChk/fw_download_check.c

configure.ac

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 18 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/FwDownloadChk/fw_download_check.c

source/FwDownloadChk/Makefile.am

source/FwDownloadChk/fw_download_check.c

source/FwDownloadChk/Makefile.am

configure.ac

source/FwDownloadChk/fw_download_check.c

changing return value for failure cases

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/FwDownloadChk/fw_download_check.c

source/bridge_utils/bridge_utils_bin/Makefile.am

source/FwDownloadChk/fw_download_check.c

source/FwDownloadChk/fw_download_check.h

source/FwDownloadChk/fw_download_check.c

added telemetry marker

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/FwDownloadChk/fw_download_check.c

updated liscence

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T06:07:51Z

source/FwDownloadChk/fw_download_check.c

+    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
+        return -1;
+    }
+    XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);
+
+    if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){


Inconsistent spacing in syscfg_get calls. Line 60 has no space after the comma in syscfg_get(NULL,"xconf_url",url, sizeof(url)), while line 128 has a space: syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)). For consistency with the rest of the function, add a space after NULL on lines 60 and 66.

Suggested change

if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){

XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");

return -1;

}

XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);

if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){

if(syscfg_get(NULL, "xconf_url",url, sizeof(url)) != 0){

XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");

return -1;

}

XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);

if(syscfg_get(NULL, "fw_to_upgrade",fname,sizeof(fname)) != 0){

Copilot · 2026-02-02T06:07:52Z

source/FwDownloadChk/fw_download_check.c

+#define _GNU_SOURCE 1
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <ctype.h>
+#include <time.h>
+#include <stdarg.h>
+
+#include <syscfg/syscfg.h>
+#include <telemetry_busmessage_sender.h>
+#include "fw_download_check.h"
+
+#define XCONF_LOG_PATH "/rdklogs/logs/xconf.txt.0"
+#define XCONF_LOG_INFO(...)  xconf_log_write("INFO", __VA_ARGS__)
+#define XCONF_LOG_ERROR(...) xconf_log_write("ERROR", __VA_ARGS__)
+
+static void xconf_log_write(const char *level, const char *fmt, ...)
+{
+    FILE *fp = fopen(XCONF_LOG_PATH, "a");
+    if (!fp) {
+        fp = stderr;
+    }
+
+    time_t now = time(NULL);
+    struct tm tm_info;
+    localtime_r(&now, &tm_info);
+    char ts[32];
+    strftime(ts, sizeof(ts), "%Y-%m-%d %H:%M:%S", &tm_info);
+
+    fprintf(fp, "%s [%s] ", ts, level);
+
+    va_list ap;
+    va_start(ap, fmt);
+    vfprintf(fp, fmt, ap);
+    va_end(ap);
+
+    if (fp != stderr) {
+        fflush(fp);
+        fclose(fp);
+    } else {
+        fflush(stderr);
+    }
+}
+
+int can_proceed_fw_download(void)
+{
+    char url[1024] = {0};
+    char fname[256] = {0};
+    char buf[64] = {0};
+    char line[256] = {0};
+
+    uint64_t fw_kb = 0;
+    uint64_t avail_kb = 0;
+    uint64_t rsrv_mb = 0, rsrv_kb = 0;
+    int imgp_pct = 0;
+
+    /* 1) Fetch URL and filename */
+    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
+        return -1;
+    }
+    XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);
+
+    if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get fw_to_upgrade\n");
+        return -1;
+    }
+    XCONF_LOG_INFO("[FWCHK] fw_to_upgrade: %s\n", fname);
+
+    /* 2) Fetch Content-Length using curl CLI via popen (NO libcurl needed) */
+    {
+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);
+
+        FILE *fp = popen(cmd, "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");
+            return -1;
+        }
+
+        if (fgets(line, sizeof(line), fp) == NULL) {
+            XCONF_LOG_ERROR("[FWCHK] Content-Length not found\n");
+            pclose(fp);
+            return -1;
+        }
+        pclose(fp);
+
+        uint64_t bytes = strtoull(line, NULL, 10);
+        if (bytes == 0) {
+            XCONF_LOG_ERROR("[FWCHK] Invalid Content-Length\n");
+            return -1;
+        }
+
+        fw_kb = (bytes + 1023ULL) / 1024ULL;
+        XCONF_LOG_INFO("[FWCHK] Firmware size: %" PRIu64 " kB\n", fw_kb);
+    }
+
+    /* 3) Read MemAvailable (kB) */
+    {
+        FILE *fp = fopen("/proc/meminfo", "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] Cannot read /proc/meminfo\n");
+            return -1;
+        }
+
+        while (fgets(line, sizeof(line), fp)) {
+            if (strncmp(line, "MemAvailable:", 13) == 0) {
+                char *p = line + 13;
+                while (*p && !isdigit((unsigned char)*p)) ++p;
+                avail_kb = strtoull(p, NULL, 10);
+                break;
+            }
+        }
+        fclose(fp);
+
+        if (avail_kb == 0) {
+            XCONF_LOG_ERROR("[FWCHK] MemAvailable not found\n");
+            return -1;
+        }
+        XCONF_LOG_INFO("[FWCHK] MemAvailable: %" PRIu64 " kB\n", avail_kb);
+    }
+
+    /* 4) syscfg variables EXACTLY as you wanted */
+    if(syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
+        return -1;
+    }
+    rsrv_mb = (uint64_t)atoi(buf);
+    rsrv_kb = rsrv_mb * 1024ULL;
+    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);
+
+    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
+        return -1;
+    }
+    imgp_pct = atoi(buf);
+    if (imgp_pct < 0) imgp_pct = 0;
+    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);
+
+    /* 5) Required Memory calculation */
+    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
+    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;
+
+    XCONF_LOG_INFO("[FWCHK] Required Memory: %" PRIu64 " kB\n", required_kb);
+
+    /* 6) Verdict */
+    if (avail_kb >= required_kb) {
+        XCONF_LOG_INFO("[FWCHK] Verdict: PROCEED\n");
+        return 1;
+    }
+
+    XCONF_LOG_INFO("[FWCHK] Verdict: BLOCK\n");
+    t2_event_d("XCONF_Dwld_Ignored_Not_EnoughMem", 1);
+    return 0;
+}


Missing copyright and license header. All other source files in this codebase include a standard Apache 2.0 license header at the beginning. This file should include the same header for legal compliance and consistency.

Copilot · 2026-02-02T06:07:52Z

source/FwDownloadChk/fw_download_check.h

+#ifndef FW_DOWNLOAD_CHECK_H
+#define FW_DOWNLOAD_CHECK_H
+
+int can_proceed_fw_download(void);
+
+#endif /* FW_DOWNLOAD_CHECK_H */


Missing copyright and license header. All other header files in this codebase include a standard Apache 2.0 license header. This file should include the same header for legal compliance and consistency.

Copilot · 2026-02-02T06:07:52Z

source/FwDownloadChk/fw_download_check.c

+    if(syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
+        return -1;
+    }
+    rsrv_mb = (uint64_t)atoi(buf);
+    rsrv_kb = rsrv_mb * 1024ULL;
+    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);
+
+    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
+        return -1;
+    }
+    imgp_pct = atoi(buf);
+    if (imgp_pct < 0) imgp_pct = 0;
+    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);


Missing validation of syscfg values. The FwDwld_AvlMem_RsrvThreshold and FwDwld_ImageProcMemPercent values from syscfg are converted using atoi without validation. If these syscfg values are missing, empty, or contain non-numeric data, atoi will return 0, which could lead to incorrect memory calculations. Consider validating that the buffers are not empty and contain valid numeric values before conversion.

Copilot · 2026-02-02T06:07:53Z

source/FwDownloadChk/Makefile.am

+# SPDX-License-Identifier: Apache-2.0
+
+AM_CFLAGS =
+
+lib_LTLIBRARIES = libfw_download_chk.la
+
+libfw_download_chk_la_SOURCES = fw_download_check.c
+
+# syscfg for syscfg_get, ccsp_common for logging headers
+libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common


Missing copyright and license header. All other Makefile.am files in this codebase include a standard Apache 2.0 license header. This file should include the same header for legal compliance and consistency.

Copilot · 2026-02-02T06:07:53Z

source/FwDownloadChk/Makefile.am

+libfw_download_chk_la_SOURCES = fw_download_check.c
+
+# syscfg for syscfg_get, ccsp_common for logging headers
+libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common


Missing library dependency. The code uses t2_event_d from telemetry_busmessage_sender.h (line 157 in fw_download_check.c), but the Makefile does not link against the telemetry library. Looking at source/FwBankInfo/Makefile.am:24, the telemetry library is linked as -ltelemetry_msgsender. This library should be added to libfw_download_chk_la_LDFLAGS to avoid linker errors.

Suggested change

libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common

libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common -ltelemetry_msgsender

Copilot · 2026-02-02T06:07:53Z

source/FwDownloadChk/fw_download_check.c

+    }
+    XCONF_LOG_INFO("[FWCHK] fw_to_upgrade: %s\n", fname);
+
+    /* 2) Fetch Content-Length using curl CLI via popen (NO libcurl needed) */


Misleading comment text. The comment says "NO libcurl needed" but this is not entirely accurate - the code still relies on curl being installed and available on the system, just using the CLI tool instead of linking against libcurl. Consider revising to clarify that this uses the curl CLI tool to avoid linking dependencies, rather than implying curl is not needed at all.

Suggested change

/* 2) Fetch Content-Length using curl CLI via popen (NO libcurl needed) */

/* 2) Fetch Content-Length using curl CLI via popen (avoids linking against libcurl; requires curl CLI installed) */

Copilot · 2026-02-02T06:07:53Z

source/FwDownloadChk/fw_download_check.h

@@ -0,0 +1,6 @@
+#ifndef FW_DOWNLOAD_CHECK_H
+#define FW_DOWNLOAD_CHECK_H
+


Missing function documentation. The function declaration lacks documentation describing its purpose, parameters, and return values. Consider adding a comment block that explains what the function does, what the return values mean (appears to be: 1 = proceed with download, 0 = block due to insufficient memory, -1 = error), and any preconditions or side effects.

Suggested change

/**

* Checks whether a firmware download can safely proceed.

*

* This function evaluates whether the system meets the necessary

* conditions to start a firmware download (for example, sufficient

* available memory or other resource constraints).

*

* Return value:

* 1 - The firmware download may proceed.

* 0 - The firmware download must be blocked (e.g. insufficient memory).

* -1 - An error occurred while performing the check.

*/

apattu200 · 2026-02-02T17:12:23Z

source/FwDownloadChk/Makefile.am

+libfw_download_chk_la_SOURCES = fw_download_check.c
+
+# syscfg for syscfg_get, ccsp_common for logging headers
+libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common


do we need -ltelemetry_msgsender ?

Copilot · 2026-02-02T17:13:20Z

@apattu200 I've opened a new pull request, #39, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.

Comments suppressed due to low confidence (2)

source/FwDownloadChk/fw_download_check.c:177

The new FwDownloadChk module lacks unit test coverage. The repository has established test infrastructure using gtest (see source/test/), and other modules like bridge_utils have comprehensive test coverage (source/test/bridge_utils/). Given the critical nature of this functionality (determining whether firmware downloads should proceed), unit tests should be added to cover:

Valid memory check scenarios (sufficient and insufficient memory)
Error cases (syscfg failures, curl failures, invalid inputs)
Edge cases (zero values, very large values, overflow conditions)
Proper handling of malformed syscfg data

int can_proceed_fw_download(void)
{
    char url[1024] = {0};
    char fname[256] = {0};
    char buf[64] = {0};
    char line[256] = {0};

    uint64_t fw_kb = 0;
    uint64_t avail_kb = 0;
    uint64_t rsrv_mb = 0, rsrv_kb = 0;
    int imgp_pct = 0;

    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
        return -1;
    }
    XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);

    if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get fw_to_upgrade\n");
        return -1;
    }
    XCONF_LOG_INFO("[FWCHK] fw_to_upgrade: %s\n", fname);

    {
        char cmd[1500];
        snprintf(cmd, sizeof(cmd),
                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);

        FILE *fp = popen(cmd, "r");
        if (!fp) {
            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");
            return -1;
        }

        if (fgets(line, sizeof(line), fp) == NULL) {
            XCONF_LOG_ERROR("[FWCHK] Content-Length not found\n");
            pclose(fp);
            return -1;
        }
        pclose(fp);

        uint64_t bytes = strtoull(line, NULL, 10);
        if (bytes == 0) {
            XCONF_LOG_ERROR("[FWCHK] Invalid Content-Length\n");
            return -1;
        }

        fw_kb = (bytes + 1023ULL) / 1024ULL;
        XCONF_LOG_INFO("[FWCHK] Firmware size: %" PRIu64 " kB\n", fw_kb);
    }

    {
        FILE *fp = fopen("/proc/meminfo", "r");
        if (!fp) {
            XCONF_LOG_ERROR("[FWCHK] Cannot read /proc/meminfo\n");
            return -1;
        }

        while (fgets(line, sizeof(line), fp)) {
            if (strncmp(line, "MemAvailable:", 13) == 0) {
                char *p = line + 13;
                while (*p && !isdigit((unsigned char)*p)) ++p;
                avail_kb = strtoull(p, NULL, 10);
                break;
            }
        }
        fclose(fp);

        if (avail_kb == 0) {
            XCONF_LOG_ERROR("[FWCHK] MemAvailable not found\n");
            return -1;
        }
        XCONF_LOG_INFO("[FWCHK] MemAvailable: %" PRIu64 " kB\n", avail_kb);
    }

    if(syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
        return -1;
    }
    rsrv_mb = (uint64_t)atoi(buf);
    rsrv_kb = rsrv_mb * 1024ULL;
    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);

    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
        return -1;
    }
    imgp_pct = atoi(buf);
    if (imgp_pct < 0) imgp_pct = 0;
    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);

    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;

    XCONF_LOG_INFO("[FWCHK] Required Memory: %" PRIu64 " kB\n", required_kb);

    if (avail_kb >= required_kb) {
        XCONF_LOG_INFO("[FWCHK] Verdict: PROCEED\n");
        return 1;
    }

    XCONF_LOG_INFO("[FWCHK] Verdict: BLOCK\n");
    t2_event_d("XCONF_Dwld_Ignored_Not_EnoughMem", 1);
    return 0;
}

source/FwDownloadChk/fw_download_check.c:177

The function has multiple failure modes that all return -1, but they represent very different scenarios:

Configuration errors (syscfg failures)
Network errors (curl failures)
System errors (/proc/meminfo unavailable)
Invalid data (parsing failures)

From an operational perspective, it would be valuable to distinguish between these failure types in telemetry or return codes so that operators can diagnose why firmware downloads are being blocked. For example, network failures might indicate a connectivity issue, while syscfg failures might indicate a configuration problem. Consider adding telemetry markers (t2_event_d calls) for different failure scenarios to aid in troubleshooting.

int can_proceed_fw_download(void)
{
    char url[1024] = {0};
    char fname[256] = {0};
    char buf[64] = {0};
    char line[256] = {0};

    uint64_t fw_kb = 0;
    uint64_t avail_kb = 0;
    uint64_t rsrv_mb = 0, rsrv_kb = 0;
    int imgp_pct = 0;

    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
        return -1;
    }
    XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);

    if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get fw_to_upgrade\n");
        return -1;
    }
    XCONF_LOG_INFO("[FWCHK] fw_to_upgrade: %s\n", fname);

    {
        char cmd[1500];
        snprintf(cmd, sizeof(cmd),
                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);

        FILE *fp = popen(cmd, "r");
        if (!fp) {
            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");
            return -1;
        }

        if (fgets(line, sizeof(line), fp) == NULL) {
            XCONF_LOG_ERROR("[FWCHK] Content-Length not found\n");
            pclose(fp);
            return -1;
        }
        pclose(fp);

        uint64_t bytes = strtoull(line, NULL, 10);
        if (bytes == 0) {
            XCONF_LOG_ERROR("[FWCHK] Invalid Content-Length\n");
            return -1;
        }

        fw_kb = (bytes + 1023ULL) / 1024ULL;
        XCONF_LOG_INFO("[FWCHK] Firmware size: %" PRIu64 " kB\n", fw_kb);
    }

    {
        FILE *fp = fopen("/proc/meminfo", "r");
        if (!fp) {
            XCONF_LOG_ERROR("[FWCHK] Cannot read /proc/meminfo\n");
            return -1;
        }

        while (fgets(line, sizeof(line), fp)) {
            if (strncmp(line, "MemAvailable:", 13) == 0) {
                char *p = line + 13;
                while (*p && !isdigit((unsigned char)*p)) ++p;
                avail_kb = strtoull(p, NULL, 10);
                break;
            }
        }
        fclose(fp);

        if (avail_kb == 0) {
            XCONF_LOG_ERROR("[FWCHK] MemAvailable not found\n");
            return -1;
        }
        XCONF_LOG_INFO("[FWCHK] MemAvailable: %" PRIu64 " kB\n", avail_kb);
    }

    if(syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
        return -1;
    }
    rsrv_mb = (uint64_t)atoi(buf);
    rsrv_kb = rsrv_mb * 1024ULL;
    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);

    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){
        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
        return -1;
    }
    imgp_pct = atoi(buf);
    if (imgp_pct < 0) imgp_pct = 0;
    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);

    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;

    XCONF_LOG_INFO("[FWCHK] Required Memory: %" PRIu64 " kB\n", required_kb);

    if (avail_kb >= required_kb) {
        XCONF_LOG_INFO("[FWCHK] Verdict: PROCEED\n");
        return 1;
    }

    XCONF_LOG_INFO("[FWCHK] Verdict: BLOCK\n");
    t2_event_d("XCONF_Dwld_Ignored_Not_EnoughMem", 1);
    return 0;
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/FwDownloadChk/fw_download_check.c

Copilot · 2026-02-02T17:39:15Z

source/FwDownloadChk/fw_download_check.c

+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);
+
+        FILE *fp = popen(cmd, "r");


Critical security vulnerability: Command injection risk. The 'url' and 'fname' variables from syscfg are directly interpolated into a shell command without any sanitization or validation. An attacker who can control these syscfg values could execute arbitrary commands. For example, if fname contains '; rm -rf /', this would be executed.

The code should either:

Use libcurl API instead of shell commands to make HTTP requests securely

If shell commands must be used, employ proper escaping/quoting and validate that the inputs contain only allowed characters

Use v_secure_popen from the secure_wrapper library (as seen in bridge_utils code at source/bridge_utils/bridge_utils_bin/bridge_util.c:433) which provides some command injection protection

Copilot · 2026-02-02T17:39:15Z

source/FwDownloadChk/fw_download_check.c

+    rsrv_kb = rsrv_mb * 1024ULL;
+    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);
+
+    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){


Using atoi for parsing configuration values is problematic because it returns 0 on error, which is indistinguishable from a valid value of "0". If the syscfg value is invalid or empty, atoi will silently return 0 without any error indication. Consider using strtol or strtoul with proper error checking (via endptr) to detect parsing failures, similar to how strtoull is used on line 110.

Copilot · 2026-02-02T17:39:15Z

source/FwDownloadChk/fw_download_check.c

+    XCONF_LOG_INFO("[FWCHK] Required Memory: %" PRIu64 " kB\n", required_kb);
+
+    if (avail_kb >= required_kb) {
+        XCONF_LOG_INFO("[FWCHK] Verdict: PROCEED\n");


Integer overflow risk in memory calculation. When calculating 'required_kb = fw_kb + rsrv_kb + img_proc_kb', if the sum exceeds UINT64_MAX, it will overflow. While unlikely with typical values, the code should validate that the addition doesn't overflow, especially since this calculation determines whether a firmware download proceeds.

Copilot · 2026-02-02T17:39:16Z

source/FwDownloadChk/fw_download_check.h

+int can_proceed_fw_download(void);
+


The function return values are inconsistent and potentially confusing. It returns -1 on error, 0 to block download, and 1 to proceed. This mixes error codes with boolean-like success/failure values in a non-standard way. A clearer API design would be to either:

Return 0 for success with an out parameter indicating whether to proceed, and -1 for errors

Return distinct positive values for different outcomes (e.g., 1 = proceed, 2 = block) and negative for errors

Use an enum for return values to make the meaning explicit

The current design makes it unclear whether callers should treat -1 the same as 0 (both prevent download) or differently.

Suggested change

int can_proceed_fw_download(void);

typedef enum {

FW_DOWNLOAD_CHECK_ERROR = -1,

FW_DOWNLOAD_CHECK_BLOCK = 0,

FW_DOWNLOAD_CHECK_PROCEED = 1

} fw_download_check_result_t;

fw_download_check_result_t can_proceed_fw_download(void);

Copilot · 2026-02-02T17:39:17Z

source/FwDownloadChk/fw_download_check.c

+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);
+
+        FILE *fp = popen(cmd, "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");
+            return -1;
+        }
+
+        if (fgets(line, sizeof(line), fp) == NULL) {
+            XCONF_LOG_ERROR("[FWCHK] Content-Length not found\n");
+            pclose(fp);
+            return -1;
+        }
+        pclose(fp);
+
+        uint64_t bytes = strtoull(line, NULL, 10);


Making an HTTP HEAD request via curl for every firmware download check could have performance and reliability implications. If the remote server is slow or unreachable, this will block the calling code. Consider:

Adding a timeout to the curl command (e.g., --max-time or --connect-timeout)

Documenting the expected latency of this operation

If this function is called frequently, consider caching the Content-Length value with a reasonable TTL

Additionally, relying on an external network call makes this function's behavior dependent on network conditions, which may not be desirable for a pre-download check.

Copilot · 2026-02-02T17:39:17Z

source/FwDownloadChk/fw_download_check.c

+
+        FILE *fp = popen(cmd, "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");


Using plain popen is a security risk. The codebase uses v_secure_popen from the secure_wrapper library in other places (e.g., source/LTime/LTime.c:72, source/bridge_utils/bridge_utils_bin/bridge_util.c:433) for safer command execution. This code should follow the same pattern and use v_secure_popen instead of popen.

Copilot · 2026-02-02T17:39:18Z

source/FwDownloadChk/fw_download_check.c

+            return -1;
+        }
+        pclose(fp);
+


The curl command output may contain a trailing newline or carriage return characters (e.g., "\r\n" from HTTP headers). The code passes the raw line to strtoull, which will stop at the first non-digit character. While this works, it would be more robust to explicitly strip whitespace from the line before parsing to avoid potential issues with different line endings or extra whitespace.

Suggested change

/* Strip trailing newline and carriage return characters before parsing */

line[strcspn(line, "\r\n")] = '\0';

Copilot · 2026-02-02T17:39:18Z

source/FwDownloadChk/fw_download_check.c

+    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
+    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;
+


Integer overflow risk in memory calculation. When calculating 'img_proc_kb = (fw_kb * imgp_pct + 99) / 100', if fw_kb is very large (e.g., > UINT64_MAX/100), the multiplication could overflow. While this is unlikely with realistic firmware sizes and percentages, the code should either validate the inputs are within reasonable bounds or use overflow-safe arithmetic.

Suggested change

uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;

uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;

uint64_t img_proc_kb = 0;

if (imgp_pct > 0) {

uint64_t imgp_pct_u = (uint64_t)imgp_pct;

/* Check for overflow in fw_kb * imgp_pct_u */

if (fw_kb > UINT64_MAX / imgp_pct_u) {

XCONF_LOG_ERROR("[FWCHK] Overflow in image processing memory multiplication\n");

return -1;

}

uint64_t prod = fw_kb * imgp_pct_u;

/* Check for overflow in prod + 99ULL */

if (prod > UINT64_MAX - 99ULL) {

XCONF_LOG_ERROR("[FWCHK] Overflow in image processing memory addition\n");

return -1;

}

img_proc_kb = (prod + 99ULL) / 100ULL;

}

/* Calculate required_kb = fw_kb + rsrv_kb + img_proc_kb with overflow checks */

uint64_t required_kb = 0;

if (fw_kb > UINT64_MAX - rsrv_kb) {

XCONF_LOG_ERROR("[FWCHK] Overflow in required memory calculation (fw_kb + rsrv_kb)\n");

return -1;

}

uint64_t tmp_required = fw_kb + rsrv_kb;

if (tmp_required > UINT64_MAX - img_proc_kb) {

XCONF_LOG_ERROR("[FWCHK] Overflow in required memory calculation (sum + img_proc_kb)\n");

return -1;

}

required_kb = tmp_required + img_proc_kb;

Copilot · 2026-02-02T17:39:18Z

source/FwDownloadChk/fw_download_check.c

+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);


Potential buffer overflow in snprintf. The 'url' can be up to 1024 bytes, 'fname' can be up to 256 bytes, plus the curl command template and potential '/' separator. The total could exceed the 1500-byte 'cmd' buffer. While snprintf will truncate, this would result in a malformed command. The code should either use a larger buffer, or validate that url and fname lengths won't cause truncation.

Suggested change

snprintf(cmd, sizeof(cmd),

"curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",

url, (url[strlen(url)-1] == '/') ? "" : "/", fname);

int n = snprintf(cmd, sizeof(cmd),

"curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",

url, (url[strlen(url)-1] == '/') ? "" : "/", fname);

if (n < 0 || (size_t)n >= sizeof(cmd)) {

XCONF_LOG_ERROR("[FWCHK] Command buffer too small for curl request\n");

return -1;

}

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T18:10:52Z

source/FwDownloadChk/Makefile.am

+libfw_download_chk_la_SOURCES = fw_download_check.c
+
+# syscfg for syscfg_get, ccsp_common for logging headers
+libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common -ltelemetry_msgsender


The Makefile is missing the telemetry library dependency. The code includes "telemetry_busmessage_sender.h" and calls t2_event_d() on line 214, but the LDFLAGS do not include the required telemetry library. Based on similar code in source/FwBankInfo/Makefile.am (line 24), this should include "-ltelemetry_msgsender". Without this, the library will fail to link.

Copilot · 2026-02-02T18:10:53Z

source/FwDownloadChk/fw_download_check.c

+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    imgp_pct = atoi(buf);


Using atoi() to parse numeric values from syscfg is problematic because atoi() cannot distinguish between a valid "0" value and a parsing error. If the syscfg value contains invalid data, atoi() will silently return 0, which may be a valid percentage value. Consider using strtol() with proper error checking (checking errno and ensuring the entire string was consumed) to detect parsing failures, similar to how Content-Length is parsed on line 153.

Copilot · 2026-02-02T18:10:53Z

source/FwDownloadChk/fw_download_check.c

+    if (imgp_pct < 0) imgp_pct = 0;
+    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);
+
+    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;


The calculation of img_proc_kb could theoretically overflow if fw_kb is very large (close to UINT64_MAX). While unlikely in practice for firmware sizes, adding the "+99ULL" before division could cause overflow. Consider checking fw_kb bounds or restructuring the calculation to avoid potential overflow, for example: "uint64_t img_proc_kb = (fw_kb / 100ULL) * (uint64_t)imgp_pct + ((fw_kb % 100ULL) * (uint64_t)imgp_pct + 99ULL) / 100ULL;".

Suggested change

uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;

uint64_t img_proc_kb = (fw_kb / 100ULL) * (uint64_t)imgp_pct

+ ((fw_kb % 100ULL) * (uint64_t)imgp_pct + 99ULL) / 100ULL;

Copilot · 2026-02-02T18:10:53Z

source/FwDownloadChk/fw_download_check.c

+    {
+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",


The awk command assumes the Content-Length header format is exactly "Content-Length: ", but HTTP headers can have variable whitespace. The current awk '{print $2}' assumes field 2 is the value, but if there are multiple spaces or tabs between the colon and value, this might fail. While grep -i handles case-insensitivity, consider using a more robust parsing approach or adding -F':' to awk and trimming whitespace explicitly.

Suggested change

"curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",

"curl -sI '%s%s%s' | awk -F':' 'BEGIN{IGNORECASE=1} "

"/^[[:space:]]*Content-Length[[:space:]]*:/ {"

"gsub(/^[ \\t]+/, \"\", $2); "

"gsub(/[ \\t\\r\\n]+$/, \"\", $2); "

"print $2; exit}'",

Copilot · 2026-02-02T18:10:54Z

source/FwDownloadChk/fw_download_check.c

+        }
+        pclose(fp);
+
+        uint64_t bytes = strtoull(line, NULL, 10);


The Content-Length value extracted from the HTTP response may contain a trailing carriage return (\r) or newline (\n) from the HTTP headers, which could affect the strtoull() parsing. While strtoull() typically stops at non-digit characters, explicitly stripping trailing whitespace/CR/LF before parsing would make the code more robust and ensure consistent behavior.

Copilot · 2026-02-02T18:10:54Z

source/FwDownloadChk/fw_download_check.c

+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    rsrv_mb = (uint64_t)atoi(buf);


Using atoi() to parse numeric values from syscfg is problematic because atoi() cannot distinguish between a valid "0" value and a parsing error. If the syscfg value contains invalid data, atoi() will silently return 0, which may be a valid threshold value. Consider using strtoul() or strtoull() with proper error checking (checking errno and ensuring the entire string was consumed) to detect parsing failures, similar to how Content-Length is parsed on line 153.

Copilot · 2026-02-02T18:10:54Z

source/FwDownloadChk/fw_download_check.c

+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);


The URL construction logic checks if url ends with '/' to avoid duplication, but this doesn't account for the case where fname starts with '/'. If both url ends with '/' and fname starts with '/', the ternary will add an empty string but fname still starts with '/', resulting in a double slash "//". While most HTTP servers handle this, it's cleaner to also check if fname[0] == '/' to ensure proper URL formation.

Suggested change

url, (url[strlen(url)-1] == '/') ? "" : "/", fname);

url,

((url[0] != '\0') && (url[strlen(url) - 1] == '/' || fname[0] == '/')) ? "" : "/",

(fname[0] == '/') ? (fname + 1) : fname);

Copilot · 2026-02-02T18:10:55Z

source/FwDownloadChk/fw_download_check.c

+        while (fgets(line, sizeof(line), fp)) {
+            if (strncmp(line, "MemAvailable:", 13) == 0) {
+                char *p = line + 13;
+                while (*p && !isdigit((unsigned char)*p)) ++p;
+                avail_kb = strtoull(p, NULL, 10);
+                break;
+            }
+        }
+        fclose(fp);
+
+        if (avail_kb == 0) {


The check for avail_kb == 0 is used to detect both parsing failure and the case where MemAvailable is not found in /proc/meminfo. However, this conflates two different error conditions. If the line "MemAvailable: 0 kB" exists (theoretically possible on a severely memory-constrained system), the code would incorrectly report it as "not found" rather than recognizing the actual zero value. Consider using a flag variable to track whether the MemAvailable line was found, separate from checking if the value is zero.

Suggested change

while (fgets(line, sizeof(line), fp)) {

if (strncmp(line, "MemAvailable:", 13) == 0) {

char *p = line + 13;

while (*p && !isdigit((unsigned char)*p)) ++p;

avail_kb = strtoull(p, NULL, 10);

break;

}

}

fclose(fp);

if (avail_kb == 0) {

int memavail_found = 0;

while (fgets(line, sizeof(line), fp)) {

if (strncmp(line, "MemAvailable:", 13) == 0) {

char *p = line + 13;

while (*p && !isdigit((unsigned char)*p)) ++p;

avail_kb = strtoull(p, NULL, 10);

memavail_found = 1;

break;

}

}

fclose(fp);

if (!memavail_found) {

source/FwDownloadChk/fw_download_check.h

Copilot · 2026-02-02T18:10:55Z

source/FwDownloadChk/fw_download_check.c

+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);


The curl command is constructed using unsanitized user input from syscfg variables "xconf_url" and "fw_to_upgrade". These values are directly interpolated into a shell command executed via popen(), which creates a command injection vulnerability. If these syscfg values contain shell metacharacters (such as ';', '|', '$', backticks, etc.), an attacker could execute arbitrary commands.

Consider using libcurl's API directly instead of shelling out to curl. This would eliminate the shell injection risk and provide better error handling. Alternatively, if you must use popen, implement strict input validation to ensure the URL and filename contain only expected characters.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T03:24:47Z

source/FwDownloadChk/fw_download_check.c

+int can_proceed_fw_download(void)
+{
+    char url[1024] = {0};
+    char fname[256] = {0};
+    char buf[64] = {0};
+    char line[256] = {0};
+
+    uint64_t fw_kb = 0;
+    uint64_t avail_kb = 0;
+    uint64_t rsrv_mb = 0, rsrv_kb = 0;
+    int imgp_pct = 0;
+
+    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    XCONF_LOG_INFO("[FWCHK] xconf_url: %s\n", url);
+
+    if(syscfg_get(NULL,"fw_to_upgrade",fname,sizeof(fname)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get fw_to_upgrade\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    XCONF_LOG_INFO("[FWCHK] fw_to_upgrade: %s\n", fname);
+
+    {
+        char cmd[1500];
+        snprintf(cmd, sizeof(cmd),
+                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);
+
+        FILE *fp = popen(cmd, "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] popen() failed for curl\n");
+            return FW_DWNLD_MEMCHK_FAILED;
+        }
+
+        if (fgets(line, sizeof(line), fp) == NULL) {
+            XCONF_LOG_ERROR("[FWCHK] Content-Length not found\n");
+            pclose(fp);
+            return FW_DWNLD_MEMCHK_FAILED;
+        }
+        pclose(fp);
+
+        uint64_t bytes = strtoull(line, NULL, 10);
+        if (bytes == 0) {
+            XCONF_LOG_ERROR("[FWCHK] Invalid Content-Length\n");
+            return FW_DWNLD_MEMCHK_FAILED;
+        }
+
+        fw_kb = (bytes + 1023ULL) / 1024ULL;
+        XCONF_LOG_INFO("[FWCHK] Firmware size: %" PRIu64 " kB\n", fw_kb);
+    }
+
+    {
+        FILE *fp = fopen("/proc/meminfo", "r");
+        if (!fp) {
+            XCONF_LOG_ERROR("[FWCHK] Cannot read /proc/meminfo\n");
+            return FW_DWNLD_MEMCHK_FAILED;
+        }
+
+        while (fgets(line, sizeof(line), fp)) {
+            if (strncmp(line, "MemAvailable:", 13) == 0) {
+                char *p = line + 13;
+                while (*p && !isdigit((unsigned char)*p)) ++p;
+                avail_kb = strtoull(p, NULL, 10);
+                break;
+            }
+        }
+        fclose(fp);
+
+        if (avail_kb == 0) {
+            XCONF_LOG_ERROR("[FWCHK] MemAvailable not found\n");
+            return FW_DWNLD_MEMCHK_FAILED;
+        }
+        XCONF_LOG_INFO("[FWCHK] MemAvailable: %" PRIu64 " kB\n", avail_kb);
+    }
+
+    if(syscfg_get(NULL, "FwDwld_AvlMem_RsrvThreshold", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_AvlMem_RsrvThreshold\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    rsrv_mb = (uint64_t)atoi(buf);
+    rsrv_kb = rsrv_mb * 1024ULL;
+    XCONF_LOG_INFO("[FWCHK] ReserveThreshold: %llu MB\n", (unsigned long long)rsrv_mb);
+
+    if(syscfg_get(NULL, "FwDwld_ImageProcMemPercent", buf, sizeof(buf)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get FwDwld_ImageProcMemPercent\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }
+    imgp_pct = atoi(buf);
+    if (imgp_pct < 0) imgp_pct = 0;
+    XCONF_LOG_INFO("[FWCHK] ImageProcPercent: %d %%\n", imgp_pct);
+
+    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
+    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;
+
+    XCONF_LOG_INFO("[FWCHK] Required Memory: %" PRIu64 " kB\n", required_kb);
+
+    if (avail_kb >= required_kb) {
+        XCONF_LOG_INFO("[FWCHK] Verdict: PROCEED\n");
+        return FW_DWNLD_MEMCHK_SUCCEED;
+    }
+
+    XCONF_LOG_INFO("[FWCHK] Verdict: BLOCK\n");
+    t2_event_d("XCONF_Dwld_Ignored_Not_EnoughMem", 1);
+    return FW_DWNLD_MEMCHK_NOT_ENOUGH_MEM;
+}


The new firmware download check functionality lacks test coverage. The repository has a comprehensive testing infrastructure (using gtest) with extensive tests for bridge_utils. The can_proceed_fw_download function has complex logic including syscfg interactions, shell command execution, file parsing, and memory calculations that should be tested. Consider adding unit tests for this module to cover success cases, error cases (missing syscfg values, curl failures, invalid Content-Length, missing MemAvailable), and edge cases (boundary conditions for memory calculations).

Copilot · 2026-02-03T03:24:47Z

source/FwDownloadChk/Makefile.am

+
+lib_LTLIBRARIES = libfw_download_chk.la
+
+libfw_download_chk_la_SOURCES = fw_download_check.c


The library header file (fw_download_check.h) is not installed or listed in any _HEADERS variable, which means it won't be available to other components that need to use this library. If this library is intended to be used by other components (which seems likely given it's built as a shared library), add a line like 'libfw_download_chk_la_HEADERS = fw_download_check.h' and specify the installation directory.

Suggested change

libfw_download_chk_la_SOURCES = fw_download_check.c

libfw_download_chk_la_SOURCES = fw_download_check.c

include_HEADERS = fw_download_check.h

Copilot · 2026-02-03T03:24:47Z

source/FwDownloadChk/fw_download_check.c

+    if (!fp) {
+        fp = stderr;
+    }
+
+    time_t now = time(NULL);
+    struct tm tm_info;
+    localtime_r(&now, &tm_info);
+    char ts[32];
+    strftime(ts, sizeof(ts), "%Y-%m-%d %H:%M:%S", &tm_info);
+
+    fprintf(fp, "%s [%s] ", ts, level);
+
+    va_list ap;
+    va_start(ap, fmt);
+    vfprintf(fp, fmt, ap);
+    va_end(ap);
+
+    if (fp != stderr) {
+        fflush(fp);
+        fclose(fp);
+    } else {
+        fflush(stderr);
+    }


When fopen fails and fp is set to stderr, the function never attempts to close stderr (correctly), but it also doesn't log or track the fact that logging to file failed. This could lead to log messages being lost if stderr is redirected or closed. Consider adding a one-time warning when the log file cannot be opened, or use a different fallback mechanism.

Copilot · 2026-02-03T03:24:47Z

source/FwDownloadChk/fw_download_check.c

+    if(syscfg_get(NULL,"xconf_url",url, sizeof(url)) != 0){
+        XCONF_LOG_ERROR("[FWCHK] Failed to get xconf_url\n");
+        return FW_DWNLD_MEMCHK_FAILED;
+    }


When syscfg_get returns a non-zero value, it doesn't necessarily mean the buffer is invalid. The code should explicitly clear or validate the buffer content to ensure it doesn't contain partial or stale data before returning. Consider initializing or checking the buffer after a failed syscfg_get call.

Copilot · 2026-02-03T03:24:48Z

source/FwDownloadChk/fw_download_check.h

+#define FW_DWNLD_MEMCHK_SUCCEED          1
+#define FW_DWNLD_MEMCHK_FAILED          -1
+#define FW_DWNLD_MEMCHK_NOT_ENOUGH_MEM   0
+


The header file lacks documentation for the function. The .c file has comprehensive documentation in the function definition, but public API headers should also document the function signature, parameters (even though there are none), return values, and side effects. This helps API consumers understand the contract without needing to read the implementation.

Suggested change

/**

* @brief Check whether a firmware download can proceed based on available memory.

*

* This function performs any required checks (such as validating available

* memory) to determine whether it is safe to start a firmware download.

*

* @return FW_DWNLD_MEMCHK_SUCCEED if the firmware download can proceed;

* FW_DWNLD_MEMCHK_NOT_ENOUGH_MEM if there is insufficient memory

* for the firmware download; or FW_DWNLD_MEMCHK_FAILED if the

* memory check itself failed or could not be completed.

*

* @note This function takes no parameters. Any side effects depend on the

* underlying implementation of the memory check (for example, system

* resource queries), but it is not expected to modify persistent state.

*/

apattu200

Looks ok for me

RDKB-60656 : Available memory check for firmware downloads Reason for change: Before firmware download, we need to check if the device have enough memory Test Procedure: 1. while firmware download, available memory check logs should be seen. 2. If available memory < required memory, firmware download should not start. Risks: medium Priority: P1 **Gerrit meta-layer changes:** https://gerrit.teamccp.com/#/q/topic:RDKB-60656+(status:open+OR+status:merged) **List all dependent GitHub PRs:** rdkcentral/provisioning-and-management#178 rdkcentral/utopia#182 rdkcentral/cable-modem-agent#23 rdkcentral/miscellaneous-broadband#37 https://github.com/rdk-gdcs/apparmor-profiles/pull/49

RDKB-60656 : Available memory check for firmware downloads Reason for change: Before firmware download, we need to check if the device have enough memory Test Procedure: 1. while firmware download, available memory check logs should be seen. 2. If available memory < required memory, firmware download should not start. Risks: medium Priority: P1 **Gerrit meta-layer changes:** https://gerrit.teamccp.com/#/q/topic:RDKB-60656+(status:open+OR+status:merged) **List all dependent GitHub PRs:** rdkcentral/xconf-client#13 rdkcentral/utopia#182 rdkcentral/cable-modem-agent#23 rdkcentral/miscellaneous-broadband#37 https://github.com/rdk-gdcs/apparmor-profiles/pull/49

RDKB-60656 : Available memory check for firmware downloads Reason for change: Before firmware download, we need to check if the device have enough memory Test Procedure: 1. while firmware download, available memory check logs should be seen. 2. If available memory < required memory, firmware download should not start. Risks: medium Priority: P1 **Gerrit meta-layer changes:** https://gerrit.teamccp.com/#/q/topic:RDKB-60656+(status:open+OR+status:merged) **List all dependent GitHub PRs:** rdkcentral/xconf-client#13 rdkcentral/provisioning-and-management#178 rdkcentral/cable-modem-agent#23 rdkcentral/miscellaneous-broadband#37 https://github.com/rdk-gdcs/apparmor-profiles/pull/49

RDKB-60656 : Available memory check for firmware downloads Reason for change: Before firmware download, we need to check if the device have enough memory Test Procedure: 1. while firmware download, available memory check logs should be seen. 2. If available memory < required memory, firmware download should not start. Risks: medium Priority: P1 **Gerrit meta-layer changes:** https://gerrit.teamccp.com/#/q/topic:RDKB-60656+(status:open+OR+status:merged) **List all dependent GitHub PRs:** rdkcentral/xconf-client#13 rdkcentral/provisioning-and-management#178 rdkcentral/utopia#182 rdkcentral/miscellaneous-broadband#37 https://github.com/rdk-gdcs/apparmor-profiles/pull/49

fw_download_check library

c4ed9e6

bunnam988 requested review from a team as code owners January 21, 2026 11:55

Copilot AI review requested due to automatic review settings January 21, 2026 11:55

Copilot started reviewing on behalf of bunnam988 January 21, 2026 11:55 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

bunnam988 and others added 2 commits January 23, 2026 07:18

added logging to xconf

184f727

Update fw_download_check.c

36a7863

Copilot AI review requested due to automatic review settings January 23, 2026 12:54

Copilot started reviewing on behalf of bunnam988 January 23, 2026 12:54 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

bunnam988 added 2 commits January 27, 2026 12:56

moving all the bridgeutil dependent libs to makefile

15e3a5f

Update fw_download_check.c

0f4afc2

changing return value for failure cases

Copilot AI review requested due to automatic review settings January 27, 2026 10:51

Copilot started reviewing on behalf of bunnam988 January 27, 2026 10:52 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

bunnam988 added 2 commits January 27, 2026 17:11

Update fw_download_check.c

07b547e

added telemetry marker

Update fw_download_check.h

95221a6

Copilot AI review requested due to automatic review settings January 28, 2026 10:54

Copilot started reviewing on behalf of bunnam988 January 28, 2026 10:54 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

source/FwDownloadChk/fw_download_check.c Show resolved Hide resolved

source/FwDownloadChk/fw_download_check.c Show resolved Hide resolved

bunnam988 added 2 commits February 2, 2026 11:21

Update fw_download_check.c

e06da95

Update Makefile.am

822d23b

updated liscence

Copilot AI review requested due to automatic review settings February 2, 2026 06:00

Copilot started reviewing on behalf of bunnam988 February 2, 2026 06:01 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

apattu200 reviewed Feb 2, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 2, 2026

Add copyright header to fw_download_check.c #39

Draft

bunnam988 added 2 commits February 2, 2026 22:58

Update fw_download_check.c

053329a

Update fw_download_check.h

c5bacd0

Copilot AI review requested due to automatic review settings February 2, 2026 17:29

Copilot started reviewing on behalf of bunnam988 February 2, 2026 17:30 View session

Update fw_download_check.c

0a42e9f

Copilot AI reviewed Feb 2, 2026

View reviewed changes

bunnam988 added 2 commits February 2, 2026 23:20

Update fw_download_check.h

d3ac215

Update fw_download_check.c

ae2c53f

Copilot AI review requested due to automatic review settings February 2, 2026 18:01

Copilot started reviewing on behalf of bunnam988 February 2, 2026 18:01 View session

Update Makefile.am

e0e54e5

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Update source/FwDownloadChk/fw_download_check.h

4b8e767

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 3, 2026 03:18

Copilot started reviewing on behalf of bunnam988 February 3, 2026 03:18 View session

Update source/FwDownloadChk/fw_download_check.c

887b8bc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Feb 3, 2026

View reviewed changes

apattu200 reviewed Feb 9, 2026

View reviewed changes

apattu200 approved these changes Feb 11, 2026

View reviewed changes

bunnam988 merged commit b5e0410 into develop Feb 11, 2026
6 of 8 checks passed

github-actions bot locked and limited conversation to collaborators Feb 11, 2026

	libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common
	libfw_download_chk_la_LDFLAGS = -lsyscfg -lccsp_common -ltelemetry_msgsender

	/* 2) Fetch Content-Length using curl CLI via popen (NO libcurl needed) */
	/* 2) Fetch Content-Length using curl CLI via popen (avoids linking against libcurl; requires curl CLI installed) */

		@@ -0,0 +1,6 @@
		#ifndef FW_DOWNLOAD_CHECK_H
		#define FW_DOWNLOAD_CHECK_H

+/**
+ * Checks whether a firmware download can safely proceed.
+ *
+ * This function evaluates whether the system meets the necessary
+ * conditions to start a firmware download (for example, sufficient
+ * available memory or other resource constraints).
+ *
+ * Return value:
+ *   1  - The firmware download may proceed.
+ *   0  - The firmware download must be blocked (e.g. insufficient memory).
+ *  -1  - An error occurred while performing the check.
+ */

-int can_proceed_fw_download(void);
+typedef enum {
+    FW_DOWNLOAD_CHECK_ERROR   = -1,
+    FW_DOWNLOAD_CHECK_BLOCK   = 0,
+    FW_DOWNLOAD_CHECK_PROCEED = 1
+} fw_download_check_result_t;
+fw_download_check_result_t can_proceed_fw_download(void);



	/* Strip trailing newline and carriage return characters before parsing */
	line[strcspn(line, "\r\n")] = '\0';

		uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
		uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;

-    uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
-    uint64_t required_kb = fw_kb + rsrv_kb + img_proc_kb;
+    uint64_t img_proc_kb = 0;
+    if (imgp_pct > 0) {
+        uint64_t imgp_pct_u = (uint64_t)imgp_pct;
+        /* Check for overflow in fw_kb * imgp_pct_u */
+        if (fw_kb > UINT64_MAX / imgp_pct_u) {
+            XCONF_LOG_ERROR("[FWCHK] Overflow in image processing memory multiplication\n");
+            return -1;
+        }
+        uint64_t prod = fw_kb * imgp_pct_u;
+        /* Check for overflow in prod + 99ULL */
+        if (prod > UINT64_MAX - 99ULL) {
+            XCONF_LOG_ERROR("[FWCHK] Overflow in image processing memory addition\n");
+            return -1;
+        }
+        img_proc_kb = (prod + 99ULL) / 100ULL;
+    }
+    /* Calculate required_kb = fw_kb + rsrv_kb + img_proc_kb with overflow checks */
+    uint64_t required_kb = 0;
+    if (fw_kb > UINT64_MAX - rsrv_kb) {
+        XCONF_LOG_ERROR("[FWCHK] Overflow in required memory calculation (fw_kb + rsrv_kb)\n");
+        return -1;
+    }
+    uint64_t tmp_required = fw_kb + rsrv_kb;
+    if (tmp_required > UINT64_MAX - img_proc_kb) {
+        XCONF_LOG_ERROR("[FWCHK] Overflow in required memory calculation (sum + img_proc_kb)\n");
+        return -1;
+    }
+    required_kb = tmp_required + img_proc_kb;

	uint64_t img_proc_kb = (fw_kb * (uint64_t)imgp_pct + 99ULL) / 100ULL;
	uint64_t img_proc_kb = (fw_kb / 100ULL) * (uint64_t)imgp_pct
	+ ((fw_kb % 100ULL) * (uint64_t)imgp_pct + 99ULL) / 100ULL;

-                 "curl -sI '%s%s%s' | grep -i Content-Length | awk '{print $2}'",
+                 "curl -sI '%s%s%s' | awk -F':' 'BEGIN{IGNORECASE=1} "
+                 "/^[[:space:]]*Content-Length[[:space:]]*:/ {"
+                 "gsub(/^[ \\t]+/, \"\", $2); "
+                 "gsub(/[ \\t\\r\\n]+$/, \"\", $2); "
+                 "print $2; exit}'",

-                 url, (url[strlen(url)-1] == '/') ? "" : "/", fname);
+                 url,
+                 ((url[0] != '\0') && (url[strlen(url) - 1] == '/' || fname[0] == '/')) ? "" : "/",
+                 (fname[0] == '/') ? (fname + 1) : fname);


		lib_LTLIBRARIES = libfw_download_chk.la

		libfw_download_chk_la_SOURCES = fw_download_check.c

	libfw_download_chk_la_SOURCES = fw_download_check.c
	libfw_download_chk_la_SOURCES = fw_download_check.c
	include_HEADERS = fw_download_check.h

+/**
+ * @brief Check whether a firmware download can proceed based on available memory.
+ *
+ * This function performs any required checks (such as validating available
+ * memory) to determine whether it is safe to start a firmware download.
+ *
+ * @return FW_DWNLD_MEMCHK_SUCCEED if the firmware download can proceed;
+ *         FW_DWNLD_MEMCHK_NOT_ENOUGH_MEM if there is insufficient memory
+ *         for the firmware download; or FW_DWNLD_MEMCHK_FAILED if the
+ *         memory check itself failed or could not be completed.
+ *
+ * @note This function takes no parameters. Any side effects depend on the
+ *       underlying implementation of the memory check (for example, system
+ *       resource queries), but it is not expected to modify persistent state.
+ */

Conversation

bunnam988 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

bunnam988 commented Jan 21, 2026 •

edited

Loading