-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add KeylimeTpmError support and change return type in tpm.rs #53
Conversation
1. Changed return type with error handling Result<> type 2. Removed rise_on_error parameter, since error is return by Result, There is no need to handle error within the run() function. 3. Converted the return outptu to String before returning 4. Changed function comment accordingly 5. Removed error logging within run(). Error information is return by returning Err() to Result<>
1. Implement From trait for KeylimeTpmError: std::string::FromUtf8Error std::io::Error std::time::SystemTimeError 2. Change return type of run() function in a tuple and a custom error Result<(String, String), KeylimeError> 3. Two enum type representing error cause by different part of tpm execution. - TpmError: error cause by TPM command execution - TpmRustError: error cause by rust function calls
@mbestavros PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frozencemetery, @leonjia0112, things look good to me. Definitely makes the code much more readable, IMO -- a lot less boilerplate.
If I were to look for any improvement, it may help to make some of the more generic errors that KeylimeTpmError
inherits a bit more verbose -- for example, it may be helpful for the FromUtf8
or serde_json
errors to tell us where exactly it's going wrong, especially if they're being used in a lot of places.
Other than that, I've included a few minor typo fixes inline; nothing major.
@mbestavros thanks for catching those typos. Those are handled. But for the improvement, I wasn't very sure what you mean. Do you suggest putting additional information instead of just using the description function? |
025ad02
to
65ed7c5
Compare
It was updated to 2.1 based on Keylime enhancement keylime#53 (agent-local revocation), but this broke communication with the Python components. We have decided that the Rust agent should not have its own API version, but rather follow API versioning from the Python components. Signed-off-by: Lily Sturmann <[email protected]>
It was updated to 2.1 based on Keylime enhancement #53 (agent-local revocation), but this broke communication with the Python components. We have decided that the Rust agent should not have its own API version, but rather follow API versioning from the Python components. Signed-off-by: Lily Sturmann <[email protected]>
Add support for tpm custom error type. This is aim on solving #40 for changing return type.
Implementation details:
serde_json::error::Error
std::num::ParseIntError
std::string::FromUtf8Error
std::io::Error
std::time::SystemTimeError
run()
function in a tuple and a custom errorResult<(String, String), TpmExecError>
KeylimeTpmError
, which has benefit in code efficient and early return efficient