Merge pull request #282 from rust-osdev/ci

phil-opp · web-flow · commit 3649e82cea00 · 2022-11-13T22:13:42.000+01:00
CI: Set up caching and update problem matcher
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -14,11 +14,12 @@ jobs:
     timeout-minutes: 10
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
+      - run: cargo --version --verbose
+      - uses: Swatinem/rust-cache@v2
+      - uses: r7kamura/rust-problem-matchers@v1.1.0
       - name: "Run `cargo check`"
-        uses: actions-rs/cargo@v1
-        with:
-          command: check
+        run: cargo check --all-targets --all
 
   test:
     name: Test
@@ -31,7 +32,9 @@ jobs:
     timeout-minutes: 30
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
+      - run: cargo --version --verbose
+      - uses: Swatinem/rust-cache@v2
 
       # install QEMU
       - name: Install QEMU (Linux)
@@ -53,34 +56,26 @@ jobs:
       - name: "Print QEMU Version"
         run: qemu-system-x86_64 --version
 
+      - uses: r7kamura/rust-problem-matchers@v1.1.0
       - name: Run api tests
-        uses: actions-rs/cargo@v1
-        with:
-          command: test
-          args: -p bootloader_api
-
+        run: cargo test -p bootloader_api
       - name: Run integration tests
-        uses: actions-rs/cargo@v1
-        with:
-          command: test
+        run: cargo test
 
   fmt:
     name: Check Formatting
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - name: Run `cargo fmt --all -- --check`
-        uses: actions-rs/cargo@v1
-        with:
-          command: fmt
-          args: --all -- --check
+      - uses: actions/checkout@v3
+      - uses: r7kamura/rust-problem-matchers@v1.1.0
+      - run: cargo fmt --all -- --check
 
   clippy:
     name: Clippy
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - name: Run `cargo clippy`
-        uses: actions-rs/cargo@v1
-        with:
-          command: clippy
+      - uses: actions/checkout@v3
+      - run: cargo --version --verbose
+      - uses: Swatinem/rust-cache@v2
+      - uses: r7kamura/rust-problem-matchers@v1.1.0
+      - run: cargo clippy --all --all-targets
diff --git a/api/build.rs b/api/build.rs
@@ -43,7 +43,7 @@ fn main() {
         );
     }
 
-    fs::write(&dest_path, code).unwrap();
+    fs::write(dest_path, code).unwrap();
     println!("cargo:rerun-if-changed=build.rs");
 
     let version_major: u16 = env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap();
diff --git a/api/src/config.rs b/api/src/config.rs
@@ -132,15 +132,14 @@ impl BootloaderConfig {
                 Option::Some(addr) => concat_1_8([1], addr.to_le_bytes()),
             },
         );
-        let buf = concat_106_9(
+
+        concat_106_9(
             buf,
             match minimum_framebuffer_width {
                 Option::None => [0; 9],
                 Option::Some(addr) => concat_1_8([1], addr.to_le_bytes()),
             },
-        );
-
-        buf
+        )
     }
 
     /// Tries to deserialize a config byte array that was created using [`Self::serialize`].
diff --git a/bios/boot_sector/src/main.rs b/bios/boot_sector/src/main.rs
@@ -2,7 +2,10 @@
 #![no_main]
 #![warn(unsafe_op_in_unsafe_fn)]
 
-use core::{arch::global_asm, slice};
+use core::{
+    arch::{asm, global_asm},
+    slice,
+};
 use fail::{print_char, UnwrapOrFail};
 
 global_asm!(include_str!("boot.s"));
