Merge branch 'clippy'

Author: benma

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

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.

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.

src/rust/bitbox02-noise/src/noise_xx.rs

@@ -93,7 +93,7 @@ impl<R: Random32> State<R> {
     /// `generate_static_private_key()`.
     pub fn init(&mut self, static_private_key: Sensitive<PrivateKey>) {
         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),

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,

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);
 }

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;

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 != '-') {

src/rust/bitbox02-rust/src/platform/bitboxbase/display.rs

@@ -27,16 +27,16 @@ pub fn write_status<W: Write>(w: &mut W, config: &Config) {
     } else {
         let _ = write!(w, "<unnamed>");
     }
-    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, "<unassigned>");
     }
-    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<Duration>) {

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 }
     }
 }
 

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 {

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

src/rust/bitbox02/src/ui.rs

@@ -116,7 +116,7 @@ impl Font {
 
 impl Default for Font {
     fn default() -> Self {
-        return Font::Default;
+        Font::Default
     }
 }
 

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 {

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) {