Skip to content

Commit 44894a0

Browse files
00xcIsaacWoods
00xc
authored and
IsaacWoods
committed
aml: fix clippy warnings and run clippy in CI (rust-osdev#237)
* aml: fix all clippy warnings * 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. * aml: tests: fix all clippy warnings * 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. * ci: run clippy on acpi crate tests
1 parent 6a54af6 commit 44894a0

File tree

14 files changed

+195
-155
lines changed

14 files changed

+195
-155
lines changed

‎.github/workflows/build.yml‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,14 @@ jobs:
7979
profile: minimal
8080
components: clippy
8181

82-
- name: Run clippy
82+
- name: Run clippy (ACPI)
8383
run: cargo clippy -p acpi
84+
85+
- name: Run clippy (ACPI tests)
86+
run: cargo clippy -p acpi --tests
87+
88+
- name: Run clippy (AML)
89+
run: cargo clippy -p aml
90+
91+
- name: Run clippy (AML tests)
92+
run: cargo clippy -p aml --tests

‎aml/src/expression.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ where
199199
}
200200

201201
let mut buffer = vec![0; buffer_size];
202-
(&mut buffer[0..bytes.len()]).copy_from_slice(bytes);
202+
buffer[0..bytes.len()].copy_from_slice(bytes);
203203
(Ok(buffer), context)
204204
})
205205
}),
@@ -564,16 +564,16 @@ where
564564
context,
565565
match source {
566566
AmlValue::Buffer(bytes) => {
567-
let foo = bytes.lock();
568-
if index >= foo.len() {
567+
let guard = bytes.lock();
568+
if index >= guard.len() {
569569
Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new(vec![]))))
570-
} else if (index + length) >= foo.len() {
570+
} else if (index + length) >= guard.len() {
571571
Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new(
572-
foo[index..].to_vec(),
572+
guard[index..].to_vec(),
573573
))))
574574
} else {
575575
Ok(AmlValue::Buffer(Arc::new(spinning_top::Spinlock::new(
576-
foo[index..(index + length)].to_vec(),
576+
guard[index..(index + length)].to_vec(),
577577
))))
578578
}
579579
}

‎aml/src/lib.rs‎

Lines changed: 53 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ use alloc::{
6666
format,
6767
string::{String, ToString},
6868
};
69-
use core::mem;
69+
use core::{mem, str::FromStr};
7070
use log::{error, warn};
7171
use misc::{ArgNum, LocalNum};
7272
use name_object::Target;
@@ -171,7 +171,7 @@ impl AmlContext {
171171
format!("buf {:X?}", abbreviated)
172172
}
173173

