Feature/singlebld by GoutamD2905 · Pull Request #38 · rdkcentral/miscellaneous-broadband

GoutamD2905 · 2026-01-23T03:36:10Z

No description provided.

Copilot

Pull request overview

This pull request introduces a new OneStack utility that retrieves partner ID information from multiple sources with a fallback mechanism. The utility is designed to query partner information from HAL APIs, file systems, and system configuration in order of priority.

Changes:

Added OneStack partner ID utility with multi-source fallback logic (HAL API → file → syscfg)
Implemented RDK logging infrastructure for the OneStack module
Integrated build system support with conditional compilation via --enable-onestack flag

Reviewed changes

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

Show a summary per file

File	Description
source/onestack/onestack_log.h	Header defining logging macros and initialization functions for OneStack module
source/onestack/onestack_log.c	Implementation of RDK logger initialization/deinitialization for OneStack
source/onestack/onestack.h	Public API header declaring partner ID retrieval function
source/onestack/onestack.c	Core implementation with fallback logic and main utility program
source/onestack/Makefile.am	Build configuration for OneStack utility with required library dependencies
source/Makefile.am	Integration of OneStack subdirectory into build system
configure.ac	Autoconf configuration for OneStack feature flag (enabled by default)

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

Copilot · 2026-01-23T03:41:37Z

source/onestack/onestack_log.c

+
+bool onestack_log_init(void)
+{
+    if (rdk_logger_init(DEBUG_INI_PATH) != 0)


The return value comparison is inconsistent with codebase conventions. In source/ServiceCtrl/servicecontrol_log.c:10, the pattern is to compare rdk_logger_init return value against RDK_SUCCESS, not against 0. The current comparison may work but doesn't follow the established pattern where RDK_SUCCESS is the symbolic constant used for success checks.

Copilot · 2026-01-23T03:41:37Z

source/onestack/onestack_log.c

+{
+    ONESTACK_DEBUG("%s: Deinitializing logging\n", __FUNCTION__);
+
+    if (rdk_logger_deinit() != 0)


The return value comparison is inconsistent with codebase conventions. In source/ServiceCtrl/servicecontrol_log.c:27, the pattern is to compare rdk_logger_deinit return value against RDK_SUCCESS, not against 0. The current comparison may work but doesn't follow the established pattern where RDK_SUCCESS is the symbolic constant used for success checks.

Copilot · 2026-01-23T03:41:37Z

source/onestack/onestack.c

+            if (fgets(buffer, sizeof(buffer), fp))
+            {
+                trim_newline(buffer);
+
+                if (buffer[0] != '\0')
+                {
+                    strncpy(pValue, buffer, size - 1);
+                    pValue[size - 1] = '\0';
+                    fclose(fp);
+                    ONESTACK_INFO("%s: Partner ID retrieved from file: %s\n", __FUNCTION__, pValue);
+                    return 0;
+                }


The file read operation doesn't validate the content length before copying. If the file contains data longer than BUFLEN_256 (256 bytes), fgets will read up to 255 bytes plus null terminator, which is safe. However, the subsequent strncpy to pValue uses 'size - 1' which could be smaller than the buffer content. While this is handled correctly with null termination, there's no validation that the partner ID read from the file is within expected bounds or contains only valid characters. Consider adding validation to ensure the partner ID contains only expected characters (alphanumeric, hyphens, underscores) to prevent potential issues with downstream consumers.

Copilot · 2026-01-23T03:41:38Z

source/onestack/onestack.c

+    for (retry = 0; retry < MAX_RETRY; retry++)
+    {
+        ONESTACK_DEBUG("%s: Attempting HAL API call (attempt %d/%d)\n", __FUNCTION__, retry + 1, MAX_RETRY);
+        if (platform_hal_getFactoryPartnerId(pValue) == 0 && pValue[0] != '\0')


The platform_hal_getFactoryPartnerId function is called without validating if the buffer size is sufficient. The function is passed pValue which could have a size smaller than what the HAL function expects or writes. The HAL function doesn't receive the size parameter, so it cannot perform bounds checking. If the HAL function writes more data than the buffer can hold, this could result in a buffer overflow. Consider checking the minimum required buffer size for platform_hal_getFactoryPartnerId or ensuring that pValue is always large enough.

Copilot · 2026-01-23T03:41:38Z

source/onestack/onestack.c

+    return -1;
+}
+
+int main(int argc, char *argv[])


The argc and argv parameters in main are unused when ONESTACK_PRODUCT_REQ is defined. While this is not technically an error, consider marking them as unused with (void)argc; (void)argv; or using compiler-specific attributes to suppress warnings, as this is a common practice seen in other utilities like source/FwBankInfo/FwBank_Info.c which doesn't use its main parameters at all.

Copilot · 2026-01-23T03:41:38Z

source/onestack/onestack.c

+        onestack_log_deinit();
+        return 0;
+    }
+
+    ONESTACK_ERROR("Failed to retrieve Partner ID\n");
+    fprintf(stderr, "\n[FAILED] Unable to retrieve Partner ID\n");
+
+    onestack_log_deinit();


The main function does not check the return value of onestack_log_deinit before returning. If deinitalization fails at line 133 or 140, the failure is silently ignored, which could lead to resource cleanup issues. Consider logging a warning if deinitalization fails.

Suggested change

onestack_log_deinit();

return 0;

}

ONESTACK_ERROR("Failed to retrieve Partner ID\n");

fprintf(stderr, "\n[FAILED] Unable to retrieve Partner ID\n");

onestack_log_deinit();

if (!onestack_log_deinit())

{

fprintf(stderr, "Warning: Logging deinitialization failed\n");

}

return 0;

}

