diff --git a/Makefile b/Makefile index 4a5190c64..babd4cb3b 100644 --- a/Makefile +++ b/Makefile @@ -91,6 +91,8 @@ run-unit-tests: | build-build $(MAKE) -C build-build test run-rust-unit-tests: ${MAKE} -C build-build rust-test +run-rust-clippy: + ${MAKE} -C build-build rust-clippy # Must run tests before creating coverage report coverage: | build-build ${MAKE} -C build-build coverage diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 02c23f26f..273edc735 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -401,6 +401,33 @@ if(NOT CMAKE_CROSSCOMPILING) ${CARGO} test --all-features --manifest-path ${CMAKE_CURRENT_SOURCE_DIR}/rust/Cargo.toml --target-dir ${RUST_BINARY_DIR} ${RUST_CARGO_FLAG} ) add_dependencies(rust-test rust-bindgen) + + add_custom_target(rust-clippy + COMMAND + # Force clippy to fully re-run. It is bad at figuring out when to run again and when to use caches. + ${CARGO} clean --manifest-path ${CMAKE_CURRENT_SOURCE_DIR}/rust/Cargo.toml --target-dir ${RUST_BINARY_DIR} + COMMAND + ${CMAKE_COMMAND} -E env + CMAKE_CURRENT_BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} + # enable nightly features in the stable release - needed to activate alloc_error_handler. + # see src/rust/bitbox02-rust-c/src/lib.rs. + # https://github.com/rust-lang/rust/issues/66740 + RUSTC_BOOTSTRAP=1 + ${CARGO} clippy + --all-features + --manifest-path ${CMAKE_CURRENT_SOURCE_DIR}/rust/Cargo.toml + --target-dir ${RUST_BINARY_DIR} + --release + -- # disabled linters: + -A clippy::large_enum_variant + -A clippy::identity_op + -A clippy::match_bool + -A clippy::new_without_default + -A clippy::single_match + -A clippy::iter_nth_zero + + ) + add_dependencies(rust-clippy rust-bindgen) endif() # If a bootloader that locks the bootloader is flashed the bootloader area is permanently read-only. diff --git a/src/rust/apps/ethereum/src/keypath.rs b/src/rust/apps/ethereum/src/keypath.rs index f3a8a0440..cdf0e2a07 100644 --- a/src/rust/apps/ethereum/src/keypath.rs +++ b/src/rust/apps/ethereum/src/keypath.rs @@ -22,7 +22,7 @@ static ACCOUNT_MAX: u32 = 99; // 100 accounts /// xpub keypath. /// @return true if the keypath is valid, false if it is invalid. pub fn is_valid_keypath_xpub(keypath: &[u32], expected_coin: u32) -> bool { - keypath.len() == 4 && &keypath[..4] == &[44 + HARDENED, expected_coin, 0 + HARDENED, 0] + keypath.len() == 4 && keypath[..4] == [44 + HARDENED, expected_coin, 0 + HARDENED, 0] } /// Does limit checks the keypath, whitelisting bip44 purpose, account and change. diff --git a/src/rust/bitbox02-noise/src/noise_xx.rs b/src/rust/bitbox02-noise/src/noise_xx.rs index 41392ee1f..d403c78bd 100644 --- a/src/rust/bitbox02-noise/src/noise_xx.rs +++ b/src/rust/bitbox02-noise/src/noise_xx.rs @@ -93,7 +93,7 @@ impl State { /// `generate_static_private_key()`. pub fn init(&mut self, static_private_key: Sensitive) { let hs = HandshakeState::new( - noise_protocol::patterns::noise_xx().clone(), + noise_protocol::patterns::noise_xx(), false, /* is_initiator = false; the app is the initiator */ &b"Noise_XX_25519_ChaChaPoly_SHA256"[..], Some(static_private_key), diff --git a/src/rust/bitbox02-rust-c/src/app_ethereum.rs b/src/rust/bitbox02-rust-c/src/app_ethereum.rs index 60adfb89d..cc519fcb8 100644 --- a/src/rust/bitbox02-rust-c/src/app_ethereum.rs +++ b/src/rust/bitbox02-rust-c/src/app_ethereum.rs @@ -14,6 +14,8 @@ use super::util::{Bytes, CStrMut}; +/// # Safety +/// `keypath` must be not NULL and contain `keypath_len` u32 elements. #[no_mangle] pub unsafe extern "C" fn rust_ethereum_keypath_is_valid_xpub( keypath: *const u32, @@ -24,6 +26,8 @@ pub unsafe extern "C" fn rust_ethereum_keypath_is_valid_xpub( ethereum::keypath::is_valid_keypath_xpub(keypath, expected_coin) } +/// # Safety +/// `keypath` must be not NULL and contain `keypath_len` u32 elements. #[no_mangle] pub unsafe extern "C" fn rust_ethereum_keypath_is_valid_address( keypath: *const u32, diff --git a/src/rust/bitbox02-rust-c/src/bitboxbase.rs b/src/rust/bitbox02-rust-c/src/bitboxbase.rs index 408bde6a1..833794c6a 100644 --- a/src/rust/bitbox02-rust-c/src/bitboxbase.rs +++ b/src/rust/bitbox02-rust-c/src/bitboxbase.rs @@ -22,7 +22,6 @@ #[allow(non_camel_case_types)] type c_char = u8; -use bitbox02; use core::time::Duration; use bitbox02_rust::platform::bitboxbase::config::Config; @@ -141,29 +140,27 @@ pub fn display_status( bitbox02::COMMANDER_OK } +/// # Safety +/// request must be not NULL. #[no_mangle] -pub extern "C" fn commander_bitboxbase( +pub unsafe extern "C" fn commander_bitboxbase( request: *const bitbox02::BitBoxBaseRequest, ) -> bitbox02::commander_error_t { // Accessing raw pointers are unsafe - let request = match unsafe { request.as_ref() } { + let request = match request.as_ref() { Some(request) => request, None => return bitbox02::COMMANDER_ERR_GENERIC, }; // It is unsafe to access C style unions match request.which_request { bitbox02::BitBoxBaseRequest_confirm_pairing_tag => { - confirm_pairing(unsafe { &request.request.confirm_pairing }) - } - bitbox02::BitBoxBaseRequest_heartbeat_tag => { - heartbeat(unsafe { &request.request.heartbeat }) + confirm_pairing(&request.request.confirm_pairing) } + bitbox02::BitBoxBaseRequest_heartbeat_tag => heartbeat(&request.request.heartbeat), bitbox02::BitBoxBaseRequest_display_status_tag => { - display_status(unsafe { &request.request.display_status }) - } - bitbox02::BitBoxBaseRequest_set_config_tag => { - set_config(unsafe { &request.request.set_config }) + display_status(&request.request.display_status) } + bitbox02::BitBoxBaseRequest_set_config_tag => set_config(&request.request.set_config), _ => bitbox02::COMMANDER_ERR_GENERIC, } } @@ -177,11 +174,13 @@ pub extern "C" fn bitboxbase_config_led_mode_get() -> StatusLedMode { config.status_led_mode.clone() } +/// # Safety +/// `res` must be NOT NULL and of size at least `res_len`. #[no_mangle] -pub extern "C" fn bitboxbase_config_ip_get(res: *mut c_char, res_len: usize) { +pub unsafe extern "C" fn bitboxbase_config_ip_get(res: *mut c_char, res_len: usize) { // It is not safe to call any functions that also touch CONFIG at the same time - let config = unsafe { &CONFIG }; - let buf = unsafe { core::slice::from_raw_parts_mut(res, res_len) }; + let config = &CONFIG; + let buf = core::slice::from_raw_parts_mut(res, res_len); let mut fcstring = FixedCString::new(buf); if let Some(ip) = &config.ip { @@ -211,11 +210,13 @@ pub extern "C" fn bitboxbase_state_get() -> BitBoxBaseBackgroundState { state.state.clone() } +/// # Safety +/// `buf` must be not NULL and be of size at least `buf_len`. #[no_mangle] -pub extern "C" fn bitboxbase_state_get_description(buf: *mut c_char, buf_len: usize) { +pub unsafe extern "C" fn bitboxbase_state_get_description(buf: *mut c_char, buf_len: usize) { assert!(!buf.is_null()); - let state = unsafe { &STATE }; - let buf = unsafe { core::slice::from_raw_parts_mut(buf, buf_len) }; + let state = &STATE; + let buf = core::slice::from_raw_parts_mut(buf, buf_len); let mut buf = FixedCString::new(buf); let _ = write!( buf, @@ -226,9 +227,11 @@ pub extern "C" fn bitboxbase_state_get_description(buf: *mut c_char, buf_len: us ); } +/// # Safety +/// `ptr` must be not NULL and of size at least `ptr_len`. #[no_mangle] -pub extern "C" fn bitboxbase_status_get(ptr: *mut c_char, ptr_len: usize) { - let buf = unsafe { core::slice::from_raw_parts_mut(ptr, ptr_len) }; +pub unsafe extern "C" fn bitboxbase_status_get(ptr: *mut c_char, ptr_len: usize) { + let buf = core::slice::from_raw_parts_mut(ptr, ptr_len); let mut wrapper = FixedCString::new(buf); - write_status(&mut wrapper, unsafe { &CONFIG }); + write_status(&mut wrapper, &CONFIG); } diff --git a/src/rust/bitbox02-rust-c/src/util.rs b/src/rust/bitbox02-rust-c/src/util.rs index c629f9c3a..fc39b0fe2 100644 --- a/src/rust/bitbox02-rust-c/src/util.rs +++ b/src/rust/bitbox02-rust-c/src/util.rs @@ -199,7 +199,7 @@ impl CStrMut { if write_slice.iter().any(|&c| c == 0) { panic!("null terminated strings can't contain null"); } - if let Err(_) = core::str::from_utf8(write_slice) { + if core::str::from_utf8(write_slice).is_err() { panic!("strings must be valid utf-8"); } slice[req] = 0; diff --git a/src/rust/bitbox02-rust/src/platform/bitboxbase/config.rs b/src/rust/bitbox02-rust/src/platform/bitboxbase/config.rs index ba81c557e..cc5658f1b 100644 --- a/src/rust/bitbox02-rust/src/platform/bitboxbase/config.rs +++ b/src/rust/bitbox02-rust/src/platform/bitboxbase/config.rs @@ -79,11 +79,10 @@ impl Config { if !hostname.is_ascii() { return Err(Error::InvalidHostname); } - if hostname.len() < 1 || hostname.len() > 63 { + if hostname.is_empty() || hostname.len() > 63 { return Err(Error::InvalidHostname); } - // Since we checked that the string contains at least one character we can safely unwrap - if hostname.chars().nth(0).unwrap() == '-' || hostname.chars().last().unwrap() == '-' { + if hostname.starts_with('-') || hostname.ends_with('-') { return Err(Error::InvalidHostname); } if hostname.chars().any(|c| !c.is_alphanumeric() && c != '-') { diff --git a/src/rust/bitbox02-rust/src/platform/bitboxbase/display.rs b/src/rust/bitbox02-rust/src/platform/bitboxbase/display.rs index 3cb0804b0..250d74538 100644 --- a/src/rust/bitbox02-rust/src/platform/bitboxbase/display.rs +++ b/src/rust/bitbox02-rust/src/platform/bitboxbase/display.rs @@ -27,16 +27,16 @@ pub fn write_status(w: &mut W, config: &Config) { } else { let _ = write!(w, ""); } - let _ = write!(w, "\n"); + let _ = writeln!(w); let _ = write!(w, "ip: "); if let Some(ip) = &config.ip { let _ = write!(w, "{}", ip); } else { let _ = write!(w, ""); } - let _ = write!(w, "\n"); - let _ = write!(w, "status: OK\n"); - let _ = write!(w, "mode: {:?}\n", config.status_led_mode); + let _ = writeln!(w); + let _ = writeln!(w, "status: OK"); + let _ = writeln!(w, "mode: {:?}", config.status_led_mode); } pub fn display_status(config: &Config, duration: Option) { diff --git a/src/rust/bitbox02-rust/src/util.rs b/src/rust/bitbox02-rust/src/util.rs index ff9b8e88d..3b2488b5e 100644 --- a/src/rust/bitbox02-rust/src/util.rs +++ b/src/rust/bitbox02-rust/src/util.rs @@ -46,10 +46,7 @@ pub struct FixedCString<'a> { impl<'a> FixedCString<'a> { pub fn new(buf: &'a mut [u8]) -> Self { - FixedCString { - buf: buf, - offset: 0, - } + FixedCString { buf, offset: 0 } } } diff --git a/src/rust/bitbox02-rust/src/workflow/pairing.rs b/src/rust/bitbox02-rust/src/workflow/pairing.rs index b65a9359b..db38234f8 100644 --- a/src/rust/bitbox02-rust/src/workflow/pairing.rs +++ b/src/rust/bitbox02-rust/src/workflow/pairing.rs @@ -13,7 +13,6 @@ // limitations under the License. use arrayvec::ArrayString; -use binascii; use core::fmt::Write; pub async fn confirm(hash: &[u8; 32]) -> bool { diff --git a/src/rust/bitbox02/src/password.rs b/src/rust/bitbox02/src/password.rs index fe481d7fa..400825678 100644 --- a/src/rust/bitbox02/src/password.rs +++ b/src/rust/bitbox02/src/password.rs @@ -31,12 +31,12 @@ impl Password { /// Returns the underlying C string buffer (null terminated), to be used in C function calls. pub fn as_cstr(&self) -> *const util::c_types::c_char { - return &self.0 as *const _; + &self.0 as *const _ } /// Returns the buffer size (including null terminator). pub fn cap(&self) -> usize { - return self.0.len(); + self.0.len() } /// Returns a &str instance for use in Rust. panics if the diff --git a/src/rust/bitbox02/src/ui.rs b/src/rust/bitbox02/src/ui.rs index d78c7b075..ca7cd36fe 100644 --- a/src/rust/bitbox02/src/ui.rs +++ b/src/rust/bitbox02/src/ui.rs @@ -116,7 +116,7 @@ impl Font { impl Default for Font { fn default() -> Self { - return Font::Default; + Font::Default } } diff --git a/src/rust/bitbox02/src/util.rs b/src/rust/bitbox02/src/util.rs index da00169ef..27ffcf773 100644 --- a/src/rust/bitbox02/src/util.rs +++ b/src/rust/bitbox02/src/util.rs @@ -13,6 +13,8 @@ // limitations under the License. /// Must be given a null-terminated string +/// # Safety +/// ptr must be not NULL. pub unsafe fn strlen_ptr(ptr: *const u8) -> isize { let mut end = ptr; loop { diff --git a/src/rust/util/src/name.rs b/src/rust/util/src/name.rs index 07525c296..52bf7576c 100644 --- a/src/rust/util/src/name.rs +++ b/src/rust/util/src/name.rs @@ -16,7 +16,7 @@ /// size, consist of printable ASCII characters only (and space), not /// start or end with whitespace, and contain no whitespace other than space. pub fn validate(name: &str, max_len: usize) -> bool { - if name.len() == 0 || name.len() > max_len { + if name.is_empty() || name.len() > max_len { return false; } if !super::ascii::all_ascii(name) {