Repository: google/OpenSK
File: src/lib.rs (Rust)

Code before:
use crate::ctap::CtapState;
pub use crate::ctap::Transport;
use crate::env::Env;
use clock::CtapInstant;

// Those macros should eventually be split into trace, debug, info, warn, and error macros when
// adding either the defmt or log feature and crate dependency.
#[cfg(feature = "debug_ctap")]
macro_rules! debug_ctap {
    ($env: expr, $($rest:tt)*) => {{
        use core::fmt::Write;
        writeln!($env.write(), $($rest)*).unwrap();
    }};
}
#[cfg(not(feature = "debug_ctap"))]
macro_rules! debug_ctap {
    ($env: expr, $($rest:tt)*) => {
        // To avoid unused variable warnings.
        let _ = $env;
    };
}

pub mod api;
pub mod clock;
// Implementation details must be public for testing (in particular fuzzing).
#[cfg(feature = "std")]
pub mod ctap;
#[cfg(not(feature = "std"))]
pub mod ctap;
pub mod env;
#[cfg(feature = "std")]
pub mod test_helpers;

/// CTAP implementation parameterized by its environment.
pub struct Ctap<E: Env> {
    env: E,
    state: CtapState,
    hid: MainHid,
    #[cfg(feature = "vendor_hid")]
    vendor_hid: VendorHid,
}

impl<E: Env> Ctap<E> {
    /// Instantiates a CTAP implementation given its environment.
    // This should only take the environment, but it temporarily takes the boot time until the
    // clock is part of the environment.
    pub fn new(mut env: E, now: CtapInstant) -> Self {
        let state = CtapState::new(&mut env, now);
        let hid = MainHid::new();
        #[cfg(feature = "vendor_hid")]
        let vendor_hid = VendorHid::new();

Review comment (bug):
Interesting. I'm surprised it used to compile before. I would have expected that the `Ctap::state()` doesn't compile because it is public and its return type is private.

The idea behind a private `ctap` module is that we consider it internal implementation detail. Users should use the `Ctap` struct.

Given that this is already broken, I'd suggest simply using `pub mod ctap` for now (without the `feature = "std"` switch). We could also remove the comment about testing given that it's outdated (although it's the intent).

Code after:
use crate::ctap::CtapState;
pub use crate::ctap::Transport;
use crate::env::Env;
use clock::CtapInstant;

// Those macros should eventually be split into trace, debug, info, warn, and error macros when
// adding either the defmt or log feature and crate dependency.
#[cfg(feature = "debug_ctap")]
macro_rules! debug_ctap {
    ($env: expr, $($rest:tt)*) => {{
        use core::fmt::Write;
        writeln!($env.write(), $($rest)*).unwrap();
    }};
}
#[cfg(not(feature = "debug_ctap"))]
macro_rules! debug_ctap {
    ($env: expr, $($rest:tt)*) => {
        // To avoid unused variable warnings.
        let _ = $env;
    };
}

pub mod api;
pub mod clock;
// TODO(kaczmarczyck): Refactor this so that ctap module isn't public.
pub mod ctap;
pub mod env;
#[cfg(feature = "std")]
pub mod test_helpers;

/// CTAP implementation parameterized by its environment.
pub struct Ctap<E: Env> {
    env: E,
    state: CtapState,
    hid: MainHid,
    #[cfg(feature = "vendor_hid")]
    vendor_hid: VendorHid,
}

impl<E: Env> Ctap<E> {
    /// Instantiates a CTAP implementation given its environment.
    // This should only take the environment, but it temporarily takes the boot time until the
    // clock is part of the environment.
    pub fn new(mut env: E, now: CtapInstant) -> Self {
        let state = CtapState::new(&mut env, now);
        let hid = MainHid::new();
        #[cfg(feature = "vendor_hid")]
        let vendor_hid = VendorHid::new();
        Ctap {
            env,
            state,