174-
if stream.len() == 0 {
174+
if stream.is_empty() {
175175
return Err(AmlError::UnexpectedEndOfStream);
176176
}
177177

@@ -287,7 +287,7 @@ impl AmlContext {
287287
self.namespace.clone().traverse(|path, level: &NamespaceLevel| match level.typ {
288288
LevelType::Device => {
289289
let status = if level.values.contains_key(&NameSeg::from_str("_STA").unwrap()) {
290-
self.invoke_method(&AmlName::from_str("_STA").unwrap().resolve(&path)?, Args::default())?
290+
self.invoke_method(&AmlName::from_str("_STA").unwrap().resolve(path)?, Args::default())?
291291
.as_status()?
292292
} else {
293293
StatusObject::default()
@@ -298,7 +298,7 @@ impl AmlContext {
298298
*/
299299
if status.present && level.values.contains_key(&NameSeg::from_str("_INI").unwrap()) {
300300
log::info!("Invoking _INI at level: {}", path);
301-
self.invoke_method(&AmlName::from_str("_INI").unwrap().resolve(&path)?, Args::default())?;
301+
self.invoke_method(&AmlName::from_str("_INI").unwrap().resolve(path)?, Args::default())?;
302302
}
303303

304304
/*
@@ -339,7 +339,7 @@ impl AmlContext {
339339

340340
/// Get the current value of a local by its local number. Can only be executed from inside a control method.
341341
pub(crate) fn local(&self, local: LocalNum) -> Result<&AmlValue, AmlError> {
342-
if let None = self.method_context {
342+
if self.method_context.is_none() {
343343
return Err(AmlError::NotExecutingControlMethod);
344344
}
345345
if local > 7 {
@@ -384,7 +384,7 @@ impl AmlContext {
384384
}
385385

386386
Target::Arg(arg_num) => {
387-
if let None = self.method_context {
387+
if self.method_context.is_none() {
388388
return Err(AmlError::NotExecutingControlMethod);
389389
}
390390

@@ -399,7 +399,7 @@ impl AmlContext {
399399
}
400400

401401
Target::Local(local_num) => {
402-
if let None = self.method_context {
402+
if self.method_context.is_none() {
403403
return Err(AmlError::NotExecutingControlMethod);
404404
}
405405

@@ -454,49 +454,53 @@ impl AmlContext {
454454
AmlName::from_str("\\_OSI").unwrap(),
455455
AmlValue::native_method(1, false, 0, |context| {
456456
let value = context.current_arg(0)?.clone();
457-
Ok(match value.as_string(context)?.as_str() {
458-
"Windows 2000" => true, // 2000
459-
"Windows 2001" => true, // XP
460-
"Windows 2001 SP1" => true, // XP SP1
461-
"Windows 2001 SP2" => true, // XP SP2
462-
"Windows 2001.1" => true, // Server 2003
463-
"Windows 2001.1 SP1" => true, // Server 2003 SP1
464-
"Windows 2006" => true, // Vista
465-
"Windows 2006 SP1" => true, // Vista SP1
466-
"Windows 2006 SP2" => true, // Vista SP2
467-
"Windows 2006.1" => true, // Server 2008
468-
"Windows 2009" => true, // 7 and Server 2008 R2
469-
"Windows 2012" => true, // 8 and Server 2012
470-
"Windows 2013" => true, // 8.1 and Server 2012 R2
471-
"Windows 2015" => true, // 10
472-
"Windows 2016" => true, // 10 version 1607
473-
"Windows 2017" => true, // 10 version 1703
474-
"Windows 2017.2" => true, // 10 version 1709
475-
"Windows 2018" => true, // 10 version 1803
476-
"Windows 2018.2" => true, // 10 version 1809
477-
"Windows 2019" => true, // 10 version 1903
478-
479-
"Darwin" => true,
480-
481-
"Linux" => {
482-
// TODO: should we allow users to specify that this should be true? Linux has a
483-
// command line option for this.
484-
warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support.");
485-
false
486-
}
487-
488-
"Extended Address Space Descriptor" => true,
489-
// TODO: support module devices
490-
"Module Device" => false,
491-
"3.0 Thermal Model" => true,
492-
"3.0 _SCP Extensions" => true,
493-
// TODO: support processor aggregator devices
494-
"Processor Aggregator Device" => false,
457+
Ok(
458+
if match value.as_string(context)?.as_str() {
459+
"Windows 2000" => true, // 2000
460+
"Windows 2001" => true, // XP
461+
"Windows 2001 SP1" => true, // XP SP1
462+
"Windows 2001 SP2" => true, // XP SP2
463+
"Windows 2001.1" => true, // Server 2003
464+
"Windows 2001.1 SP1" => true, // Server 2003 SP1
465+
"Windows 2006" => true, // Vista
466+
"Windows 2006 SP1" => true, // Vista SP1
467+
"Windows 2006 SP2" => true, // Vista SP2
468+
"Windows 2006.1" => true, // Server 2008
469+
"Windows 2009" => true, // 7 and Server 2008 R2
470+
"Windows 2012" => true, // 8 and Server 2012
471+
"Windows 2013" => true, // 8.1 and Server 2012 R2
472+
"Windows 2015" => true, // 10
473+
"Windows 2016" => true, // 10 version 1607
474+
"Windows 2017" => true, // 10 version 1703
475+
"Windows 2017.2" => true, // 10 version 1709
476+
"Windows 2018" => true, // 10 version 1803
477+
"Windows 2018.2" => true, // 10 version 1809
478+
"Windows 2019" => true, // 10 version 1903
479+
480+
"Darwin" => true,
481+
482+
"Linux" => {
483+
// TODO: should we allow users to specify that this should be true? Linux has a
484+
// command line option for this.
485+
warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support.");
486+
false
487+
}
495488

496-
_ => false,
497-
}
498-
.then_some(AmlValue::ones())
499-
.unwrap_or(AmlValue::zero()))
489+
"Extended Address Space Descriptor" => true,
490+
// TODO: support module devices
491+
"Module Device" => false,
492+
"3.0 Thermal Model" => true,
493+
"3.0 _SCP Extensions" => true,
494+
// TODO: support processor aggregator devices
495+
"Processor Aggregator Device" => false,
496+
497+
_ => false,
498+
} {
499+
AmlValue::ones()
500+
} else {
501+
AmlValue::zero()
502+
},
503+
)
500504
}),
501505
)
502506
.unwrap();

‎aml/src/name_object.rs‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ where
101101
PREFIX_CHAR => prefix_path.parse(input, context),
102102
_ => name_path()
103103
.map(|path| {
104-
if path.len() == 0 {
104+
if path.is_empty() {
105105
return Err(Propagate::Err(AmlError::EmptyNamesAreInvalid));
106106
}
107107

@@ -175,7 +175,7 @@ pub struct NameSeg(pub(crate) [u8; 4]);
175175
impl NameSeg {
176176
pub(crate) fn from_str(string: &str) -> Result<NameSeg, AmlError> {
177177
// Each NameSeg can only have four chars, and must have at least one
178-
if string.len() < 1 || string.len() > 4 {
178+
if string.is_empty() || string.len() > 4 {
179179
return Err(AmlError::InvalidNameSeg);
180180
}
181181

@@ -234,28 +234,25 @@ where
234234
}
235235

236236
fn is_lead_name_char(byte: u8) -> bool {
237-
(byte >= b'A' && byte <= b'Z') || byte == b'_'
238-
}
239-
240-
fn is_digit_char(byte: u8) -> bool {
241-
byte >= b'0' && byte <= b'9'
237+
byte.is_ascii_uppercase() || byte == b'_'
242238
}
243239

244240
fn is_name_char(byte: u8) -> bool {
245-
is_lead_name_char(byte) || is_digit_char(byte)
241+
is_lead_name_char(byte) || byte.is_ascii_digit()
246242
}
247243

248244
#[cfg(test)]
249245
mod tests {
250246
use super::*;
251247
use crate::{parser::Parser, test_utils::*, AmlError};
248+
use core::str::FromStr;
252249

253250
#[test]
254251
fn test_name_seg() {
255252
let mut context = crate::test_utils::make_test_context();
256253

257254
check_ok!(
258-
name_seg().parse(&[b'A', b'F', b'3', b'Z'], &mut context),
255+
name_seg().parse(b"AF3Z", &mut context),
259256
NameSeg([b'A', b'F', b'3', b'Z']),
260257
&[]
261258
);
@@ -295,12 +292,12 @@ mod tests {
295292
let mut context = crate::test_utils::make_test_context();
296293

297294
check_ok!(
298-
name_string().parse(&[b'^', b'A', b'B', b'C', b'D'], &mut context),
295+
name_string().parse(b"^ABCD", &mut context),
299296
AmlName::from_str("^ABCD").unwrap(),
300297
&[]
301298
);
302299
check_ok!(
303-
name_string().parse(&[b'^', b'^', b'^', b'A', b'B', b'C', b'D'], &mut context),
300+
name_string().parse(b"^^^ABCD", &mut context),
304301
AmlName::from_str("^^^ABCD").unwrap(),
305302
&[]
306303
);

0 commit comments

Comments
 (0)