DemesneGH commented on PR #278:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/278#issuecomment-3900522469
Thanks for the changes! Since this is a core part of the API design, it's
worth taking the extra time to consider it carefully. I really appreciate your
patience and collaboration on this!
Some new suggestions:
I’m wondering if we can also eliminate the another `unreachable!()` block in
updating size.
Since checking capacity, copying memory, and updating the size are almost
always performed together, how about merging them into a single atomic method?
This makes the example code much cleaner and user-friendly, also prevents
developers from forgetting necessary checks or size updates.
Like this:
```
impl ParamMemref<'_, _> {
/// Copies data into the buffer and automatically updates the memref
size.
/// Returns `ShortBufferErr` and updates the size hint if the capacity
is insufficient.
pub fn copy_from_and_update_size(&mut self, src: &[u8]) -> Result<(),
Error> {
let len = src.len();
// 1. Check capacity
self.ensure_capacity(len)?;
// 2. Access the buffer and perform the copy
let mut buf = self.buffer()?;
buf.copy_from(src)?;
// 3. Update the raw memref size, the len has been checked
unsafe { self.raw.as_mut() }.size = len;
Ok(())
}
}
```
In the example:
```
let bytes = serialized.as_bytes();
// Atomically check capacity, copy data, and update the size
p.copy_from_and_update_size(bytes)?;
```
Let me know what you think, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
