close #6: correctly free allocated memory #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And extend policy example by printing short description for each version found, to test it.
Adjust return type of ver_iter_* to be *const c_char, as that data does not need manual freeing. ver_file_parser_* function do require it (they use to_c_string) and the parser itself also needs to be freed. Add a SafeVerFileParser type to free the parser on drop, and free the data returned by the parser ASAP after cloning into a Rust String. Data is allocated with C++ 'new', so we add custom functions to call 'delete' instead of relying on libc::free to avoid any potential issues.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for merging my previous PR! Here's a followup for #6.
I added
to_c_stringas an ugly workaround, and yes, it indeed does require a manual free. Turns out, the parser itself does too. Not a fan of intermingling rust memory allocation and c++ one, so I added seperate "free" functions to the lib that call delete.To test, I extended the "policy" example and added some stuff to the simple API to ensure the new code paths are taken. Then I ran "cargo valgrind", verifying that there are indeed no memory leaks now.
Valgrind also showed my that the ones you had marked
*mut c_charpreviously were in fact automatically released, so I changed them to "const" and updated the comment. I hope that's correct and libapt isn't somehow stupid about that too :)I suppose a
impl Dropwrapper for the returned string data (potentiallyDeref<str>or smth) would work too, but I figured we copy the string once, so might as well copy it twice and make our lives easier by just having Rust do the final free.