From d76c7a5b1e76fa6aa3aa17cf61f6c2a6bfefed2f Mon Sep 17 00:00:00 2001 From: Zhang Junyang Date: Thu, 15 Aug 2024 15:17:59 +0800 Subject: [PATCH] OSDK check and clippy with `cfg(ktest)` --- Makefile | 2 +- kernel/aster-nix/src/fs/exfat/mod.rs | 7 +++---- kernel/aster-nix/src/fs/exfat/utils.rs | 5 ++--- kernel/aster-nix/src/fs/pipe.rs | 4 ++-- kernel/aster-nix/src/process/sync/condvar.rs | 8 ++++---- kernel/aster-nix/src/taskless.rs | 6 ++++-- kernel/aster-nix/src/vm/vmar/options.rs | 2 +- kernel/libs/aster-util/src/coeff.rs | 6 +++--- osdk/src/cli.rs | 6 +++--- osdk/src/commands/mod.rs | 11 +++++++++-- osdk/test-kernel/src/tree.rs | 6 ++---- ostd/src/lib.rs | 1 + ostd/src/mm/dma/dma_stream.rs | 3 ++- ostd/src/sync/atomic_bits.rs | 16 ++++++++-------- ostd/src/sync/wait.rs | 2 +- ostd/src/task/task/mod.rs | 2 ++ 16 files changed, 48 insertions(+), 39 deletions(-) diff --git a/Makefile b/Makefile index c70452dc..88140b5e 100644 --- a/Makefile +++ b/Makefile @@ -210,7 +210,7 @@ format: @make --no-print-directory -C test format .PHONY: check -check: $(CARGO_OSDK) +check: initramfs $(CARGO_OSDK) @./tools/format_all.sh --check # Check Rust format issues @# Check if STD_CRATES and NOSTD_CRATES combined is the same as all workspace members @sed -n '/^\[workspace\]/,/^\[.*\]/{/members = \[/,/\]/p}' Cargo.toml | \ diff --git a/kernel/aster-nix/src/fs/exfat/mod.rs b/kernel/aster-nix/src/fs/exfat/mod.rs index fb2a7700..67fca56e 100644 --- a/kernel/aster-nix/src/fs/exfat/mod.rs +++ b/kernel/aster-nix/src/fs/exfat/mod.rs @@ -484,7 +484,7 @@ mod test { let mut read = vec![0u8; BUF_SIZE]; let read_after_rename = a_inode_new.read_bytes_at(0, &mut read); assert!( - read_after_rename.is_ok() && read_after_rename.clone().unwrap() == BUF_SIZE, + read_after_rename.is_ok() && read_after_rename.unwrap() == BUF_SIZE, "Fail to read after rename: {:?}", read_after_rename.unwrap_err() ); @@ -495,8 +495,7 @@ mod test { let new_buf = vec![7u8; NEW_BUF_SIZE]; let new_write_after_rename = a_inode_new.write_bytes_at(0, &new_buf); assert!( - new_write_after_rename.is_ok() - && new_write_after_rename.clone().unwrap() == NEW_BUF_SIZE, + new_write_after_rename.is_ok() && new_write_after_rename.unwrap() == NEW_BUF_SIZE, "Fail to write file after rename: {:?}", new_write_after_rename.unwrap_err() ); @@ -984,7 +983,7 @@ mod test { let mut file_names: Vec = (0..file_num).map(|x| x.to_string()).collect(); file_names.sort(); let mut file_inodes: Vec> = Vec::new(); - for (_file_id, file_name) in file_names.iter().enumerate() { + for file_name in file_names.iter() { let inode = create_file(root.clone(), file_name); file_inodes.push(inode); } diff --git a/kernel/aster-nix/src/fs/exfat/utils.rs b/kernel/aster-nix/src/fs/exfat/utils.rs index 3eb2355a..41e41979 100644 --- a/kernel/aster-nix/src/fs/exfat/utils.rs +++ b/kernel/aster-nix/src/fs/exfat/utils.rs @@ -60,7 +60,6 @@ impl DosTimestamp { #[cfg(not(ktest))] { use crate::time::clocks::RealTimeClock; - DosTimestamp::from_duration(RealTimeClock::get().read_time()) } @@ -68,9 +67,9 @@ impl DosTimestamp { #[cfg(ktest)] { use crate::time::SystemTime; - return DosTimestamp::from_duration( + DosTimestamp::from_duration( SystemTime::UNIX_EPOCH.duration_since(&SystemTime::UNIX_EPOCH)?, - ); + ) } } diff --git a/kernel/aster-nix/src/fs/pipe.rs b/kernel/aster-nix/src/fs/pipe.rs index 23404377..ee676475 100644 --- a/kernel/aster-nix/src/fs/pipe.rs +++ b/kernel/aster-nix/src/fs/pipe.rs @@ -331,7 +331,7 @@ mod test { #[ktest] fn test_read_closed() { test_blocking( - |writer| drop(writer), + drop, |reader| { let mut buf = [0; 1]; assert_eq!(reader.read(&mut writer_from(&mut buf)).unwrap(), 0); @@ -350,7 +350,7 @@ mod test { Errno::EPIPE ); }, - |reader| drop(reader), + drop, Ordering::WriteThenRead, ); } diff --git a/kernel/aster-nix/src/process/sync/condvar.rs b/kernel/aster-nix/src/process/sync/condvar.rs index 2f18bc2c..45ec27e0 100644 --- a/kernel/aster-nix/src/process/sync/condvar.rs +++ b/kernel/aster-nix/src/process/sync/condvar.rs @@ -291,7 +291,7 @@ mod test { while !*started { started = cvar.wait(started).unwrap_or_else(|err| err.into_guard()); } - assert_eq!(*started, true); + assert!(*started); } } @@ -316,7 +316,7 @@ mod test { .wait_timeout(started, Duration::from_secs(1)) .unwrap_or_else(|err| err.into_guard()); } - assert_eq!(*started, true); + assert!(*started); } } @@ -338,7 +338,7 @@ mod test { let started = cvar .wait_while(lock.lock(), |started| *started) .unwrap_or_else(|err| err.into_guard()); - assert_eq!(*started, false); + assert!(!*started); } } @@ -360,7 +360,7 @@ mod test { let (started, _) = cvar .wait_timeout_while(lock.lock(), Duration::from_secs(1), |started| *started) .unwrap_or_else(|err| err.into_guard()); - assert_eq!(*started, false); + assert!(!*started); } } } diff --git a/kernel/aster-nix/src/taskless.rs b/kernel/aster-nix/src/taskless.rs index fc3f6689..a5b8f89e 100644 --- a/kernel/aster-nix/src/taskless.rs +++ b/kernel/aster-nix/src/taskless.rs @@ -216,7 +216,7 @@ mod test { let mut counter = 0; // Schedule this taskless for `SCHEDULE_TIMES`. - while taskless.is_scheduled.load(Ordering::Acquire) == false { + while !taskless.is_scheduled.load(Ordering::Acquire) { taskless.schedule(); counter += 1; if counter == SCHEDULE_TIMES { @@ -227,7 +227,9 @@ mod test { // Wait for all taskless having finished. while taskless.is_running.load(Ordering::Acquire) || taskless.is_scheduled.load(Ordering::Acquire) - {} + { + core::hint::spin_loop() + } assert_eq!(counter, COUNTER.load(Ordering::Relaxed)); } diff --git a/kernel/aster-nix/src/vm/vmar/options.rs b/kernel/aster-nix/src/vm/vmar/options.rs index 95f9cd4d..8af45336 100644 --- a/kernel/aster-nix/src/vm/vmar/options.rs +++ b/kernel/aster-nix/src/vm/vmar/options.rs @@ -136,7 +136,7 @@ impl VmarChildOptions { #[cfg(ktest)] mod test { use aster_rights::Full; - use ostd::{mm::VmIo, prelude::*}; + use ostd::prelude::*; use super::*; use crate::vm::{ diff --git a/kernel/libs/aster-util/src/coeff.rs b/kernel/libs/aster-util/src/coeff.rs index 7cf1eed6..1a86f898 100644 --- a/kernel/libs/aster-util/src/coeff.rs +++ b/kernel/libs/aster-util/src/coeff.rs @@ -134,8 +134,8 @@ mod test { #[ktest] fn calculation() { let coeff = Coeff::new(23456, 56789, 1_000_000_000); - assert!(coeff * 0 as u64 == 0); - assert!(coeff * 100 as u64 == 100 * 23456 / 56789); - assert!(coeff * 1_000_000_000 as u64 == 1_000_000_000 * 23456 / 56789); + assert!(coeff * 0_u64 == 0); + assert!(coeff * 100_u64 == 100 * 23456 / 56789); + assert!(coeff * 1_000_000_000_u64 == 1_000_000_000 * 23456 / 56789); } } diff --git a/osdk/src/cli.rs b/osdk/src/cli.rs index 33e29b80..ba052e36 100644 --- a/osdk/src/cli.rs +++ b/osdk/src/cli.rs @@ -49,9 +49,9 @@ pub fn main() { OsdkSubcommand::Test(test_args) => { execute_test_command(&load_config(&test_args.common_args), test_args); } - OsdkSubcommand::Check(args) => execute_forwarded_command("check", &args.args), - OsdkSubcommand::Clippy(args) => execute_forwarded_command("clippy", &args.args), - OsdkSubcommand::Doc(args) => execute_forwarded_command("doc", &args.args), + OsdkSubcommand::Check(args) => execute_forwarded_command("check", &args.args, true), + OsdkSubcommand::Clippy(args) => execute_forwarded_command("clippy", &args.args, true), + OsdkSubcommand::Doc(args) => execute_forwarded_command("doc", &args.args, false), } } diff --git a/osdk/src/commands/mod.rs b/osdk/src/commands/mod.rs index 0e279b93..22f6b074 100644 --- a/osdk/src/commands/mod.rs +++ b/osdk/src/commands/mod.rs @@ -16,8 +16,10 @@ pub use self::{ use crate::arch::get_default_arch; -/// Execute the forwarded cargo command with args containing the subcommand and its arguments. -pub fn execute_forwarded_command(subcommand: &str, args: &Vec) -> ! { +/// Execute the forwarded cargo command with arguments. +/// +/// The `cfg_ktest` parameter controls whether `cfg(ktest)` is enabled. +pub fn execute_forwarded_command(subcommand: &str, args: &Vec, cfg_ktest: bool) -> ! { let mut cargo = util::cargo(); cargo.arg(subcommand).args(util::COMMON_CARGO_ARGS); if !args.contains(&"--target".to_owned()) { @@ -27,6 +29,11 @@ pub fn execute_forwarded_command(subcommand: &str, args: &Vec) -> ! { let env_rustflags = std::env::var("RUSTFLAGS").unwrap_or_default(); let rustflags = env_rustflags + " --check-cfg cfg(ktest)"; + let rustflags = if cfg_ktest { + rustflags + " --cfg ktest" + } else { + rustflags + }; cargo.env("RUSTFLAGS", rustflags); diff --git a/osdk/test-kernel/src/tree.rs b/osdk/test-kernel/src/tree.rs index 85ac5c9f..a822a776 100644 --- a/osdk/test-kernel/src/tree.rs +++ b/osdk/test-kernel/src/tree.rs @@ -221,9 +221,7 @@ mod tests { macro_rules! gen_test_case { () => {{ - fn dummy_fn() { - () - } + fn dummy_fn() {} let mut tree = KtestTree::new(); let new = |m: &'static str, f: &'static str, p: &'static str| { KtestItem::new( @@ -295,7 +293,7 @@ mod tests { for mov in crate_.iter() { let module = mov; for test in module.iter() { - collection.push(&test); + collection.push(test); } } } diff --git a/ostd/src/lib.rs b/ostd/src/lib.rs index cff2f80e..4d250977 100644 --- a/ostd/src/lib.rs +++ b/ostd/src/lib.rs @@ -119,6 +119,7 @@ mod test { use crate::prelude::*; #[ktest] + #[allow(clippy::eq_op)] fn trivial_assertion() { assert_eq!(0, 0); } diff --git a/ostd/src/mm/dma/dma_stream.rs b/ostd/src/mm/dma/dma_stream.rs index 56d9828a..00dd6f96 100644 --- a/ostd/src/mm/dma/dma_stream.rs +++ b/ostd/src/mm/dma/dma_stream.rs @@ -334,9 +334,10 @@ mod test { .alloc_contiguous() .unwrap(); let vm_segment_child = vm_segment_parent.range(0..1); - let _dma_stream_parent = + let dma_stream_parent = DmaStream::map(vm_segment_parent, DmaDirection::Bidirectional, false); let dma_stream_child = DmaStream::map(vm_segment_child, DmaDirection::Bidirectional, false); + assert!(dma_stream_parent.is_ok()); assert!(dma_stream_child.is_err()); } diff --git a/ostd/src/sync/atomic_bits.rs b/ostd/src/sync/atomic_bits.rs index 1b92799e..3ddeeac4 100644 --- a/ostd/src/sync/atomic_bits.rs +++ b/ostd/src/sync/atomic_bits.rs @@ -313,24 +313,24 @@ mod test { fn set_get() { let bits = AtomicBits::new_zeroes(128); for i in 0..bits.len() { - assert!(bits.get(i) == false); + assert!(!bits.get(i)); bits.set(i, true); - assert!(bits.get(i) == true); + assert!(bits.get(i)); bits.set(i, false); - assert!(bits.get(i) == false); + assert!(!bits.get(i)); } let bits = AtomicBits::new_ones(128); for i in 0..bits.len() { - assert!(bits.get(i) == true); + assert!(bits.get(i)); bits.set(i, false); - assert!(bits.get(i) == false); + assert!(!bits.get(i)); bits.set(i, true); - assert!(bits.get(i) == true); + assert!(bits.get(i)); } } @@ -389,9 +389,9 @@ mod test { #[ktest] fn iter() { let bits = AtomicBits::new_zeroes(7); - assert!(bits.iter().all(|bit| bit == false)); + assert!(bits.iter().all(|bit| !bit)); let bits = AtomicBits::new_ones(128); - assert!(bits.iter().all(|bit| bit == true)); + assert!(bits.iter().all(|bit| bit)); } } diff --git a/ostd/src/sync/wait.rs b/ostd/src/sync/wait.rs index 2aa60117..56ca700c 100644 --- a/ostd/src/sync/wait.rs +++ b/ostd/src/sync/wait.rs @@ -293,7 +293,7 @@ mod test { Task::yield_now(); cond_cloned.store(true, Ordering::Relaxed); - wake(&*queue_cloned); + wake(&queue_cloned); }) .data(()) .spawn() diff --git a/ostd/src/task/task/mod.rs b/ostd/src/task/task/mod.rs index 9604232a..8bf1cc93 100644 --- a/ostd/src/task/task/mod.rs +++ b/ostd/src/task/task/mod.rs @@ -383,6 +383,7 @@ mod test { #[ktest] fn create_task() { + #[allow(clippy::eq_op)] let task = || { assert_eq!(1, 1); }; @@ -395,6 +396,7 @@ mod test { #[ktest] fn spawn_task() { + #[allow(clippy::eq_op)] let task = || { assert_eq!(1, 1); };