ONESTACK_ERROR("Failed to retrieve Partner ID\n");

fprintf(stderr, "\n[FAILED] Unable to retrieve Partner ID\n");

if (!onestack_log_deinit())

{

fprintf(stderr, "Warning: Logging deinitialization failed\n");

}

Copilot · 2026-01-23T03:41:38Z

configure.ac

+yes) ONESTACK_ENABLE=true ;;
+no)  ONESTACK_ENABLE=false ;;
+*) AC_MSG_ERROR([bad value ${enableval} for --enable-onestack]) ;;
+esac],[ONESTACK_ENABLE=true])


The OneStack utility defaults to enabled (ONESTACK_ENABLE=true) while most other optional features in configure.ac default to disabled (false). For example, dhcp_manager (line 136), socketExampleEnable (line 113), and wbCfgTestAppEnable (line 94) all default to false. Unless there's a specific reason for OneStack to be enabled by default, consider changing this to false for consistency with other optional utilities.

apattu200 · 2026-01-23T06:58:24Z

source/Makefile.am

 endif

+if ONESTACK_ENABLE
+ONESTACK = onestack


rename to something else like set_device_stackmode, apply_device_stackmode etc.

apattu200 · 2026-01-23T07:06:44Z

source/onestack/onestack.c

+    for (retry = 0; retry < MAX_RETRY; retry++)
+    {
+        ONESTACK_DEBUG("%s: Attempting HAL API call (attempt %d/%d)\n", __FUNCTION__, retry + 1, MAX_RETRY);
+        if (platform_hal_getFactoryPartnerId(pValue) == 0 && pValue[0] != '\0')


In our test devices, this may always return comcast eventhough activate partnerid is set to someother. Please check /nvram/.partnerID if not exists go with the HAL

apattu200 · 2026-01-23T07:08:00Z

source/onestack/onestack.c

+
+    // 3. Fallback to syscfg
+    ONESTACK_DEBUG("%s: Attempting syscfg_get for PartnerID\n", __FUNCTION__);
+    if (syscfg_get(NULL, "PartnerID", pValue, size) == 0 && pValue[0] != '\0')


This util expected to run even before syscfg. so this won't work.

apattu200 · 2026-01-23T07:09:43Z

source/onestack/onestack_log.h

+#include <stdbool.h>
+#include "rdk_debug.h"
+
+#define LOG_MODULE "LOG.RDK.ONESTACK"


We don't need a separate log file, similar to apply system defaults, log to consolelog.txt.

GoutamD2905 added 3 commits January 21, 2026 19:24

single build code

4a37580

Updating code

06964e3

Updating code

af6575b

GoutamD2905 requested review from a team as code owners January 23, 2026 03:36

Copilot AI review requested due to automatic review settings January 23, 2026 03:36

Copilot started reviewing on behalf of GoutamD2905 January 23, 2026 03:36 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

apattu200 reviewed Jan 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/singlebld#38

Feature/singlebld#38
GoutamD2905 wants to merge 3 commits intodevelopfrom
feature/singlebld

GoutamD2905 commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

apattu200 Jan 23, 2026

Uh oh!

apattu200 Jan 23, 2026

Uh oh!

apattu200 Jan 23, 2026

Uh oh!

apattu200 Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

GoutamD2905 commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants