m4sterchain commented on code in PR #235:
URL:
https://github.com/apache/teaclave-trustzone-sdk/pull/235#discussion_r2384482937
##########
optee-teec/macros/src/lib.rs:
##########
@@ -143,16 +165,14 @@ pub fn plugin_invoke(_args: TokenStream, input:
TokenStream) -> TokenStream {
fn inner(#params) -> optee_teec::Result<()> {
#f_block
}
- let mut inbuf = unsafe { std::slice::from_raw_parts_mut(data,
in_len as usize) };
+ let mut inbuf = std::slice::from_raw_parts_mut(data, in_len as
usize);
Review Comment:
1. **Null Check for `data`**
Add an explicit null check for the `data` pointer before calling
[`from_raw_parts_mut`](https://doc.rust-lang.org/std/slice/fn.from_raw_parts_mut.html#safety).
The current implementation directly calls `from_raw_parts_mut(data,
in_len as usize)`, which is undefined behavior if `data` is null—even when
`in_len == 0`.
2. **Clarify Contradictory Documentation**
The function-level comments contain a small inconsistency:
- It states that a **valid empty call** allows `data = null` when `in_len
= 0`.
- It also states that **invalid pointers** include `data = null`.
Please clarify this to avoid confusion, I suggest our ffi logic should
handle such case as described in 1.
3. **Purpose of `get_out_slice` Copy**
The FFI layer wraps the `data` buffer with `params` and passes it to
`inner(&mut params)`, which already allows reading/writing in place.
After `inner` returns, the FFI layer calls `params.get_out_slice()` and
copies the output back to `data`.
- If `params` operates directly on the same buffer, what is the purpose
of this additional copy?
- Could you please confirm if this is redundant writes or intentionally
for some corner case?
4. **Handling of `out_len` on Insufficient Buffer**
In some C APIs that use an in/out buffer, `out_len` is used to return the
*required* size when the provided buffer is too small.
It’s not clear whether this convention applies here.
- If so, the current design seems to ignore this scenario.
- If not, consider clarifying this in the documentation to avoid
ambiguity.
--
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]
