Clarify ownership responsibilities in the C API docs #3296
Merged
Conversation
This file contains 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
|
Hi, this is my first PR for hyper, I may be doing some things incorrectly. Please read the commits one at a time. Thanks! |
|
Thanks so much for the documentation improvements, these are great! ❤️ It looks like the generate header file has some changes the CI doesn't like. Is it because the |
nnethercote
force-pushed
the
docs-clarify-ownership
branch
from
a857a26 to
e76f044
Compare
Curl had two memory leaks in its usage of the hyper C API. I fixed these in curl/curl#11729. While fixing these, as a hyper newbie I found a lack of clarity in the docs about where the responsibilities lie for allocating and deallocating things. I had to repeatedly look at the `Box::into_raw`/`Box::from_raw` calls in the C API code to properly understand things. This commit extends the docs. - For all functions that allocate things, make it clear which functions can be used to subsequently deallocate. - For all functions that free things, make it clear if there are (or aren't) other functions that may have otherwise consumed the thing, thus removing the need for explicit freeing. - Clarify some functions that consume things. This is the documentation I wished was present when I was fixing the leaks in curl.
nnethercote
force-pushed
the
docs-clarify-ownership
branch
from
e76f044 to
27246a9
Compare
|
I had cbindgen 0.12.0 installed, and CI uses 0.25.0! I upgraded, which should fix the problem. |
seanmonstar
approved these changes
Aug 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When using the C API for the first time I found the documentation missing some important details.