From 8213d735929f04f850e937f9f37443882ec2202b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 24 Dec 2024 02:43:00 +0100 Subject: [PATCH 1/5] aml: fix all clippy warnings --- aml/src/expression.rs | 12 ++--- aml/src/lib.rs | 102 +++++++++++++++++++++-------------------- aml/src/name_object.rs | 13 ++---- aml/src/namespace.rs | 72 +++++++++++++++-------------- aml/src/opregion.rs | 51 +++++++++++++++++---- aml/src/parser.rs | 1 + aml/src/pci_routing.rs | 5 +- aml/src/resource.rs | 2 +- aml/src/statement.rs | 2 +- aml/src/value.rs | 16 ++++--- 10 files changed, 158 insertions(+), 118 deletions(-) diff --git a/aml/src/expression.rs b/aml/src/expression.rs index 6b56ceea..5dfa1fb6 100644 --- a/aml/src/expression.rs +++ b/aml/src/expression.rs @@ -199,7 +199,7 @@ where } let mut buffer = vec![0; buffer_size]; - (&mut buffer[0..bytes.len()]).copy_from_slice(bytes); + buffer[0..bytes.len()].copy_from_slice(bytes); (Ok(buffer), context) }) }), @@ -564,16 +564,16 @@ where context, match source { AmlValue::Buffer(bytes) => { - let foo = bytes.lock(); - if index >= foo.len() { + let guard = bytes.lock(); + if index >= guard.len() { Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new(vec![])))) - } else if (index + length) >= foo.len() { + } else if (index + length) >= guard.len() { Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new( - foo[index..].to_vec(), + guard[index..].to_vec(), )))) } else { Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new( - foo[index..(index + length)].to_vec(), + guard[index..(index + length)].to_vec(), )))) } } diff --git a/aml/src/lib.rs b/aml/src/lib.rs index b0e3db8e..90027e3b 100644 --- a/aml/src/lib.rs +++ b/aml/src/lib.rs @@ -66,7 +66,7 @@ use alloc::{ format, string::{String, ToString}, }; -use core::mem; +use core::{mem, str::FromStr}; use log::{error, warn}; use misc::{ArgNum, LocalNum}; use name_object::Target; @@ -171,7 +171,7 @@ impl AmlContext { format!("buf {:X?}", abbreviated) } - if stream.len() == 0 { + if stream.is_empty() { return Err(AmlError::UnexpectedEndOfStream); } @@ -287,7 +287,7 @@ impl AmlContext { self.namespace.clone().traverse(|path, level: &NamespaceLevel| match level.typ { LevelType::Device => { let status = if level.values.contains_key(&NameSeg::from_str("_STA").unwrap()) { - self.invoke_method(&AmlName::from_str("_STA").unwrap().resolve(&path)?, Args::default())? + self.invoke_method(&AmlName::from_str("_STA").unwrap().resolve(path)?, Args::default())? .as_status()? } else { StatusObject::default() @@ -298,7 +298,7 @@ impl AmlContext { */ if status.present && level.values.contains_key(&NameSeg::from_str("_INI").unwrap()) { log::info!("Invoking _INI at level: {}", path); - self.invoke_method(&AmlName::from_str("_INI").unwrap().resolve(&path)?, Args::default())?; + self.invoke_method(&AmlName::from_str("_INI").unwrap().resolve(path)?, Args::default())?; } /* @@ -339,7 +339,7 @@ impl AmlContext { /// Get the current value of a local by its local number. Can only be executed from inside a control method. pub(crate) fn local(&self, local: LocalNum) -> Result<&AmlValue, AmlError> { - if let None = self.method_context { + if self.method_context.is_none() { return Err(AmlError::NotExecutingControlMethod); } if local > 7 { @@ -384,7 +384,7 @@ impl AmlContext { } Target::Arg(arg_num) => { - if let None = self.method_context { + if self.method_context.is_none() { return Err(AmlError::NotExecutingControlMethod); } @@ -399,7 +399,7 @@ impl AmlContext { } Target::Local(local_num) => { - if let None = self.method_context { + if self.method_context.is_none() { return Err(AmlError::NotExecutingControlMethod); } @@ -454,49 +454,53 @@ impl AmlContext { AmlName::from_str("\\_OSI").unwrap(), AmlValue::native_method(1, false, 0, |context| { let value = context.current_arg(0)?.clone(); - Ok(match value.as_string(context)?.as_str() { - "Windows 2000" => true, // 2000 - "Windows 2001" => true, // XP - "Windows 2001 SP1" => true, // XP SP1 - "Windows 2001 SP2" => true, // XP SP2 - "Windows 2001.1" => true, // Server 2003 - "Windows 2001.1 SP1" => true, // Server 2003 SP1 - "Windows 2006" => true, // Vista - "Windows 2006 SP1" => true, // Vista SP1 - "Windows 2006 SP2" => true, // Vista SP2 - "Windows 2006.1" => true, // Server 2008 - "Windows 2009" => true, // 7 and Server 2008 R2 - "Windows 2012" => true, // 8 and Server 2012 - "Windows 2013" => true, // 8.1 and Server 2012 R2 - "Windows 2015" => true, // 10 - "Windows 2016" => true, // 10 version 1607 - "Windows 2017" => true, // 10 version 1703 - "Windows 2017.2" => true, // 10 version 1709 - "Windows 2018" => true, // 10 version 1803 - "Windows 2018.2" => true, // 10 version 1809 - "Windows 2019" => true, // 10 version 1903 - - "Darwin" => true, - - "Linux" => { - // TODO: should we allow users to specify that this should be true? Linux has a - // command line option for this. - warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support."); - false - } - - "Extended Address Space Descriptor" => true, - // TODO: support module devices - "Module Device" => false, - "3.0 Thermal Model" => true, - "3.0 _SCP Extensions" => true, - // TODO: support processor aggregator devices - "Processor Aggregator Device" => false, + Ok( + if match value.as_string(context)?.as_str() { + "Windows 2000" => true, // 2000 + "Windows 2001" => true, // XP + "Windows 2001 SP1" => true, // XP SP1 + "Windows 2001 SP2" => true, // XP SP2 + "Windows 2001.1" => true, // Server 2003 + "Windows 2001.1 SP1" => true, // Server 2003 SP1 + "Windows 2006" => true, // Vista + "Windows 2006 SP1" => true, // Vista SP1 + "Windows 2006 SP2" => true, // Vista SP2 + "Windows 2006.1" => true, // Server 2008 + "Windows 2009" => true, // 7 and Server 2008 R2 + "Windows 2012" => true, // 8 and Server 2012 + "Windows 2013" => true, // 8.1 and Server 2012 R2 + "Windows 2015" => true, // 10 + "Windows 2016" => true, // 10 version 1607 + "Windows 2017" => true, // 10 version 1703 + "Windows 2017.2" => true, // 10 version 1709 + "Windows 2018" => true, // 10 version 1803 + "Windows 2018.2" => true, // 10 version 1809 + "Windows 2019" => true, // 10 version 1903 + + "Darwin" => true, + + "Linux" => { + // TODO: should we allow users to specify that this should be true? Linux has a + // command line option for this. + warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support."); + false + } - _ => false, - } - .then_some(AmlValue::ones()) - .unwrap_or(AmlValue::zero())) + "Extended Address Space Descriptor" => true, + // TODO: support module devices + "Module Device" => false, + "3.0 Thermal Model" => true, + "3.0 _SCP Extensions" => true, + // TODO: support processor aggregator devices + "Processor Aggregator Device" => false, + + _ => false, + } { + AmlValue::ones() + } else { + AmlValue::zero() + }, + ) }), ) .unwrap(); diff --git a/aml/src/name_object.rs b/aml/src/name_object.rs index 8fd9cf75..214585bb 100644 --- a/aml/src/name_object.rs +++ b/aml/src/name_object.rs @@ -101,7 +101,7 @@ where PREFIX_CHAR => prefix_path.parse(input, context), _ => name_path() .map(|path| { - if path.len() == 0 { + if path.is_empty() { return Err(Propagate::Err(AmlError::EmptyNamesAreInvalid)); } @@ -175,7 +175,7 @@ pub struct NameSeg(pub(crate) [u8; 4]); impl NameSeg { pub(crate) fn from_str(string: &str) -> Result { // Each NameSeg can only have four chars, and must have at least one - if string.len() < 1 || string.len() > 4 { + if string.is_empty() || string.len() > 4 { return Err(AmlError::InvalidNameSeg); } @@ -234,21 +234,18 @@ where } fn is_lead_name_char(byte: u8) -> bool { - (byte >= b'A' && byte <= b'Z') || byte == b'_' -} - -fn is_digit_char(byte: u8) -> bool { - byte >= b'0' && byte <= b'9' + byte.is_ascii_uppercase() || byte == b'_' } fn is_name_char(byte: u8) -> bool { - is_lead_name_char(byte) || is_digit_char(byte) + is_lead_name_char(byte) || byte.is_ascii_digit() } #[cfg(test)] mod tests { use super::*; use crate::{parser::Parser, test_utils::*, AmlError}; + use core::str::FromStr; #[test] fn test_name_seg() { diff --git a/aml/src/namespace.rs b/aml/src/namespace.rs index ecb2b2b7..7b1d3c85 100644 --- a/aml/src/namespace.rs +++ b/aml/src/namespace.rs @@ -4,7 +4,7 @@ use alloc::{ string::{String, ToString}, vec::Vec, }; -use core::fmt; +use core::{fmt, str::FromStr}; /// A handle is used to refer to an AML value without actually borrowing it until you need to /// access it (this makes borrowing situation much easier as you only have to consider who's @@ -70,6 +70,12 @@ pub struct Namespace { root: NamespaceLevel, } +impl Default for Namespace { + fn default() -> Self { + Self::new() + } +} + impl Namespace { pub fn new() -> Namespace { Namespace { @@ -103,9 +109,7 @@ impl Namespace { * If the level has already been added, we don't need to add it again. The parser can try to add it * multiple times if the ASL contains multiple blocks that add to the same scope/device. */ - if !level.children.contains_key(&last_seg) { - level.children.insert(last_seg, NamespaceLevel::new(typ)); - } + level.children.entry(last_seg).or_insert_with(|| NamespaceLevel::new(typ)); } Ok(()) @@ -268,7 +272,7 @@ impl Namespace { loop { let name = level_name.resolve(&scope)?; if let Ok((level, last_seg)) = self.get_level_for_path(&name) { - if let Some(_) = level.children.get(&last_seg) { + if level.children.contains_key(&last_seg) { return Ok(name); } } @@ -342,7 +346,7 @@ impl Namespace { where F: FnMut(&AmlName, &NamespaceLevel) -> Result, { - for (name, ref child) in level.children.iter() { + for (name, child) in level.children.iter() { let name = AmlName::from_name_seg(*name).resolve(scope)?; if f(&name, child)? { @@ -396,26 +400,11 @@ impl fmt::Debug for Namespace { } } -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] -pub struct AmlName(Vec); - -impl AmlName { - pub fn root() -> AmlName { - AmlName(alloc::vec![NameComponent::Root]) - } - - pub fn from_name_seg(seg: NameSeg) -> AmlName { - AmlName(alloc::vec![NameComponent::Segment(seg)]) - } - - pub fn from_components(components: Vec) -> AmlName { - assert!(components.len() > 0); - AmlName(components) - } +impl FromStr for AmlName { + type Err = AmlError; - /// Convert a string representation of an AML name into an `AmlName`. - pub fn from_str(mut string: &str) -> Result { - if string.len() == 0 { + fn from_str(mut string: &str) -> Result { + if string.is_empty() { return Err(AmlError::EmptyNamesAreInvalid); } @@ -427,7 +416,7 @@ impl AmlName { string = &string[1..]; } - if string.len() > 0 { + if !string.is_empty() { // Divide the rest of it into segments, and parse those for mut part in string.split('.') { // Handle prefix chars @@ -440,7 +429,25 @@ impl AmlName { } } - Ok(AmlName(components)) + Ok(Self(components)) + } +} + +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)] +pub struct AmlName(Vec); + +impl AmlName { + pub fn root() -> AmlName { + AmlName(alloc::vec![NameComponent::Root]) + } + + pub fn from_name_seg(seg: NameSeg) -> AmlName { + AmlName(alloc::vec![NameComponent::Segment(seg)]) + } + + pub fn from_components(components: Vec) -> AmlName { + assert!(!components.is_empty()); + AmlName(components) } pub fn as_string(&self) -> String { @@ -472,10 +479,7 @@ impl AmlName { return false; } - match self.0[0] { - NameComponent::Segment(_) => true, - _ => false, - } + matches!(self.0[0], NameComponent::Segment(_)) } /// Normalize an AML path, resolving prefix chars. Returns `AmlError::InvalidNormalizedName` if the path @@ -556,10 +560,10 @@ pub enum NameComponent { } impl NameComponent { - pub fn as_segment(self) -> Result { + pub fn as_segment(self) -> Option { match self { - NameComponent::Segment(seg) => Ok(seg), - NameComponent::Root | NameComponent::Prefix => Err(()), + NameComponent::Segment(seg) => Some(seg), + NameComponent::Root | NameComponent::Prefix => None, } } } diff --git a/aml/src/opregion.rs b/aml/src/opregion.rs index 533f329a..12898f92 100644 --- a/aml/src/opregion.rs +++ b/aml/src/opregion.rs @@ -6,6 +6,7 @@ use crate::{ AmlValue, }; use bit_field::BitField; +use core::str::FromStr; #[derive(Clone, Debug)] pub struct OpRegion { @@ -187,10 +188,22 @@ impl OpRegion { RegionSpace::SystemMemory => { let address = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; match length { - 8 => Ok(context.handler.write_u8(address, value as u8)), - 16 => Ok(context.handler.write_u16(address, value as u16)), - 32 => Ok(context.handler.write_u32(address, value as u32)), - 64 => Ok(context.handler.write_u64(address, value)), + 8 => { + context.handler.write_u8(address, value as u8); + Ok(()) + } + 16 => { + context.handler.write_u16(address, value as u16); + Ok(()) + } + 32 => { + context.handler.write_u32(address, value as u32); + Ok(()) + } + 64 => { + context.handler.write_u64(address, value); + Ok(()) + } _ => Err(AmlError::FieldInvalidAccessSize), } } @@ -198,9 +211,18 @@ impl OpRegion { RegionSpace::SystemIo => { let port = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; match length { - 8 => Ok(context.handler.write_io_u8(port, value as u8)), - 16 => Ok(context.handler.write_io_u16(port, value as u16)), - 32 => Ok(context.handler.write_io_u32(port, value as u32)), + 8 => { + context.handler.write_io_u8(port, value as u8); + Ok(()) + } + 16 => { + context.handler.write_io_u16(port, value as u16); + Ok(()) + } + 32 => { + context.handler.write_io_u32(port, value as u32); + Ok(()) + } _ => Err(AmlError::FieldInvalidAccessSize), } } @@ -241,9 +263,18 @@ impl OpRegion { let offset = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; match length { - 8 => Ok(context.handler.write_pci_u8(seg, bbn, device, function, offset, value as u8)), - 16 => Ok(context.handler.write_pci_u16(seg, bbn, device, function, offset, value as u16)), - 32 => Ok(context.handler.write_pci_u32(seg, bbn, device, function, offset, value as u32)), + 8 => { + context.handler.write_pci_u8(seg, bbn, device, function, offset, value as u8); + Ok(()) + } + 16 => { + context.handler.write_pci_u16(seg, bbn, device, function, offset, value as u16); + Ok(()) + } + 32 => { + context.handler.write_pci_u32(seg, bbn, device, function, offset, value as u32); + Ok(()) + } _ => Err(AmlError::FieldInvalidAccessSize), } } diff --git a/aml/src/parser.rs b/aml/src/parser.rs index 1aa5842e..1726a223 100644 --- a/aml/src/parser.rs +++ b/aml/src/parser.rs @@ -62,6 +62,7 @@ where /// parsing with `other`, returning the result of that parser in all cases. Other errors from the first /// parser are propagated without attempting the second parser. To chain more than two parsers using /// `or`, see the `choice!` macro. + #[allow(unused)] fn or(self, other: OtherParser) -> Or<'a, 'c, Self, OtherParser, R> where OtherParser: Parser<'a, 'c, R>, diff --git a/aml/src/pci_routing.rs b/aml/src/pci_routing.rs index 247ea94a..7acf9264 100644 --- a/aml/src/pci_routing.rs +++ b/aml/src/pci_routing.rs @@ -9,6 +9,7 @@ use crate::{ }; use alloc::vec::Vec; use bit_field::BitField; +use core::str::FromStr; pub use crate::resource::IrqDescriptor; @@ -60,7 +61,7 @@ impl PciRoutingTable { pub fn from_prt_path(prt_path: &AmlName, context: &mut AmlContext) -> Result { let mut entries = Vec::new(); - let prt = context.invoke_method(&prt_path, Args::default())?; + let prt = context.invoke_method(prt_path, Args::default())?; if let AmlValue::Package(ref inner_values) = prt { for value in inner_values { if let AmlValue::Package(ref pin_package) = value { @@ -118,7 +119,7 @@ impl PciRoutingTable { } AmlValue::String(ref name) => { let link_object_name = - context.namespace.search_for_level(&AmlName::from_str(name)?, &prt_path)?; + context.namespace.search_for_level(&AmlName::from_str(name)?, prt_path)?; entries.push(PciRoute { device, function, diff --git a/aml/src/resource.rs b/aml/src/resource.rs index 834484d0..abe907ae 100644 --- a/aml/src/resource.rs +++ b/aml/src/resource.rs @@ -25,7 +25,7 @@ pub fn resource_descriptor_list(descriptor: &AmlValue) -> Result, let buffer_data = bytes.lock(); let mut bytes = buffer_data.as_slice(); - while bytes.len() > 0 { + while !bytes.is_empty() { let (descriptor, remaining_bytes) = resource_descriptor(bytes)?; if let Some(descriptor) = descriptor { diff --git a/aml/src/statement.rs b/aml/src/statement.rs index a24e78ad..3687a686 100644 --- a/aml/src/statement.rs +++ b/aml/src/statement.rs @@ -153,7 +153,7 @@ where .then(comment_scope( DebugVerbosity::AllScopes, "DefElse", - pkg_length().feed(|length| take_to_end_of_pkglength(length)), + pkg_length().feed(take_to_end_of_pkglength), )) .map(|((), else_branch): ((), &[u8])| Ok(else_branch)), // TODO: can this be `id().map(&[])`? diff --git a/aml/src/value.rs b/aml/src/value.rs index 363f5804..4da19842 100644 --- a/aml/src/value.rs +++ b/aml/src/value.rs @@ -142,10 +142,12 @@ pub enum AmlType { ThermalZone, } +type NativeMethod = Arc Result + Send + Sync>; + #[derive(Clone)] pub enum MethodCode { Aml(Vec), - Native(Arc Result + Send + Sync>), + Native(NativeMethod), } impl fmt::Debug for MethodCode { @@ -214,7 +216,7 @@ impl AmlValue { } pub fn ones() -> AmlValue { - AmlValue::Integer(u64::max_value()) + AmlValue::Integer(u64::MAX) } pub fn native_method(arg_count: u8, serialize: bool, sync_level: u8, f: F) -> AmlValue @@ -264,7 +266,7 @@ impl AmlValue { match self { AmlValue::Boolean(value) => Ok(*value), AmlValue::Integer(value) => Ok(*value != 0), - AmlValue::Field{ .. } => Ok(self.as_integer(context)? != 0), + AmlValue::Field { .. } => Ok(self.as_integer(context)? != 0), _ => Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::Integer }), } } @@ -272,7 +274,7 @@ impl AmlValue { pub fn as_integer(&self, context: &mut AmlContext) -> Result { match self { AmlValue::Integer(value) => Ok(*value), - AmlValue::Boolean(value) => Ok(if *value { u64::max_value() } else { 0 }), + AmlValue::Boolean(value) => Ok(if *value { u64::MAX } else { 0 }), AmlValue::Buffer(ref bytes) => { /* * "The first 8 bytes of the buffer are converted to an integer, taking the first @@ -393,8 +395,8 @@ impl AmlValue { // TODO: implement all of the rules match desired_type { - AmlType::Integer => self.as_integer(context).map(|value| AmlValue::Integer(value)), - AmlType::Buffer => self.as_buffer(context).map(|value| AmlValue::Buffer(value)), + AmlType::Integer => self.as_integer(context).map(AmlValue::Integer), + AmlType::Buffer => self.as_buffer(context).map(AmlValue::Buffer), AmlType::FieldUnit => panic!( "Can't implicitly convert to FieldUnit. This must be special-cased by the caller for now :(" ), @@ -462,7 +464,7 @@ impl AmlValue { let mut lower = 0u32; lower.view_bits_mut::()[0..32].clone_from_bitslice(bits); upper.view_bits_mut::()[0..(length - 32)].clone_from_bitslice(&bits[32..]); - Ok(AmlValue::Integer((upper as u64) << 32 + (lower as u64))) + Ok(AmlValue::Integer((upper as u64) << (32 + (lower as u64)))) } else { let mut value = 0u32; value.view_bits_mut::()[0..length].clone_from_bitslice(bits); From 5e4c1508f12e23561a9049c4a7e7b1d6dc5c839e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 24 Dec 2024 02:45:58 +0100 Subject: [PATCH 2/5] ci: run clippy on aml crate Now that all warnings have been fixed for the aml crate, run clippy as part of CI. Once the warnings for the rest of the crates are fixed, they can be included in the CI run. --- .github/workflows/build.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b68e565b..9e11bb9b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -79,5 +79,8 @@ jobs: profile: minimal components: clippy - - name: Run clippy + - name: Run clippy (ACPI) run: cargo clippy -p acpi + + - name: Run clippy (AML) + run: cargo clippy -p aml From 8842310eb792fa5b238058f16b46820243eac8a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 24 Dec 2024 03:01:30 +0100 Subject: [PATCH 3/5] aml: tests: fix all clippy warnings --- aml/src/name_object.rs | 6 +++--- aml/src/namespace.rs | 36 ++++++++++++++++++------------------ aml/src/pkg_length.rs | 2 +- aml/src/term_object.rs | 2 +- aml/src/test_utils.rs | 17 ++++------------- 5 files changed, 27 insertions(+), 36 deletions(-) diff --git a/aml/src/name_object.rs b/aml/src/name_object.rs index 214585bb..4c51a496 100644 --- a/aml/src/name_object.rs +++ b/aml/src/name_object.rs @@ -252,7 +252,7 @@ mod tests { let mut context = crate::test_utils::make_test_context(); check_ok!( - name_seg().parse(&[b'A', b'F', b'3', b'Z'], &mut context), + name_seg().parse(b"AF3Z", &mut context), NameSeg([b'A', b'F', b'3', b'Z']), &[] ); @@ -292,12 +292,12 @@ mod tests { let mut context = crate::test_utils::make_test_context(); check_ok!( - name_string().parse(&[b'^', b'A', b'B', b'C', b'D'], &mut context), + name_string().parse(b"^ABCD", &mut context), AmlName::from_str("^ABCD").unwrap(), &[] ); check_ok!( - name_string().parse(&[b'^', b'^', b'^', b'A', b'B', b'C', b'D'], &mut context), + name_string().parse(b"^^^ABCD", &mut context), AmlName::from_str("^^^ABCD").unwrap(), &[] ); diff --git a/aml/src/namespace.rs b/aml/src/namespace.rs index 7b1d3c85..8f196fcc 100644 --- a/aml/src/namespace.rs +++ b/aml/src/namespace.rs @@ -600,12 +600,12 @@ mod tests { #[test] fn test_is_normal() { - assert_eq!(AmlName::root().is_normal(), true); - assert_eq!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_normal(), true); - assert_eq!(AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_normal(), false); - assert_eq!(AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_normal(), false); - assert_eq!(AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_normal(), false); - assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().is_normal(), true); + assert!(AmlName::root().is_normal()); + assert!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_normal()); + assert!(!AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_normal()); + assert!(!AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_normal()); + assert!(!AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_normal()); + assert!(AmlName::from_str("_SB.PCI0.VGA").unwrap().is_normal()); } #[test] @@ -638,22 +638,22 @@ mod tests { #[test] fn test_is_absolute() { - assert_eq!(AmlName::root().is_absolute(), true); - assert_eq!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_absolute(), true); - assert_eq!(AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_absolute(), true); - assert_eq!(AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_absolute(), true); - assert_eq!(AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_absolute(), false); - assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().is_absolute(), false); + assert!(AmlName::root().is_absolute()); + assert!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_absolute()); + assert!(AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_absolute()); + assert!(AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_absolute()); + assert!(!AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_absolute()); + assert!(!AmlName::from_str("_SB.PCI0.VGA").unwrap().is_absolute()); } #[test] fn test_search_rules_apply() { - assert_eq!(AmlName::root().search_rules_apply(), false); - assert_eq!(AmlName::from_str("\\_SB").unwrap().search_rules_apply(), false); - assert_eq!(AmlName::from_str("^VGA").unwrap().search_rules_apply(), false); - assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().search_rules_apply(), false); - assert_eq!(AmlName::from_str("VGA").unwrap().search_rules_apply(), true); - assert_eq!(AmlName::from_str("_SB").unwrap().search_rules_apply(), true); + assert!(!AmlName::root().search_rules_apply()); + assert!(!AmlName::from_str("\\_SB").unwrap().search_rules_apply()); + assert!(!AmlName::from_str("^VGA").unwrap().search_rules_apply()); + assert!(!AmlName::from_str("_SB.PCI0.VGA").unwrap().search_rules_apply()); + assert!(AmlName::from_str("VGA").unwrap().search_rules_apply()); + assert!(AmlName::from_str("_SB").unwrap().search_rules_apply()); } #[test] diff --git a/aml/src/pkg_length.rs b/aml/src/pkg_length.rs index 214d1253..f3b03a5d 100644 --- a/aml/src/pkg_length.rs +++ b/aml/src/pkg_length.rs @@ -189,7 +189,7 @@ mod tests { check_ok!( pkg_length() - .feed(|length| crate::parser::take_to_end_of_pkglength(length)) + .feed(crate::parser::take_to_end_of_pkglength) .parse(&[0x05, 0x01, 0x02, 0x03, 0x04, 0xff, 0xff, 0xff], &mut context), &[0x01, 0x02, 0x03, 0x04], &[0xff, 0xff, 0xff] diff --git a/aml/src/term_object.rs b/aml/src/term_object.rs index 3859e81f..4ddb14f0 100644 --- a/aml/src/term_object.rs +++ b/aml/src/term_object.rs @@ -1056,7 +1056,7 @@ mod test { ); check_ok_value!( computational_data().parse(&[0xff, 0x98, 0xc3], &mut context), - AmlValue::Integer(u64::max_value()), + AmlValue::Integer(u64::MAX), &[0x98, 0xc3] ); check_ok_value!( diff --git a/aml/src/test_utils.rs b/aml/src/test_utils.rs index ff07a0e1..0664ca44 100644 --- a/aml/src/test_utils.rs +++ b/aml/src/test_utils.rs @@ -125,10 +125,7 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { use crate::value::MethodCode; match a { - AmlValue::Uninitialized => match b { - AmlValue::Uninitialized => true, - _ => false, - }, + AmlValue::Uninitialized => matches!(b, AmlValue::Uninitialized), AmlValue::Boolean(a) => match b { AmlValue::Boolean(b) => a == b, _ => false, @@ -151,10 +148,7 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { } _ => false, }, - AmlValue::Device => match b { - AmlValue::Device => true, - _ => false, - }, + AmlValue::Device => matches!(b, AmlValue::Device), AmlValue::Method { flags, code } => match b { AmlValue::Method { flags: b_flags, code: b_code } => { if flags != b_flags { @@ -195,7 +189,7 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { AmlValue::Package(a) => match b { AmlValue::Package(b) => { for (a, b) in a.iter().zip(b) { - if crudely_cmp_values(a, b) == false { + if !crudely_cmp_values(a, b) { return false; } } @@ -210,9 +204,6 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { } _ => false, }, - AmlValue::ThermalZone => match b { - AmlValue::ThermalZone => true, - _ => false, - }, + AmlValue::ThermalZone => matches!(b, AmlValue::ThermalZone), } } From c7d209d417e2527a34c9122ce1809ae19411c660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 24 Dec 2024 03:02:44 +0100 Subject: [PATCH 4/5] ci: run clippy on aml crate tests Now that all warnings have been fixed for the aml crate tests, run clippy as part of CI. --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9e11bb9b..67280c68 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -84,3 +84,6 @@ jobs: - name: Run clippy (AML) run: cargo clippy -p aml + + - name: Run clippy (AML tests) + run: cargo clippy -p aml --tests From 6f6bb3c9197ee8d96ed83191cca157a21ba2dc81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Tue, 24 Dec 2024 03:06:20 +0100 Subject: [PATCH 5/5] ci: run clippy on acpi crate tests --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 67280c68..d20a65fb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -82,6 +82,9 @@ jobs: - name: Run clippy (ACPI) run: cargo clippy -p acpi + - name: Run clippy (ACPI tests) + run: cargo clippy -p acpi --tests + - name: Run clippy (AML) run: cargo clippy -p aml