Skip to content

RDKB-63687 Code development to reduce SM app memory usage#921

Open
skarth597 wants to merge 9 commits intordkcentral:developfrom
skarth597:sm_app
Open

RDKB-63687 Code development to reduce SM app memory usage#921
skarth597 wants to merge 9 commits intordkcentral:developfrom
skarth597:sm_app

Conversation

@skarth597
Copy link
Contributor

Reason for change: To reduce the SM app memory usage
Test Procedure: Debug
Priority: P1
Risks: Low
Signed-off-by: Samyuktha Karthikeyan samyuktha_karthikeyan@comcast.com

Copilot AI review requested due to automatic review settings February 25, 2026 09:22
@skarth597 skarth597 requested a review from a team as a code owner February 25, 2026 09:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces SM app memory usage by changing client_assoc_stats from a fixed-size per-MAX_NUM_RADIOS global array to a dynamically allocated buffer sized to the runtime number of radios.

Changes:

  • Replaced client_assoc_stats[MAX_NUM_RADIOS] with a heap-allocated client_assoc_stats pointer.
  • Allocated client_assoc_stats during sm_init() using getNumberRadios() and freed it during sm_deinit().

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

Comment on lines +892 to 898
client_assoc_stats = calloc(radios_count, sizeof(client_assoc_stats_t));
if (client_assoc_stats == NULL) {
wifi_util_error_print(WIFI_SM, "%s:%d: calloc failed for client_assoc_stats\n", __func__, __LINE__);
return RETURN_ERR;
}
rc = sm_report_init(app);

Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sm_report_init(app) fails, sm_init() returns an error but client_assoc_stats (allocated here) is not freed, and the newly-created maps also remain allocated. Add cleanup on the failure path (free client_assoc_stats, destroy sm_stats_config_map / report_tasks_map, etc.), or refactor sm_init() to a single goto-based cleanup block.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Reason for change: To reduce the SM app memory usage
Test Procedure: Debug
Priority: P1
Risks: Low
Signed-off-by: Samyuktha Karthikeyan <samyuktha_karthikeyan@comcast.com>
Copilot AI review requested due to automatic review settings February 25, 2026 11:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment on lines +892 to +895
client_assoc_stats = calloc(radios_count, sizeof(client_assoc_stats_t));
if (client_assoc_stats == NULL) {
wifi_util_error_print(WIFI_SM, "%s:%d: calloc failed for client_assoc_stats\n", __func__, __LINE__);
return RETURN_ERR;
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the calloc failure path, sm_stats_config_map and report_tasks_map have already been created, but the function returns immediately without destroying them. Add cleanup for those allocations (and any other init steps in this function) before returning to avoid leaks when SM init fails early.

Copilot uses AI. Check for mistakes.
Comment on lines 897 to +901
rc = sm_report_init(app);
if (rc != RETURN_OK) {
free(client_assoc_stats);
client_assoc_stats = NULL;
}
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sm_report_init(app) fails, this code frees client_assoc_stats but leaves sm_stats_config_map / report_tasks_map allocated and returns the error. Please add a consolidated failure cleanup path (e.g., goto cleanup) that also destroys these maps (and any other partially-initialized resources) before returning.

Copilot uses AI. Check for mistakes.
Reason for change: To reduce the SM app memory usage
Test Procedure: Debug
Priority: P1
Risks: Low
Signed-off-by: Samyuktha Karthikeyan <samyuktha_karthikeyan@comcast.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants