DemesneGH commented on code in PR #154:
URL:
https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/154#discussion_r1873279365
##########
optee-utee/src/extension.rs:
##########
@@ -26,32 +26,304 @@ pub struct LoadablePlugin {
uuid: Uuid
}
+pub struct LoadablePluginCommand<'a> {
+ plugin: &'a LoadablePlugin,
+ cmd_id: u32,
+ sub_cmd_id: u32,
+ buffer: Vec<u8>,
+}
+
impl LoadablePlugin {
pub fn new(uuid: &Uuid) -> Self {
Self { uuid: uuid.to_owned() }
}
- pub fn invoke(&mut self, command_id: u32, subcommand_id: u32, data: &[u8])
-> Result<Vec<u8>> {
- let raw_uuid: Uuid = self.uuid;
+ pub fn invoke(&self, command_id: u32, subcommand_id: u32, data: &[u8]) ->
Result<Vec<u8>> {
Review Comment:
Can we have some comments on each functions to provide guidance for
developers, such as the suitable scenario for `invoke()` and
`invoke_with_capacity()`? The comments can also be rendered into our API
documentation.
FYI One example for the comment format:
https://github.com/apache/incubator-teaclave-trustzone-sdk/blob/20e698863932d395b48002e5fb126491e0caef0d/optee-utee/src/object.rs#L83-L94
##########
optee-utee/src/extension.rs:
##########
@@ -26,32 +26,304 @@ pub struct LoadablePlugin {
uuid: Uuid
}
+pub struct LoadablePluginCommand<'a> {
+ plugin: &'a LoadablePlugin,
+ cmd_id: u32,
+ sub_cmd_id: u32,
+ buffer: Vec<u8>,
+}
+
impl LoadablePlugin {
pub fn new(uuid: &Uuid) -> Self {
Self { uuid: uuid.to_owned() }
}
- pub fn invoke(&mut self, command_id: u32, subcommand_id: u32, data: &[u8])
-> Result<Vec<u8>> {
- let raw_uuid: Uuid = self.uuid;
+ pub fn invoke(&self, command_id: u32, subcommand_id: u32, data: &[u8]) ->
Result<Vec<u8>> {
+ self.new_command_with_capacity(command_id, data.len())
+ .sub_cmd(subcommand_id)
+ .chain_write_body(data)
+ .call()
+ }
+ pub fn invoke_with_capacity<'a>(
+ &'a self,
+ command_id: u32,
+ subcommand_id: u32,
+ capacity: usize,
+ ) -> LoadablePluginCommand<'a> {
+ self.new_command_with_capacity(command_id, capacity)
+ .sub_cmd(subcommand_id)
+ }
+ // for people who wants to set the buffer size for response manually
+ pub fn new_command_with_capacity<'a>(
+ &'a self,
+ command_id: u32,
+ capacity: usize,
+ ) -> LoadablePluginCommand<'a> {
+ LoadablePluginCommand::new_with_capacity(self, command_id, capacity)
+ }
+}
+
+impl<'a> LoadablePluginCommand<'a> {
+ // use this to set sub_cmd if needed
+ pub fn sub_cmd(mut self, sub_cmd_id: u32) -> Self {
+ self.sub_cmd_id = sub_cmd_id;
+ self
+ }
+ // use this to write request body if needed
+ pub fn write_body(&mut self, data: &[u8]) {
+ self.buffer.extend_from_slice(data);
+ }
+ // same with write_body, but chainable
+ pub fn chain_write_body(mut self, data: &[u8]) -> Self {
+ self.write_body(data);
+ self
+ }
+ // invoke the command, and get result from it
+ pub fn call(self) -> Result<Vec<u8>> {
let mut outlen: usize = 0;
+ let mut buffer = self.buffer;
+ buffer.resize(buffer.capacity(), 0); // resize to capacity first
match unsafe {
raw::tee_invoke_supp_plugin(
- raw_uuid.as_raw_ptr(),
- command_id as u32,
- subcommand_id as u32,
- data.as_ptr() as _,
- data.len(),
+ self.plugin.uuid.as_raw_ptr(),
+ self.cmd_id,
+ self.sub_cmd_id,
+ // convert the pointer manually, as in some platform c_char is
i8
+ buffer.as_mut_slice().as_mut_ptr() as *mut _,
+ buffer.len(),
&mut outlen as *mut usize,
)
} {
raw::TEE_SUCCESS => {
- assert!(outlen <= (data.len()));
- let mut outbuf = vec![0; outlen];
- outbuf.copy_from_slice(&data[..outlen]);
-
- Ok(outbuf)
- },
+ assert!(outlen <= buffer.len());
Review Comment:
This is a legacy issue that we use `assert!` which may cause panic. Please
help to correct this to raise an error for developer, thanks!
##########
optee-utee/src/extension.rs:
##########
@@ -26,32 +26,304 @@ pub struct LoadablePlugin {
uuid: Uuid
}
+pub struct LoadablePluginCommand<'a> {
+ plugin: &'a LoadablePlugin,
+ cmd_id: u32,
+ sub_cmd_id: u32,
+ buffer: Vec<u8>,
+}
+
impl LoadablePlugin {
pub fn new(uuid: &Uuid) -> Self {
Self { uuid: uuid.to_owned() }
}
- pub fn invoke(&mut self, command_id: u32, subcommand_id: u32, data: &[u8])
-> Result<Vec<u8>> {
- let raw_uuid: Uuid = self.uuid;
+ pub fn invoke(&self, command_id: u32, subcommand_id: u32, data: &[u8]) ->
Result<Vec<u8>> {
+ self.new_command_with_capacity(command_id, data.len())
+ .sub_cmd(subcommand_id)
+ .chain_write_body(data)
+ .call()
+ }
+ pub fn invoke_with_capacity<'a>(
+ &'a self,
+ command_id: u32,
+ subcommand_id: u32,
+ capacity: usize,
+ ) -> LoadablePluginCommand<'a> {
+ self.new_command_with_capacity(command_id, capacity)
+ .sub_cmd(subcommand_id)
+ }
+ // for people who wants to set the buffer size for response manually
+ pub fn new_command_with_capacity<'a>(
+ &'a self,
+ command_id: u32,
+ capacity: usize,
+ ) -> LoadablePluginCommand<'a> {
+ LoadablePluginCommand::new_with_capacity(self, command_id, capacity)
+ }
+}
+
+impl<'a> LoadablePluginCommand<'a> {
+ // use this to set sub_cmd if needed
+ pub fn sub_cmd(mut self, sub_cmd_id: u32) -> Self {
+ self.sub_cmd_id = sub_cmd_id;
+ self
+ }
+ // use this to write request body if needed
+ pub fn write_body(&mut self, data: &[u8]) {
+ self.buffer.extend_from_slice(data);
+ }
+ // same with write_body, but chainable
+ pub fn chain_write_body(mut self, data: &[u8]) -> Self {
+ self.write_body(data);
+ self
+ }
+ // invoke the command, and get result from it
+ pub fn call(self) -> Result<Vec<u8>> {
let mut outlen: usize = 0;
+ let mut buffer = self.buffer;
+ buffer.resize(buffer.capacity(), 0); // resize to capacity first
match unsafe {
raw::tee_invoke_supp_plugin(
- raw_uuid.as_raw_ptr(),
- command_id as u32,
- subcommand_id as u32,
- data.as_ptr() as _,
- data.len(),
+ self.plugin.uuid.as_raw_ptr(),
+ self.cmd_id,
+ self.sub_cmd_id,
+ // convert the pointer manually, as in some platform c_char is
i8
+ buffer.as_mut_slice().as_mut_ptr() as *mut _,
+ buffer.len(),
&mut outlen as *mut usize,
)
} {
raw::TEE_SUCCESS => {
- assert!(outlen <= (data.len()));
- let mut outbuf = vec![0; outlen];
- outbuf.copy_from_slice(&data[..outlen]);
-
- Ok(outbuf)
- },
+ assert!(outlen <= buffer.len());
+ buffer.resize(outlen, 0);
+ Ok(buffer)
+ }
code => Err(Error::from_raw_error(code)),
}
-
+ }
+}
+
+impl<'a> LoadablePluginCommand<'a> {
+ fn new_with_capacity(plugin: &'a LoadablePlugin, cmd_id: u32, capacity:
usize) -> Self {
+ Self {
+ plugin,
+ cmd_id,
+ sub_cmd_id: 0,
Review Comment:
`new_with_capacity()` set a default sub_cmd_id then we set that manually:
```
self.new_command_with_capacity(command_id, capacity)
.sub_cmd(subcommand_id)
```
How about setting the sub_cmd when new()?
```
new_with_capacity(plugin: &'a LoadablePlugin, cmd_id: u32, sub_cmd_id: u32,
capacity: usize)
```
--
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: dev-unsubscr...@teaclave.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org
For additional commands, e-mail: dev-h...@teaclave.apache.org
