Merge pull request #282 from rust-osdev/ci

CI: Set up caching and update problem matcher

Author: phil-opp

Diff for: .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/[email protected]
       - 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/[email protected]
       - 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/[email protected]
+      - 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/[email protected]
+      - run: cargo clippy --all --all-targets

Diff for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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");