Skip to content
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

Port run and KeylimeTpmError to unique modules #82

Merged
merged 2 commits into from
Oct 30, 2019
Merged

Port run and KeylimeTpmError to unique modules #82

merged 2 commits into from
Oct 30, 2019

Conversation

lukehinds
Copy link
Member

This change ports the run function from tpm.rs into its own
file / module cmd_exec.rs - primary reason for this is that non
tpm functions also call on the run function, so its not
exclusive to tpm operations (for example secure_mount uses run)

For KeylimeTpmError - this has been ported into a module
keylime_error, so that we can group all errors into a single
module, as it makes sense that we will eventually have more
error types and have them neatly self contained in single module

Resolves: #81

This change ports the `run` function from `tpm.rs` into its own
file / module `cmd_exec.rs` - primary reason for this is that non
tpm functions also call on the `run` function, so its not
exclusive to tpm operations (for example secure_mount uses run)

For `KeylimeTpmError` - this has been ported into a module
`keylime_error`, so that we can group all errors into a single
module, as it makes sense that we will eventually have more
error types and have them neatly self contained in single module

Resolves: #81
pub fn run<'a>(
command: String,
output_path: Option<&str>,
) -> Result<(String, String), keylime_error::KeylimeTpmError> {
Copy link
Member Author

@lukehinds lukehinds Oct 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps nit picking, but just noticed run is returning KeylimeTpmError which although works programatically, still reads wrong. If ok, I will refactor this in the tpm2 patch I am fixing up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems fine as long as we don't lose track of it.

Reinstate  std::env into tpm.rs, which was mistakenly removed
in main commit for PR #82
Copy link
Contributor

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commits should be squished before merging.

src/cmd_exec.rs Show resolved Hide resolved
src/cmd_exec.rs Show resolved Hide resolved
@lukehinds lukehinds merged commit bdf879b into keylime:master Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move run and KeyLimeTpmError into their own modules
2 participants