-
Notifications
You must be signed in to change notification settings - Fork 113
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 more doc comments to restricted kernel #4746
Conversation
This should help other teams find their way around more easily
/// Simple no_std compatible equivalent of [`std::io::Read`]. | ||
/// | ||
/// [`std::io::Read`]: <https://doc.rust-lang.org/std/io/trait.Read.html> | ||
pub trait Read { |
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.
interestingly rust doc does not get these links by default. Perhaps related to the building docs from a crate that declares #![no_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.
Thanks
oak_crypto/src/encryptor.rs
Outdated
@@ -106,6 +106,8 @@ impl EncryptionKeyProvider { | |||
} | |||
} | |||
|
|||
/// Exposes the ability to derive a session key from for the provided encapsulated private key, |
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.
nit: "from for" -> "from"
/// #![feature(alloc_error_handler)] | ||
/// | ||
/// extern crate alloc; | ||
/// | ||
/// use oak_restricted_kernel_sdk::entrypoint; | ||
/// | ||
/// #[entrypoint] | ||
/// fn start_enclave_app() -> ! { | ||
/// // business logic starts here | ||
/// /* ... */ | ||
/// } | ||
/// ``` |
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.
interesting bit of trivia, when running tests, cargo compiles this! and throws an error
11 | fn start_enclave_app() -> ! {
| ----------------- ^ expected `!`, found `()`
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 was going to ask about that, because indeed it does not seem semantically correct Rust, and cargo test should reject it. Anyways you can fix that with a loop {}
or similar.
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.
An unimplemented takes care of it. But we run into issues still since the examples declares no_std, which leads us down the rabbit hole of needing to set up the proper target to run the test with (I got that working with https://doc.rust-lang.org/cargo/reference/unstable.html#per-package-target), and then also setting up our own no_std test harness (that's where I gave up).
Unfortunately I think we have to instruct the compiler to ignore this doc test for now.
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.
tracked in #4748
* Add more doc comments to restricted kernel This should help other teams find their way around more easily * fix typos * ignore doctest since it fails compiling for a no_std target * fix todo so the linter won't complain
This should help other teams find their way around more easily