Remove memory allocation and replace with slices #157
Conversation
This change completely removes the memory management from Heimlig, instead relying on slices provided in the requests for both input and output parameters. As part of this change, the `Channel` concept was replaced with a `Source` + `Sink` abstraction which lends itself better to a future use of async `Stream` and `Sink` from futures.
SCingolani
force-pushed
the
slices-for-params
branch
from
c12fd8a
to
8b38fa4
Compare
| let aad = match &aad { | ||
| Some(aad) => aad.as_slice(), | ||
| Some(aad) => aad, | ||
| None => &[] as &[u8], | ||
| }; | ||
| match crate::crypto::chacha20poly1305::decrypt_in_place_detached( |
There was a problem hiding this comment.
shall we check tag size before calling decrypt_in_place_detached? or does it do a check internally? if so, what is the correct size?
There was a problem hiding this comment.
decrypt_in_place_detached checks the tag size.
The TAG_SIZE is 16 bytes.
There was a problem hiding this comment.
See proposed change in e62f76c, i.e., make it explicit in the types that the tag should have the correct size; this means offloading the error handling to the user; see for instance the test in scheduler.
- Handle request sink being full on `Api::get_random()`. - Make `Response::GetRandom` preserve the mutability of the provided slice. - Remove `common::Error::Alloc` since we do not allocate anymore. - Small fixes in `chachapoly_worker.rs`.
SCingolani
force-pushed
the
slices-for-params
branch
from
f970059
to
e62f76c
Compare
|
If we do e62f76c then we should also remove the tag size check in |
Agreed, let's split the tag size checking into a separate PR and get this one merged. |
This reverts commit e62f76c.
| api.get_random(random_size) | ||
| let random_output = Box::leak(Box::new([0u8; 16])); | ||
| info!(target: "CLIENT", "Sending request: random data (size={})", random_output.len()); | ||
| api.get_random(random_output.as_mut_slice()) |
There was a problem hiding this comment.
Try prepending a &mut instead.
| ); | ||
| // release the memory | ||
| drop(unsafe { Box::from_raw(data) }); |
There was a problem hiding this comment.
Verify that the memory returned matches the one handed out for the request.
| .expect("Failed to allocate buffer for random data") | ||
| .init([0; 16]); | ||
| // Safety: we forget about the box below, so it doesn't get dropped! | ||
| let random_buffer = unsafe { |
There was a problem hiding this comment.
Upstream as leak (and from_raw) to heapless upstream.
SCingolani
force-pushed
the
slices-for-params
branch
from
29542dc
to
75a49f4
Compare
This change completely removes the memory management from Heimlig, instead relying on slices provided in the requests for both input and output parameters.
As part of this change, the
Channelconcept was replaced with aSource+Sinkabstraction which lends itself better to a future use of the asyncStreamandSinktraits and simplifies the handling of lifetimes.This change stems from a discussion in #156 where an alternative abstraction over parameters to and from Heimlig was suggested. Compared to that, this change leverages the full power of the borrow checker to make sure that neither the internal Heimlig code, or the integration code mishandles the memory used to pass in parameters and write out results.