-
Notifications
You must be signed in to change notification settings - Fork 11
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 impl TryFrom<&[u8]> and other methods for SigData #14
Conversation
src/attestation_types/quoteheader.rs
Outdated
//! The QuoteHeader is part of the Quote structure. See the Quote module for more. | ||
|
||
use super::quote::QuoteError; | ||
use std::convert::TryFrom; |
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.
you might get rid of std
by using core::convert::TryFrom
src/attestation_types/mod.rs
Outdated
@@ -6,5 +6,12 @@ | |||
|
|||
#[cfg(feature = "std")] | |||
pub mod quote; | |||
|
|||
#[cfg(feature = "std")] |
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.
might not be needed
src/attestation_types/sigdata.rs
Outdated
|
||
/// A.4, Table 4 | ||
#[derive(Default)] | ||
#[repr(C)] |
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.
I don't think this makes sense with Vec
.
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.
If it can't be #[repr(c)]
then I may need to have a different version of the struct that's created from this original once the size is known. Maybe a debt issue...
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.
Why do you need #[repr(C)]
? for FFI?
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.
Yeah, this may potentially get passed around between Rust and C.
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.
It may also be possible in the near future to remove the vec
fields, which would be ideal. Although the Quote
can change in size, I think the version we're using will always be the same size. But I have to test that theory.
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.
src/attestation_types/quote.rs
Outdated
report::{Body, ReportError}, | ||
sigdata::SigData, | ||
}; | ||
use std::fmt; |
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.
could use core::fmt
and get rid of std
Signed-off-by: Lily Sturmann <[email protected]>
Note that the size of
SigData
isn't known at compilation time, so animpl TryFrom<&[u8]> for SigData
is appropriate.I'm also open to making some of the byte lengths, etc., into constants, but there are so many that I thought it would just be even less readable. Open to suggestions on this.
Related to enarx/enarx#917
related to enarx/enarx#84