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.

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

json_free() guidance #22

Open
milankowww opened this issue Apr 17, 2024 · 1 comment
Open

json_free() guidance #22

milankowww opened this issue Apr 17, 2024 · 1 comment
Assignees
Labels
bug critical Issue that possibly prevents the usage of the library

Comments

@milankowww
Copy link

What is the proper guideline for json_free() to prevent memory leaks? It seems to me from the recursive nature of json_free() that it is sufficient to call it for the topmost object, but still, in the README example you don't call it on any error, which leads me to believe the example contains a memory leak (of course the program exits but that's not the point).

Is it "call it on the value of the topmost unwrap but only if it was successful", or is it something else?

@forkachild forkachild added the bug label Apr 17, 2024
@forkachild forkachild self-assigned this Apr 17, 2024
@forkachild
Copy link
Owner

forkachild commented Apr 17, 2024

Yes, your assumption is correct. It will leak memory on failure. This can be resolved in 2 ways:

  1. Calling json_free() at fallible points in each parsing step.
  2. Sizing the JSON before parsing. This will solve the following:
    1. Fail without allocating anything.
    2. Have one monolithic allocation, avoiding redundant segmentation (due to multiple allocations).
    3. Contiguous cache-friendly memory access.

I would strongly prefer solution 2. What are your thoughts on this?

@forkachild forkachild added the critical Issue that possibly prevents the usage of the library label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug critical Issue that possibly prevents the usage of the library
Projects
None yet
Development

No branches or pull requests

2 participants