-
Notifications
You must be signed in to change notification settings - Fork 21
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
RDKBWIFI-17 Fixes for lack of cJSON_free for cJSON_Print output #117
base: main
Are you sure you want to change the base?
RDKBWIFI-17 Fixes for lack of cJSON_free for cJSON_Print output #117
Conversation
@@ -111,7 +111,9 @@ int main(int argc, const char *argv[]) | |||
|
|||
if ((node = exec(line, strlen(line), NULL)) != NULL) { | |||
if ((obj = (cJSON *)network_tree_to_json(node)) != NULL) { | |||
printf("%s\n", cJSON_Print(obj)); | |||
char* tmp = cJSON_Print(obj); | |||
printf("%s\n", tmp); |
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.
It is cli which will be deprecated.
snprintf(subdoc->buff,sizeof(em_subdoc_data_buff_t),"%s",cJSON_Print(parent_obj)); | ||
char* tmp = cJSON_Print(parent_obj); | ||
snprintf(subdoc->buff,sizeof(em_subdoc_data_buff_t),"%s",tmp); | ||
cJSON_free(tmp); |
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.
I am not able to reproduce the issue again:
https://jira.rdkcentral.com/jira/browse/RDKBWIFI-17?focusedId=457298&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-457298
Engineer with higher experience in cJSON should give opinion if we can process this diff. I would like avoid double-free situation
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.
@soumyasmunshi let me know what do you think about that pull request. It seems evident that we have problem with memory leaks in unified-wifi-mesh due to cJSON_Print. I would like to avoid push this changes before your presentation for your bosses to avoid double-free-corruption (in the case if it is freed already).
All these memory leaks were detected by AddressSanitizer but I keep only one callstack: https://jira.rdkcentral.com/jira/browse/RDKBWIFI-17?focusedId=457298&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-457298
I think that this change can needed more tests (at least reproduce the issue). I will keep it as draft.
Yes, thanks for fixing this, that was a leak and defect.
Soumya
From: marcin-matula-2329518 ***@***.***>
Date: Thursday, January 9, 2025 at 8:18 AM
To: rdkcentral/unified-wifi-mesh ***@***.***>
Cc: Munshi, Soumya ***@***.***>, Mention ***@***.***>
Subject: Re: [rdkcentral/unified-wifi-mesh] RDKBWIFI-17 Fixes for lack of cJSON_free for cJSON_Print output (PR #117)
@marcin-matula-2329518 commented on this pull request.
________________________________
In src/dm/dm_easy_mesh.cpp<https://urldefense.com/v3/__https:/github.com/rdkcentral/unified-wifi-mesh/pull/117*discussion_r1909108223__;Iw!!CQl3mcHX2A!HNOfkz7N6pgKxrfmxYM9YzahHA-B_Jw0XfYggnIO9egaJE0TLZ-VrjDNv8h_NLKVacQrtVNvOWoS9KSzU_GWdYlxcxmZWg$>:
@@ -353,7 +353,9 @@ int dm_easy_mesh_t::encode_config_reset(em_subdoc_info_t *subdoc, const char *ke
formatted_json = cJSON_Print(parent_obj);
printf("%s:%d: %s\n", __func__, __LINE__, formatted_json);
- snprintf(subdoc->buff,sizeof(em_subdoc_data_buff_t),"%s",cJSON_Print(parent_obj));
+ char* tmp = cJSON_Print(parent_obj);
+ snprintf(subdoc->buff,sizeof(em_subdoc_data_buff_t),"%s",tmp);
+ cJSON_free(tmp);
@soumyasmunshi<https://urldefense.com/v3/__https:/github.com/soumyasmunshi__;!!CQl3mcHX2A!HNOfkz7N6pgKxrfmxYM9YzahHA-B_Jw0XfYggnIO9egaJE0TLZ-VrjDNv8h_NLKVacQrtVNvOWoS9KSzU_GWdYkcbJZEcA$> let me know what do you think about that pull request. It seems evident that we have problem with memory leaks in unified-wifi-mesh due to cJSON_Print. I would like to avoid push this changes before your presentation for your bosses to avoid double-free-corruption (in the case if it is freed already).
All these memory leaks were detected by AddressSanitizer but I keep only one callstack: https://jira.rdkcentral.com/jira/browse/RDKBWIFI-17?focusedId=457298&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-457298
I think that this change can needed more tests (at least reproduce the issue).
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/rdkcentral/unified-wifi-mesh/pull/117*discussion_r1909108223__;Iw!!CQl3mcHX2A!HNOfkz7N6pgKxrfmxYM9YzahHA-B_Jw0XfYggnIO9egaJE0TLZ-VrjDNv8h_NLKVacQrtVNvOWoS9KSzU_GWdYlxcxmZWg$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/BH3D6PJZBOOFXDWZZBKDDKT2J2ONFAVCNFSM6AAAAABU4QJBQWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNBQGM3DQNJTGQ__;!!CQl3mcHX2A!HNOfkz7N6pgKxrfmxYM9YzahHA-B_Jw0XfYggnIO9egaJE0TLZ-VrjDNv8h_NLKVacQrtVNvOWoS9KSzU_GWdYm_sqnnzQ$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
4b46282
to
2b5ac10
Compare
No description provided.