Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix memory leaks with cJSON #1653

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Fix memory leaks with cJSON #1653

merged 1 commit into from
Aug 28, 2024

Conversation

MattCatz
Copy link
Contributor

cJSON structures need to be deleted rather than free-ed so all sub-structures are cleaned up.

I don't think this was causing any major leaks but should now be causing no leaks :D.

See:

Onion/include/cjson/cJSON.h

Line 146 in 7dfc008

/* Memory Management: the caller is always responsible to free the results from all variants of cJSON_Parse (with cJSON_Delete) and cJSON_Print (with stdlib free, cJSON_Hooks.free_fn, or cJSON_free as appropriate). The exception is cJSON_PrintPreallocated, where the caller has full responsibility of the buffer. */

@MattCatz
Fix memory leaks with cJSON
fc283e5 
`cJSON` structures need to be deleted rather than free-ed so all
sub-structures are cleaned up.

I don't think this was causing any major leaks but should now be
causing no leaks :D.
Aemiii91
Aemiii91 approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@Aemiii91 Aemiii91 left a comment

Choose a reason for hiding this comment

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

You learn something every day.. did you compile and test this?

@Aemiii91 Aemiii91 added the bug in onion Something isn't working label Aug 27, 2024
@MattCatz
Copy link
Contributor Author

You learn something every day.. did you compile and test this?

I have compiled and run this. I cherry-picked this form between some other patches so let me know if it doesn't apply correctly.

@Aemiii91 Aemiii91 added this pull request to the merge queue Aug 28, 2024
Merged via the queue into OnionUI:main with commit 2a0c231 Aug 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aemiii91 Aemiii91 Aemiii91 approved these changes

Assignees

@MattCatz MattCatz

Labels
bug in onion Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MattCatz @Aemiii91