@@ -54,7 +57,7 @@ pub extern "C" fn first_stage(disk_number: u16) {
 
         start_lba += u64::from(sectors);
         number_of_sectors -= u32::from(sectors);
-        target_addr = target_addr + u32::from(sectors) * 512;
+        target_addr += u32::from(sectors) * 512;
 
         if number_of_sectors == 0 {
             break;
@@ -73,5 +76,7 @@ pub extern "C" fn first_stage(disk_number: u16) {
         print_char(b'R');
     }
 
-    loop {}
+    loop {
+        unsafe { asm!("hlt") }
+    }
 }
diff --git a/bios/boot_sector/src/mbr.rs b/bios/boot_sector/src/mbr.rs
@@ -11,7 +11,7 @@ pub(crate) fn get_partition(partitions_raw: &[u8], index: usize) -> PartitionTab
     let offset = index * ENTRY_SIZE;
     let buffer = partitions_raw.get(offset..).unwrap_or_fail(b'c');
 
-    let bootable_raw = *buffer.get(0).unwrap_or_fail(b'd');
+    let bootable_raw = *buffer.first().unwrap_or_fail(b'd');
     let bootable = bootable_raw == 0x80;
 
     let partition_type = *buffer.get(4).unwrap_or_fail(b'e');
diff --git a/bios/common/src/lib.rs b/bios/common/src/lib.rs
@@ -62,3 +62,7 @@ pub struct E820MemoryRegion {
     pub region_type: u32,
     pub acpi_extended_attributes: u32,
 }
+
+pub fn hlt() {
+    unsafe { core::arch::asm!("hlt") };
+}
diff --git a/bios/common/src/racy_cell.rs b/bios/common/src/racy_cell.rs
@@ -7,6 +7,11 @@ impl<T> RacyCell<T> {
         Self(UnsafeCell::new(v))
     }
 
+    /// Gets a mutable pointer to the wrapped value.
+    ///
+    /// ## Safety
+    /// Ensure that the access is unique (no active references, mutable or not).
+    #[allow(clippy::mut_from_ref)]
     pub unsafe fn get_mut(&self) -> &mut T {
         unsafe { &mut *self.0.get() }
     }
diff --git a/bios/stage-2/src/disk.rs b/bios/stage-2/src/disk.rs
@@ -47,7 +47,7 @@ impl Read for DiskAccess {
 
             start_lba += u64::from(sectors);
             number_of_sectors -= u64::from(sectors);
-            target_addr = target_addr + u32::from(sectors) * 512;
+            target_addr += u32::from(sectors) * 512;
 
             if number_of_sectors == 0 {
                 break;
diff --git a/bios/stage-2/src/fat.rs b/bios/stage-2/src/fat.rs
@@ -28,7 +28,7 @@ struct Bpb {
     fat_size_16: u16,
     total_sectors_32: u32,
     fat_size_32: u32,
-    root_cluster: u32,
+    _root_cluster: u32,
 }
 
 impl Bpb {
@@ -71,7 +71,7 @@ impl Bpb {
             fat_size_16,
             total_sectors_32,
             fat_size_32,
-            root_cluster,
+            _root_cluster: root_cluster,
         }
     }
 
@@ -209,7 +209,7 @@ impl<D: Read + Seek> FileSystem<D> {
     ) -> impl Iterator<Item = Result<RawDirectoryEntry, ()>> + 'a {
         match self.bpb.fat_type() {
             FatType::Fat32 => {
-                self.bpb.root_cluster;
+                // self.bpb.root_cluster;
                 unimplemented!();
             }
             FatType::Fat12 | FatType::Fat16 => {
@@ -391,7 +391,7 @@ impl<'a> RawDirectoryEntry<'a> {
         } else {
             fn slice_to_string(slice: &[u8]) -> Result<&str, ()> {
                 const SKIP_SPACE: u8 = 0x20;
-                let mut iter = slice.into_iter().copied();
+                let mut iter = slice.iter().copied();
                 match iter.position(|c| c != SKIP_SPACE) {
                     Some(start_idx) => {
                         let end_idx =
diff --git a/bios/stage-2/src/main.rs b/bios/stage-2/src/main.rs
@@ -7,7 +7,7 @@ use crate::{
         copy_to_protected_mode, enter_protected_mode_and_jump_to_stage_3, enter_unreal_mode,
     },
 };
-use bootloader_x86_64_bios_common::{BiosFramebufferInfo, BiosInfo, Region};
+use bootloader_x86_64_bios_common::{hlt, BiosFramebufferInfo, BiosInfo, Region};
 use byteorder::{ByteOrder, LittleEndian};
 use core::{fmt::Write as _, slice};
 use disk::AlignedArrayBuffer;
@@ -50,12 +50,12 @@ fn start(disk_number: u16, partition_table_start: *const u8) -> ! {
 
         let mut entries = [PartitionTableEntry::empty(); MAX_ENTRIES];
         let raw = unsafe { slice::from_raw_parts(partition_table_start, ENTRY_SIZE * MAX_ENTRIES) };
-        for idx in 0..MAX_ENTRIES {
+        for (idx, entry) in entries.iter_mut().enumerate() {
             let offset = idx * ENTRY_SIZE;
             let partition_type = PartitionType::from_mbr_tag_byte(raw[offset + 4]);
             let lba = LittleEndian::read_u32(&raw[offset + 8..]);
             let len = LittleEndian::read_u32(&raw[offset + 12..]);
-            entries[idx] = PartitionTableEntry::new(partition_type, lba, len);
+            *entry = PartitionTableEntry::new(partition_type, lba, len);
         }
         entries
     };
@@ -146,7 +146,9 @@ fn start(disk_number: u16, partition_table_start: *const u8) -> ! {
 
     enter_protected_mode_and_jump_to_stage_3(STAGE_3_DST, &mut info);
 
-    loop {}
+    loop {
+        hlt();
+    }
 }
 
 fn load_file(
diff --git a/bios/stage-2/src/memory_map.rs b/bios/stage-2/src/memory_map.rs
@@ -47,7 +47,7 @@ pub unsafe fn query_memory_map() -> Result<&'static mut [E820MemoryRegion], ()>
         if buf_written_len != 0 {
             let buf = &buf[..buf_written_len];
 
-            let (&base_raw, rest) = split_array_ref(&buf);
+            let (&base_raw, rest) = split_array_ref(buf);
             let (&len_raw, rest) = split_array_ref(rest);
             let (&kind_raw, rest) = split_array_ref(rest);
             let acpi_extended_raw: [u8; 4] = rest.try_into().unwrap_or_default();
diff --git a/bios/stage-3/src/main.rs b/bios/stage-3/src/main.rs
@@ -3,7 +3,7 @@
 #![deny(unsafe_op_in_unsafe_fn)]
 
 use crate::screen::Writer;
-use bootloader_x86_64_bios_common::BiosInfo;
+use bootloader_x86_64_bios_common::{hlt, BiosInfo};
 use core::{arch::asm, fmt::Write as _};
 
 mod gdt;
@@ -24,7 +24,9 @@ pub extern "C" fn _start(info: &mut BiosInfo) {
     gdt::LONG_MODE_GDT.load();
     enter_long_mode_and_jump_to_stage_4(info);
 
-    loop {}
+    loop {
+        hlt();
+    }
 }
 
 #[no_mangle]
diff --git a/bios/stage-4/src/main.rs b/bios/stage-4/src/main.rs
@@ -227,7 +227,7 @@ fn detect_rsdp() -> Option<PhysAddr> {
     }
 }
 
-#[cfg(all(not(test), target_os = "none"))]
+#[cfg(target_os = "none")]
 #[panic_handler]
 fn panic(info: &core::panic::PanicInfo) -> ! {
     unsafe {
diff --git a/common/src/legacy_memory_region.rs b/common/src/legacy_memory_region.rs
@@ -11,6 +11,10 @@ pub trait LegacyMemoryRegion: Copy + core::fmt::Debug {
     fn start(&self) -> PhysAddr;
     /// Returns the size of the region in bytes.
     fn len(&self) -> u64;
+    /// Returns whether this region is empty.
+    fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
     /// Returns the type of the region, e.g. whether it is usable or reserved.
     fn kind(&self) -> MemoryRegionKind;
 
@@ -81,6 +85,11 @@ where
         self.original.len()
     }
 
+    /// Returns whether this memory map is empty.
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
     /// Returns the largest detected physical memory address.
     ///
     /// Useful for creating a mapping for all physical memory.
diff --git a/common/src/logger.rs b/common/src/logger.rs
@@ -51,7 +51,7 @@ fn get_char_raster(c: char) -> RasterizedChar {
             font_constants::CHAR_RASTER_HEIGHT,
         )
     }
-    get(c).unwrap_or(get(BACKUP_CHAR).expect("Should get raster of backup char."))
+    get(c).unwrap_or_else(|| get(BACKUP_CHAR).expect("Should get raster of backup char."))
 }
 
 impl LockedLogger {
@@ -62,6 +62,7 @@ impl LockedLogger {
 
     /// Force-unlocks the logger to prevent a deadlock.
     ///
+    /// ## Safety
     /// This method is not memory safe and should be only used when absolutely necessary.
     pub unsafe fn force_unlock(&self) {
         unsafe { self.0.force_unlock() };
diff --git a/src/fat.rs b/src/fat.rs
@@ -24,7 +24,7 @@ pub fn create_fat_filesystem(
         .write(true)
         .create(true)
         .truncate(true)
-        .open(&out_fat_path)
+        .open(out_fat_path)
         .unwrap();
     let fat_size_padded_and_rounded = ((needed_size + 1024 * 64 - 1) / MB + 1) * MB;
     fat_file.set_len(fat_size_padded_and_rounded).unwrap();
diff --git a/src/gpt.rs b/src/gpt.rs
@@ -12,11 +12,11 @@ pub fn create_gpt_disk(fat_image: &Path, out_gpt_path: &Path) -> anyhow::Result<
         .truncate(true)
         .read(true)
         .write(true)
-        .open(&out_gpt_path)
+        .open(out_gpt_path)
         .with_context(|| format!("failed to create GPT file at `{}`", out_gpt_path.display()))?;
 
     // set file size
-    let partition_size: u64 = fs::metadata(&fat_image)
+    let partition_size: u64 = fs::metadata(fat_image)
         .context("failed to read metadata of fat image")?
         .len();
     let disk_size = partition_size + 1024 * 64; // for GPT headers
@@ -61,7 +61,7 @@ pub fn create_gpt_disk(fat_image: &Path, out_gpt_path: &Path) -> anyhow::Result<
     disk.seek(io::SeekFrom::Start(start_offset))
         .context("failed to seek to start offset")?;
     io::copy(
-        &mut File::open(&fat_image).context("failed to open FAT image")?,
+        &mut File::open(fat_image).context("failed to open FAT image")?,
         &mut disk,
     )
     .context("failed to copy FAT image to GPT disk")?;
diff --git a/src/lib.rs b/src/lib.rs
@@ -127,7 +127,7 @@ impl UefiBoot {
         files.insert(KERNEL_FILE_NAME, self.kernel.as_path());
 
         let out_file = NamedTempFile::new().context("failed to create temp file")?;
-        fat::create_fat_filesystem(files, &out_file.path())
+        fat::create_fat_filesystem(files, out_file.path())
             .context("failed to create UEFI FAT filesystem")?;
 
         Ok(out_file)
diff --git a/src/mbr.rs b/src/mbr.rs
@@ -68,7 +68,7 @@ pub fn create_mbr_disk(
         .truncate(true)
         .read(true)
         .write(true)
-        .open(&out_mbr_path)
+        .open(out_mbr_path)
         .with_context(|| {
             format!(
                 "failed to create MBR disk image at `{}`",
diff --git a/tests/lto.rs b/tests/lto.rs
@@ -5,7 +5,7 @@ use bootloader_test_runner::run_test_kernel;
 #[test]
 fn basic_boot() {
     // build test kernel manually to force-enable link-time optimization
-    let mut cmd = Command::new(std::env::var("CARGO").unwrap_or("cargo".into()));
+    let mut cmd = Command::new(std::env::var("CARGO").unwrap_or_else(|_| "cargo".into()));
     cmd.arg("build");
     cmd.arg("-p").arg("test_kernel_lto");
     cmd.arg("--target").arg("x86_64-unknown-none");
diff --git a/uefi/src/main.rs b/uefi/src/main.rs
diff --git a/uefi/src/memory_descriptor.rs b/uefi/src/memory_descriptor.rs
diff --git a/x86_64-stage-4.json b/x86_64-stage-4.json

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ fn main() {`
`43`	`43`	`);`
`44`	`44`	`}`
`45`	`45`
`46`		`- fs::write(&dest_path, code).unwrap();`
	`46`	`+ fs::write(dest_path, code).unwrap();`
`47`	`47`	`println!("cargo:rerun-if-changed=build.rs");`
`48`	`48`
`49`	`49`	`let version_major: u16 = env!("CARGO_PKG_VERSION_MAJOR").parse().unwrap();`
Original file line number	Diff line number	Diff line change
`@@ -62,3 +62,7 @@ pub struct E820MemoryRegion {`
`62`	`62`	`pub region_type: u32,`
`63`	`63`	`pub acpi_extended_attributes: u32,`
`64`	`64`	`}`
	`65`	`+`
	`66`	`+pub fn hlt() {`
	`67`	`+ unsafe { core::arch::asm!("hlt") };`
	`68`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,11 @@ impl<T> RacyCell<T> {`
`7`	`7`	`Self(UnsafeCell::new(v))`
`8`	`8`	`}`
`9`	`9`
	`10`	`+ /// Gets a mutable pointer to the wrapped value.`
	`11`	`+ ///`
	`12`	`+ /// ## Safety`
	`13`	`+ /// Ensure that the access is unique (no active references, mutable or not).`
	`14`	`+ #[allow(clippy::mut_from_ref)]`
`10`	`15`	`pub unsafe fn get_mut(&self) -> &mut T {`
`11`	`16`	`unsafe { &mut *self.0.get() }`
`12`	`17`	`}`