From 18890896853c9ff7ce335979c92031ba5645b592 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Thu, 9 Nov 2023 22:24:00 +0000 Subject: [PATCH 1/2] Improve clippy coverage --- .github/workflows/rustfmt.yml | 3 +++ src/descriptor/lang_id.rs | 2 +- src/device.rs | 48 ++++++++++++++++------------------- src/device_builder.rs | 8 +++--- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/.github/workflows/rustfmt.yml b/.github/workflows/rustfmt.yml index 80b5bf0..3ebfa79 100644 --- a/.github/workflows/rustfmt.yml +++ b/.github/workflows/rustfmt.yml @@ -16,3 +16,6 @@ jobs: components: rustfmt - run: cargo fmt --all -- --check - run: cargo clippy --all-features + - run: cargo clippy --features defmt + - run: cargo clippy --features control-buffer-256 + - run: cargo clippy diff --git a/src/descriptor/lang_id.rs b/src/descriptor/lang_id.rs index d88f89a..e56a466 100644 --- a/src/descriptor/lang_id.rs +++ b/src/descriptor/lang_id.rs @@ -349,7 +349,7 @@ pub enum LangID { MN_MONG_MN = 0x0C50, DZ_BT = 0x0C51, TMZ_MA = 0x0C5F, - QUZ_PE = 0x0C6b, + QUZ_PE = 0x0C6B, LOCALE_CUSTOM_UNSPECIFIED = 0x1000, AR_LY = 0x1001, ZH_SG = 0x1004, diff --git a/src/device.rs b/src/device.rs index 213f963..b25ae7e 100644 --- a/src/device.rs +++ b/src/device.rs @@ -606,32 +606,28 @@ impl UsbDevice<'_, B> { } }; - lang_id_list - .iter() - .fuse() - .position(|list_lang_id| match *list_lang_id { - Some(list_lang_id) if req_lang_id == list_lang_id => true, - _ => false, - }) - .or_else(|| { - // Since we construct the list of full supported lang_ids previously, - // we can safely reject requests which ask for other lang_id. - #[cfg(feature = "defmt")] - defmt::warn!( - "Receive unknown LANGID {:#06X}, reject the request", - req_lang_id - ); - None - }) - .and_then(|lang_id_list_index| { - match index { - 1 => config.manufacturer, - 2 => config.product, - 3 => config.serial_number, - _ => unreachable!(), - } - .map(|str_list| str_list[lang_id_list_index]) - }) + let position = + lang_id_list.iter().fuse().position(|list_lang_id| { + matches!(*list_lang_id, Some(list_lang_id) if req_lang_id == list_lang_id) + }); + #[cfg(feature = "defmt")] + if position.is_none() { + // Since we construct the list of full supported lang_ids previously, + // we can safely reject requests which ask for other lang_id. + defmt::warn!( + "Receive unknown LANGID {:#06X}, reject the request", + req_lang_id + ); + } + position.and_then(|lang_id_list_index| { + match index { + 1 => config.manufacturer, + 2 => config.product, + 3 => config.serial_number, + _ => unreachable!(), + } + .map(|str_list| str_list[lang_id_list_index]) + }) } else { // for other custom STRINGs diff --git a/src/device_builder.rs b/src/device_builder.rs index 8cc12eb..e2041b5 100644 --- a/src/device_builder.rs +++ b/src/device_builder.rs @@ -114,7 +114,7 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> { /// /// Default: (none) pub fn set_extra_lang_ids(mut self, extra_lang_ids: &'a [LangID]) -> Self { - if extra_lang_ids.len() == 0 { + if extra_lang_ids.is_empty() { self.config.extra_lang_ids = None; return self; } @@ -154,7 +154,7 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> { /// /// Default: (none) pub fn manufacturer(mut self, manufacturer_ls: &'a [&'a str]) -> Self { - if manufacturer_ls.len() == 0 { + if manufacturer_ls.is_empty() { self.config.manufacturer = None; return self; } @@ -184,7 +184,7 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> { /// /// Default: (none) pub fn product(mut self, product_ls: &'a [&'a str]) -> Self { - if product_ls.len() == 0 { + if product_ls.is_empty() { self.config.product = None; return self; } @@ -214,7 +214,7 @@ impl<'a, B: UsbBus> UsbDeviceBuilder<'a, B> { /// /// Default: (none) pub fn serial_number(mut self, serial_number_ls: &'a [&'a str]) -> Self { - if serial_number_ls.len() == 0 { + if serial_number_ls.is_empty() { self.config.serial_number = None; return self; } From d07713a7e019fe523a8d1ef765f6a2d9fc19f40e Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Thu, 9 Nov 2023 22:26:56 +0000 Subject: [PATCH 2/2] Change unused `.ok()` to `let _ =` `.ok()` satisfies the `must_use` without actually generating any error/panic on failure giving a false sense that the result it being checked. This changes occurrences of unused `.ok()` to `let _ = ` which explicitly shows the result is being ignored. --- src/control_pipe.rs | 6 ++-- src/device.rs | 54 +++++++++++++++++------------------ tests/test_class_host/main.rs | 2 +- 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/control_pipe.rs b/src/control_pipe.rs index ba1ba2b..f6e3958 100644 --- a/src/control_pipe.rs +++ b/src/control_pipe.rs @@ -154,12 +154,12 @@ impl ControlPipe<'_, B> { | ControlState::DataInLast | ControlState::DataInZlp | ControlState::StatusOut => { - self.ep_out.read(&mut []).ok(); + let _ = self.ep_out.read(&mut []); self.state = ControlState::Idle; } _ => { // Discard the packet - self.ep_out.read(&mut []).ok(); + let _ = self.ep_out.read(&mut []); // Unexpected OUT packet self.set_error() @@ -235,7 +235,7 @@ impl ControlPipe<'_, B> { _ => return Err(UsbError::InvalidState), }; - self.ep_in.write(&[]).ok(); + let _ = self.ep_in.write(&[]); self.state = ControlState::StatusIn; Ok(()) } diff --git a/src/device.rs b/src/device.rs index b25ae7e..06cb55b 100644 --- a/src/device.rs +++ b/src/device.rs @@ -324,13 +324,13 @@ impl UsbDevice<'_, B> { 0x0000 }; - xfer.accept_with(&status.to_le_bytes()).ok(); + let _ = xfer.accept_with(&status.to_le_bytes()); } (Recipient::Interface, Request::GET_STATUS) => { let status: u16 = 0x0000; - xfer.accept_with(&status.to_le_bytes()).ok(); + let _ = xfer.accept_with(&status.to_le_bytes()); } (Recipient::Endpoint, Request::GET_STATUS) => { @@ -342,7 +342,7 @@ impl UsbDevice<'_, B> { 0x0000 }; - xfer.accept_with(&status.to_le_bytes()).ok(); + let _ = xfer.accept_with(&status.to_le_bytes()); } (Recipient::Device, Request::GET_DESCRIPTOR) => { @@ -355,13 +355,13 @@ impl UsbDevice<'_, B> { _ => CONFIGURATION_NONE, }; - xfer.accept_with(&config.to_le_bytes()).ok(); + let _ = xfer.accept_with(&config.to_le_bytes()); } (Recipient::Interface, Request::GET_INTERFACE) => { // Reject interface numbers bigger than 255 if req.index > core::u8::MAX.into() { - xfer.reject().ok(); + let _ = xfer.reject(); return; } @@ -370,14 +370,13 @@ impl UsbDevice<'_, B> { for cls in classes { if let Some(setting) = cls.get_alt_setting(InterfaceNumber(req.index as u8)) { - xfer.accept_with(&setting.to_le_bytes()).ok(); + let _ = xfer.accept_with(&setting.to_le_bytes()); return; } } // If no class returned an alternate setting, return the default value - xfer.accept_with(&DEFAULT_ALTERNATE_SETTING.to_le_bytes()) - .ok(); + let _ = xfer.accept_with(&DEFAULT_ALTERNATE_SETTING.to_le_bytes()); } _ => (), @@ -385,7 +384,7 @@ impl UsbDevice<'_, B> { } if self.control.waiting_for_response() { - self.control.reject().ok(); + let _ = self.control.reject(); } } @@ -414,13 +413,13 @@ impl UsbDevice<'_, B> { Request::FEATURE_DEVICE_REMOTE_WAKEUP, ) => { self.remote_wakeup_enabled = false; - xfer.accept().ok(); + let _ = xfer.accept(); } (Recipient::Endpoint, Request::CLEAR_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { self.bus .set_stalled(((req.index as u8) & 0x8f).into(), false); - xfer.accept().ok(); + let _ = xfer.accept(); } ( @@ -429,13 +428,13 @@ impl UsbDevice<'_, B> { Request::FEATURE_DEVICE_REMOTE_WAKEUP, ) => { self.remote_wakeup_enabled = true; - xfer.accept().ok(); + let _ = xfer.accept(); } (Recipient::Endpoint, Request::SET_FEATURE, Request::FEATURE_ENDPOINT_HALT) => { self.bus .set_stalled(((req.index as u8) & 0x8f).into(), true); - xfer.accept().ok(); + let _ = xfer.accept(); } (Recipient::Device, Request::SET_ADDRESS, 1..=127) => { @@ -445,22 +444,22 @@ impl UsbDevice<'_, B> { } else { self.pending_address = req.value as u8; } - xfer.accept().ok(); + let _ = xfer.accept(); } (Recipient::Device, Request::SET_CONFIGURATION, CONFIGURATION_VALUE_U16) => { self.device_state = UsbDeviceState::Configured; - xfer.accept().ok(); + let _ = xfer.accept(); } (Recipient::Device, Request::SET_CONFIGURATION, CONFIGURATION_NONE_U16) => { match self.device_state { UsbDeviceState::Default => { - xfer.reject().ok(); + let _ = xfer.accept(); } _ => { self.device_state = UsbDeviceState::Addressed; - xfer.accept().ok(); + let _ = xfer.accept(); } } } @@ -468,7 +467,7 @@ impl UsbDevice<'_, B> { (Recipient::Interface, Request::SET_INTERFACE, alt_setting) => { // Reject interface numbers and alt settings bigger than 255 if req.index > core::u8::MAX.into() || alt_setting > core::u8::MAX.into() { - xfer.reject().ok(); + let _ = xfer.reject(); return; } @@ -476,28 +475,28 @@ impl UsbDevice<'_, B> { for cls in classes { if cls.set_alt_setting(InterfaceNumber(req.index as u8), alt_setting as u8) { - xfer.accept().ok(); + let _ = xfer.accept(); return; } } // Default behaviour, if no class implementation accepted the alternate setting. if alt_setting == DEFAULT_ALTERNATE_SETTING_U16 { - xfer.accept().ok(); + let _ = xfer.accept(); } else { - xfer.reject().ok(); + let _ = xfer.reject(); } } _ => { - xfer.reject().ok(); + let _ = xfer.reject(); return; } } } if self.control.waiting_for_response() { - self.control.reject().ok(); + let _ = self.control.reject(); } } @@ -510,12 +509,11 @@ impl UsbDevice<'_, B> { xfer: ControlIn, f: impl FnOnce(&mut DescriptorWriter) -> Result<()>, ) { - xfer.accept(|buf| { + let _ = xfer.accept(|buf| { let mut writer = DescriptorWriter::new(buf); f(&mut writer)?; Ok(writer.position()) - }) - .ok(); + }); } match dtype { @@ -642,13 +640,13 @@ impl UsbDevice<'_, B> { if let Some(s) = s { accept_writer(xfer, |w| w.string(s)); } else { - xfer.reject().ok(); + let _ = xfer.reject(); } } }, _ => { - xfer.reject().ok(); + let _ = xfer.reject(); } } } diff --git a/tests/test_class_host/main.rs b/tests/test_class_host/main.rs index 4d816e2..9cab6cb 100644 --- a/tests/test_class_host/main.rs +++ b/tests/test_class_host/main.rs @@ -67,7 +67,7 @@ fn run_tests(tests: &[(&str, TestFn)]) { } print!("test {} ... ", name); - stdout().flush().ok(); + let _ = stdout().flush(); let mut out = String::new();