Merge pull request #132 from rust-osdev/clippy-ci

Resolve remaining clippy warnings and add clippy job to CI

Author: phil-opp

.github/workflows/build.yml

@@ -139,3 +139,12 @@ jobs:
     - uses: actions/checkout@v1
     - run: rustup install nightly
     - run: cargo +nightly fmt -- --check
+
+  clippy:
+    name: "Clippy"
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+    - uses: actions/checkout@v1
+    - run: rustup install nightly
+    - run: cargo +nightly clippy -- -D warnings

src/addr.rs

@@ -38,6 +38,7 @@ pub struct PhysAddr(u64);
 #[derive(Debug)]
 pub struct VirtAddrNotValid(u64);
 
+#[allow(clippy::trivially_copy_pass_by_ref)]
 impl VirtAddr {
     /// Creates a new canonical virtual address.
     ///
@@ -288,6 +289,7 @@ impl PhysAddr {
     }
 
     /// Convenience method for checking if a physical address is null.
+    #[allow(clippy::trivially_copy_pass_by_ref)]
     #[inline]
     pub fn is_null(&self) -> bool {
         self.0 == 0
@@ -459,15 +461,15 @@ mod tests {
         // align 1
         assert_eq!(align_up(0, 1), 0);
         assert_eq!(align_up(1234, 1), 1234);
-        assert_eq!(align_up(0xffffffffffffffff, 1), 0xffffffffffffffff);
+        assert_eq!(align_up(0xffff_ffff_ffff_ffff, 1), 0xffff_ffff_ffff_ffff);
         // align 2
         assert_eq!(align_up(0, 2), 0);
         assert_eq!(align_up(1233, 2), 1234);
-        assert_eq!(align_up(0xfffffffffffffffe, 2), 0xfffffffffffffffe);
+        assert_eq!(align_up(0xffff_ffff_ffff_fffe, 2), 0xffff_ffff_ffff_fffe);
         // address 0
         assert_eq!(align_up(0, 128), 0);
         assert_eq!(align_up(0, 1), 0);
         assert_eq!(align_up(0, 2), 0);
-        assert_eq!(align_up(0, 0x8000000000000000), 0);
+        assert_eq!(align_up(0, 0x8000_0000_0000_0000), 0);
     }
 }

src/instructions/port.rs

@@ -134,6 +134,8 @@ impl<T: PortRead> PortReadOnly<T> {
 
     /// Reads from the port.
     ///
+    /// ## Safety
+    ///
     /// This function is unsafe because the I/O port could have side effects that violate memory
     /// safety.
     #[inline]
@@ -163,6 +165,8 @@ impl<T: PortWrite> PortWriteOnly<T> {
 
     /// Writes to the port.
     ///
+    /// ## Safety
+    ///
     /// This function is unsafe because the I/O port could have side effects that violate memory
     /// safety.
     #[inline]
@@ -192,6 +196,8 @@ impl<T: PortReadWrite> Port<T> {
 
     /// Reads from the port.
     ///
+    /// ## Safety
+    ///
     /// This function is unsafe because the I/O port could have side effects that violate memory
     /// safety.
     #[inline]
@@ -201,6 +207,8 @@ impl<T: PortReadWrite> Port<T> {
 
     /// Writes to the port.
     ///
+    /// ## Safety
+    ///
     /// This function is unsafe because the I/O port could have side effects that violate memory
     /// safety.
     #[inline]

src/instructions/random.rs

@@ -5,6 +5,7 @@
 pub struct RdRand(());
 
 #[cfg(target_arch = "x86_64")]
+#[allow(clippy::trivially_copy_pass_by_ref)]
 impl RdRand {
     /// Creates Some(RdRand) if RDRAND is supported, None otherwise
     pub fn new() -> Option<Self> {

src/instructions/segmentation.rs

@@ -8,6 +8,11 @@ use crate::structures::gdt::SegmentSelector;
 /// to %cs. Instead we push the new segment selector
 /// and return value on the stack and use lretq
 /// to reload cs and continue at 1:.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid code segment descriptor.
 pub unsafe fn set_cs(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
     #[inline(always)]
@@ -29,6 +34,11 @@ pub unsafe fn set_cs(sel: SegmentSelector) {
 }
 
 /// Reload stack segment register.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid stack segment descriptor.
 #[inline]
 pub unsafe fn load_ss(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
@@ -39,6 +49,11 @@ pub unsafe fn load_ss(sel: SegmentSelector) {
 }
 
 /// Reload data segment register.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid data segment descriptor.
 #[inline]
 pub unsafe fn load_ds(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
@@ -49,6 +64,11 @@ pub unsafe fn load_ds(sel: SegmentSelector) {
 }
 
 /// Reload es segment register.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid extra segment descriptor.
 #[inline]
 pub unsafe fn load_es(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
@@ -59,6 +79,11 @@ pub unsafe fn load_es(sel: SegmentSelector) {
 }
 
 /// Reload fs segment register.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid fs segment descriptor.
 #[inline]
 pub unsafe fn load_fs(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
@@ -69,6 +94,11 @@ pub unsafe fn load_fs(sel: SegmentSelector) {
 }
 
 /// Reload gs segment register.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that `sel`
+/// is a valid gs segment descriptor.
 #[inline]
 pub unsafe fn load_gs(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]
@@ -79,6 +109,11 @@ pub unsafe fn load_gs(sel: SegmentSelector) {
 }
 
 /// Swap `KernelGsBase` MSR and `GsBase` MSR.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that the
+/// swap operation cannot lead to undefined behavior.
 #[inline]
 pub unsafe fn swap_gs() {
     #[cfg(feature = "inline_asm")]

src/instructions/tables.rs

@@ -4,9 +4,17 @@ use crate::structures::gdt::SegmentSelector;
 
 pub use crate::structures::DescriptorTablePointer;
 
-/// Load a GDT. Use the
+/// Load a GDT.
+///
+/// Use the
 /// [`GlobalDescriptorTable`](crate::structures::gdt::GlobalDescriptorTable) struct for a high-level
 /// interface to loading a GDT.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that the given
+/// `DescriptorTablePointer` points to a valid GDT and that loading this
+/// GDT is safe.
 #[inline]
 pub unsafe fn lgdt(gdt: &DescriptorTablePointer) {
     #[cfg(feature = "inline_asm")]
@@ -16,9 +24,17 @@ pub unsafe fn lgdt(gdt: &DescriptorTablePointer) {
     crate::asm::x86_64_asm_lgdt(gdt as *const _);
 }
 
-/// Load an IDT. Use the
+/// Load an IDT.
+///
+/// Use the
 /// [`InterruptDescriptorTable`](crate::structures::idt::InterruptDescriptorTable) struct for a high-level
 /// interface to loading an IDT.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that the given
+/// `DescriptorTablePointer` points to a valid IDT and that loading this
+/// IDT is safe.
 #[inline]
 pub unsafe fn lidt(idt: &DescriptorTablePointer) {
     #[cfg(feature = "inline_asm")]
@@ -29,6 +45,12 @@ pub unsafe fn lidt(idt: &DescriptorTablePointer) {
 }
 
 /// Load the task state register using the `ltr` instruction.
+///
+/// ## Safety
+///
+/// This function is unsafe because the caller must ensure that the given
+/// `SegmentSelector` points to a valid TSS entry in the GDT and that loading
+/// this TSS is safe.
 #[inline]
 pub unsafe fn load_tss(sel: SegmentSelector) {
     #[cfg(feature = "inline_asm")]

src/registers/control.rs

@@ -158,8 +158,12 @@ mod x86_64 {
 
         /// Write CR0 flags.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. disabling paging.
+        /// Preserves the value of reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by disabling paging.
         #[inline]
         pub unsafe fn write(flags: Cr0Flags) {
             let old_value = Self::read_raw();
@@ -171,8 +175,12 @@ mod x86_64 {
 
         /// Write raw CR0 flags.
         ///
-        /// Does _not_ preserve any values, including reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. disabling paging.
+        /// Does _not_ preserve any values, including reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by disabling paging.
         #[inline]
         pub unsafe fn write_raw(value: u64) {
             #[cfg(feature = "inline_asm")]
@@ -184,8 +192,12 @@ mod x86_64 {
 
         /// Updates CR0 flags.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. disabling paging.
+        /// Preserves the value of reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by disabling paging.
         #[inline]
         pub unsafe fn update<F>(f: F)
         where
@@ -283,8 +295,13 @@ mod x86_64 {
 
         /// Write CR4 flags.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. physical address extension.
+        /// Preserves the value of reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by overwriting the physical address extension
+        /// flag.
         #[inline]
         pub unsafe fn write(flags: Cr4Flags) {
             let old_value = Self::read_raw();
@@ -296,8 +313,13 @@ mod x86_64 {
 
         /// Write raw CR4 flags.
         ///
-        /// Does _not_ preserve any values, including reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. physical address extension.
+        /// Does _not_ preserve any values, including reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by overwriting the physical address extension
+        /// flag.
         #[inline]
         pub unsafe fn write_raw(value: u64) {
             #[cfg(feature = "inline_asm")]
@@ -309,8 +331,12 @@ mod x86_64 {
 
         /// Updates CR4 flags.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to violate memory
-        /// safety by e.g. physical address extension.
+        /// Preserves the value of reserved fields.
+        /// ## Safety
+        ///
+        /// This function is unsafe because it's possible to violate memory
+        /// safety through it, e.g. by overwriting the physical address extension
+        /// flag.
         #[inline]
         pub unsafe fn update<F>(f: F)
         where

src/registers/model_specific.rs

@@ -110,6 +110,11 @@ mod x86_64 {
 
     impl Msr {
         /// Read 64 bits msr register.
+        ///
+        /// ## Safety
+        ///
+        /// The caller must ensure that this read operation has no unsafe side
+        /// effects.
         #[inline]
         pub unsafe fn read(&self) -> u64 {
             #[cfg(feature = "inline_asm")]
@@ -124,6 +129,11 @@ mod x86_64 {
         }
 
         /// Write 64 bits to msr register.
+        ///
+        /// ## Safety
+        ///
+        /// The caller must ensure that this write operation has no unsafe side
+        /// effects.
         #[inline]
         pub unsafe fn write(&mut self, value: u64) {
             #[cfg(feature = "inline_asm")]
@@ -153,8 +163,12 @@ mod x86_64 {
 
         /// Write the EFER flags, preserving reserved values.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to break memory
-        /// safety, e.g. by disabling long mode.
+        /// Preserves the value of reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// Unsafe because it's possible to break memory
+        /// safety with wrong flags, e.g. by disabling long mode.
         #[inline]
         pub unsafe fn write(flags: EferFlags) {
             let old_value = Self::read_raw();
@@ -166,17 +180,25 @@ mod x86_64 {
 
         /// Write the EFER flags.
         ///
-        /// Does not preserve any bits, including reserved fields. Unsafe because it's possible to
-        /// break memory safety, e.g. by disabling long mode.
+        /// Does not preserve any bits, including reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// Unsafe because it's possible to
+        /// break memory safety with wrong flags, e.g. by disabling long mode.
         #[inline]
         pub unsafe fn write_raw(flags: u64) {
             Self::MSR.write(flags);
         }
 
         /// Update EFER flags.
         ///
-        /// Preserves the value of reserved fields. Unsafe because it's possible to break memory
-        /// safety, e.g. by disabling long mode.
+        /// Preserves the value of reserved fields.
+        ///
+        /// ## Safety
+        ///
+        /// Unsafe because it's possible to break memory
+        /// safety with wrong flags, e.g. by disabling long mode.
         #[inline]
         pub unsafe fn update<F>(f: F)
         where
@@ -284,7 +306,8 @@ mod x86_64 {
         ///  this field + 8. Because SYSCALL always switches to CPL 0, the RPL bits
         /// 33:32 should be initialized to 00b.
         ///
-        /// # Unsafety
+        /// # Safety
+        ///
         /// Unsafe because this can cause system instability if passed in the
         /// wrong values for the fields.
         #[inline]