Implement pre allocation context creation #125
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
apoelstra
reviewed
Jul 5, 2019
apoelstra
reviewed
Jul 5, 2019
apoelstra
reviewed
Jul 5, 2019
src/context.rs
Outdated
| const DESCRIPTION: &'static str = "signing only"; | ||
|
|
||
| fn deallocate(ptr: *mut [u8]) { | ||
| let _ = ptr; |
There was a problem hiding this comment.
I think it'd be clearer to write
fn deallocate(_: *mut [u8]) {
// do nothing
}
I read let _ = ptr as "drop ptr" which is technically what's happening but it's a bit weird because dropping a pointer is a noop.
apoelstra
reviewed
Jul 5, 2019
src/lib.rs
Outdated
| @@ -570,7 +570,7 @@ impl<C: Context> Secp256k1<C> { | |||
| } | |||
|
|
|||
| /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context | |||
| pub fn preallocate_size() -> usize { | |||
| pub(crate) fn preallocate_size_gen() -> usize { | |||
There was a problem hiding this comment.
I think we should still export this. It's needed to estimate the allocation size in generic contexts.
There was a problem hiding this comment.
I mostly didn't want to confuse the user with too many functions, some generic and some not
|
Confirmed that I can build and run a rust-wasm project with this. ACK. |
|
@apoelstra That's great :) Thanks! |
apoelstra
changed the title
apoelstra
approved these changes
Jul 5, 2019
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.
Hi,
After talking with @apoelstra and after the great feedback and discussion on #116
I wrote this new API.
If it's too broad of a change I can try to split it into smaller PRs.
After this there shouldn't be any free/malloc/fprintf symbols in this library.
(which will help both wasm(#124) and full
no-stdembedded devices)Edit: I missed a
realloc(didn't even think of that one haha), rebased and pushed.Fixes #114
Fixes #117
Fixes #124