Merge pull request #88 from Orycterope/clippy

clippy

.travis.yml

@@ -20,6 +20,7 @@ script:
 - cargo make iso-release
 - cargo make test
 - cargo make doc-full
+- cargo make clippy-ci
 
 deploy:
 - provider: script

Makefile.toml

@@ -207,3 +207,53 @@ workspace = false
 description = "Run the tests in 32bit mode"
 command = "cargo"
 args = ["test", "--target=i686-unknown-linux-gnu"]
+
+[tasks.clippy]
+workspace = false
+dependencies = ["install-rust-src"]
+install_crate = { rustup_component_name = "clippy" }
+description = "Run clippy"
+command = "cargo"
+args = ["xclippy", "--target=i386-unknown-none", "--",
+	"-A", "clippy::redundant_field_names",
+	"-A", "clippy::unreadable_literal",
+	"-A", "clippy::identity_op",
+	"-A", "clippy::zero_prefixed_literal",
+	"-W", "clippy::cast_possible_wrap",
+	"-W", "clippy::cast_sign_loss",
+	"-W", "clippy::default_trait_access",
+	"-W", "clippy::explicit_into_iter_loop",
+	"-W", "clippy::explicit_iter_loop",
+	"-W", "clippy::missing_docs_in_private_items",
+	"-W", "clippy::mut_mut",
+	"-W", "clippy::replace_consts",
+	"-W", "clippy::used_underscore_binding",
+	"-W", "clippy::wildcard_dependencies",
+	"-W", "clippy::wrong_pub_self_convention",
+	]
+
+[tasks.clippy-ci]
+workspace = false
+dependencies = ["install-rust-src"]
+install_crate = { rustup_component_name = "clippy" }
+description = "Run clippy"
+command = "cargo"
+args = ["xclippy", "--target=i386-unknown-none", "--",
+	"-A", "clippy::redundant_field_names",
+	"-A", "clippy::unreadable_literal",
+	"-A", "clippy::identity_op",
+	"-A", "clippy::zero_prefixed_literal",
+	"-W", "clippy::cast_possible_wrap",
+	"-W", "clippy::cast_sign_loss",
+	"-W", "clippy::default_trait_access",
+	"-W", "clippy::explicit_into_iter_loop",
+	"-W", "clippy::explicit_iter_loop",
+	"-W", "clippy::missing_docs_in_private_items",
+	"-W", "clippy::mut_mut",
+	"-W", "clippy::replace_consts",
+	"-W", "clippy::used_underscore_binding",
+	"-W", "clippy::wildcard_dependencies",
+	"-W", "clippy::wrong_pub_self_convention",
+	"-D", "warnings"
+	]
+

ahci/src/main.rs

@@ -1,4 +1,3 @@
-//! TODO: Write some AHCI documentation
 #![feature(alloc, const_let)]
 #![no_std]
 
@@ -11,7 +10,11 @@
 #![cfg_attr(test, allow(unused_imports))]
 
 // rustdoc warnings
-#![warn(missing_docs)] // hopefully this will soon become deny(missing_docs)
+// TODO: Write documentation for AHCI
+// BODY: Documentation lints are disabled on AHCI for now, to avoid conflicts.
+// BODY: Get @orycterope to write some doc \o/
+#![allow(missing_docs)]
+#![allow(clippy::missing_docs_in_private_items)]
 #![deny(intra_doc_link_resolution_failure)]
 
 #[macro_use]

ahci/src/pci.rs

@@ -106,6 +106,7 @@ impl PciDevice {
     ///
     /// This is done by reading the Device ID - Vendor ID register (register 0).
     /// If `0xFFFF_FFFF` is read back, this means that the device was non-existent, and we return None.
+    #[allow(clippy::absurd_extreme_comparisons)]
     fn probe(bus: u8, slot: u8, function: u8) -> Option<Self> {
         debug_assert!(bus <= MAX_BUS);
         debug_assert!(slot <= MAX_SLOT);
@@ -208,15 +209,16 @@ impl PciDevice {
 }
 
 /// Read one of the 64 32-bit registers of a pci bus>device>func.
+#[allow(clippy::absurd_extreme_comparisons)]
 fn pci_config_read_word(bus: u8, slot: u8, func: u8, register: u8) -> u32 {
     debug_assert!(bus <= MAX_BUS);
     debug_assert!(slot <= MAX_SLOT);
     debug_assert!(func <= MAX_FUNC);
     debug_assert!(register <= MAX_REGISTER);
-    let lbus = bus as u32;
-    let lslot = slot as u32;
-    let lfunc = func as u32;
-    let lregister = register as u32;
+    let lbus = u32::from(bus);
+    let lslot = u32::from(slot);
+    let lfunc = u32::from(func);
+    let lregister = u32::from(register);
     let mut ports = PCI_CONFIG_PORTS.lock();
 
     /* create the configuration address */
@@ -231,15 +233,16 @@ fn pci_config_read_word(bus: u8, slot: u8, func: u8, register: u8) -> u32 {
 }
 
 /// Read one of the 64 32-bit registers of a pci bus>device>func.
+#[allow(clippy::absurd_extreme_comparisons)]
 fn pci_config_write_word(bus: u8, slot: u8, func: u8, register: u8, value: u32) {
     debug_assert!(bus <= MAX_BUS);
     debug_assert!(slot <= MAX_SLOT);
     debug_assert!(func <= MAX_FUNC);
     debug_assert!(register <= MAX_REGISTER);
-    let lbus = bus as u32;
-    let lslot = slot as u32;
-    let lfunc = func as u32;
-    let lregister = register as u32;
+    let lbus = u32::from(bus);
+    let lslot = u32::from(slot);
+    let lfunc = u32::from(func);
+    let lregister = u32::from(register);
     let mut ports = PCI_CONFIG_PORTS.lock();
 
     /* create the configuration address */

bootstrap/src/address.rs

@@ -1,7 +1,6 @@
 //! Physical and Virtual address wrappers
 
 use core::fmt::{Formatter, Error, Display, Debug, LowerHex};
-use crate::paging::PAGE_SIZE;
 use crate::frame_alloc::{round_to_page, round_to_page_upper};
 
 /// Represents a Physical address
@@ -18,12 +17,12 @@ pub struct VirtualAddress(pub usize);
 
 impl VirtualAddress  {
     /// Gets the address as a `usize`.
-    pub fn addr(&self) -> usize { self.0 }
+    pub fn addr(self) -> usize { self.0 }
 }
 
 impl PhysicalAddress {
     /// Gets the address as a `usize`.
-    pub fn addr(&self) -> usize { self.0 }
+    pub fn addr(self) -> usize { self.0 }
 }
 
 impl ::core::ops::Add<usize> for VirtualAddress {

bootstrap/src/bootstrap_logging.rs

@@ -9,14 +9,14 @@ const COM1: u16 = 0x3F8;
 
 /// Init the rs232 COM1. Must be called before logging anything.
 pub unsafe fn init_bootstrap_log() {
-    let data_port       = COM1 + 0;
+    let _data_port      = COM1 + 0;
     let interrupt_port  = COM1 + 1;
     let baud_diviser_lo = COM1 + 0; // when DLB is set, data and intr
     let baud_diviser_hi = COM1 + 1; // become baud divisor lo and hi
     let fifo_port       = COM1 + 2;
     let lcr_port        = COM1 + 3;
-    let mcr_port        = COM1 + 4;
-    let status_port     = COM1 + 5;
+    let _mcr_port       = COM1 + 4;
+    let _status_port    = COM1 + 5;
 
     bootstrap_outb(interrupt_port , 0x00);       // Disable interrupts
     bootstrap_outb(lcr_port       , 0x80);       // Enable DLAB (set baud rate divisor)

bootstrap/src/elf_loader.rs

@@ -6,9 +6,9 @@ use core::fmt::Write;
 use core::slice;
 use xmas_elf::ElfFile;
 use xmas_elf::program::{ProgramHeader, Type::Load, SegmentData};
-use crate::paging::{PagingOffPageSet, PAGE_SIZE, UserLand, MappingType, PageTablesSet, EntryFlags};
-use crate::address::{PhysicalAddress, VirtualAddress};
-use kfs_libutils::{self, align_up};
+use crate::paging::{PagingOffPageSet, PAGE_SIZE, PageTablesSet, EntryFlags};
+use crate::address::VirtualAddress;
+use kfs_libutils::align_up;
 use crate::frame_alloc::FrameAllocator;
 
 /// Loads the kernel in high memory
@@ -27,19 +27,20 @@ pub fn load_kernel(page_table: &mut PagingOffPageSet, multiboot_info: &BootInfor
     for ph in kernel_elf.program_iter().filter(|ph|
         ph.get_type().expect("Failed to get type of elf program header") == Load)
     {
-        load_segment(page_table, &ph, &kernel_elf);
+        load_segment(page_table, ph, &kernel_elf);
     }
 
     // return the entry point
     let entry_point = kernel_elf.header.pt2.entry_point();
-    writeln!(Serial, "Entry point : {:#x?}", entry_point);
+    let _ = writeln!(Serial, "Entry point : {:#x?}", entry_point);
     entry_point as usize
 }
 
 /// Loads an elf segment by coping file_size bytes to the right address,
 /// and filling remaining with 0s.
 /// This is used by NOBITS sections (.bss), this way we initialize them to 0.
-fn load_segment(page_table: &mut PagingOffPageSet, segment: &ProgramHeader<'_>, elf_file: &ElfFile<'_>) {
+#[allow(clippy::match_bool)] // more readable
+fn load_segment(page_table: &mut PagingOffPageSet, segment: ProgramHeader<'_>, elf_file: &ElfFile<'_>) {
     // Map the segment memory
     let mem_size_total = align_up(segment.mem_size() as usize, PAGE_SIZE);
     let vaddr = segment.virtual_addr() as usize;
@@ -64,7 +65,7 @@ fn load_segment(page_table: &mut PagingOffPageSet, segment: &ProgramHeader<'_>,
         SegmentData::Undefined(elf_data) =>
         {
             let dest_ptr = phys_addr.addr() as *mut u8;
-            let mut dest = unsafe { slice::from_raw_parts_mut(dest_ptr, mem_size_total) };
+            let dest = unsafe { slice::from_raw_parts_mut(dest_ptr, mem_size_total) };
             let (dest_data, dest_pad) = dest.split_at_mut(segment.file_size() as usize);
 
             // Copy elf data
@@ -78,7 +79,7 @@ fn load_segment(page_table: &mut PagingOffPageSet, segment: &ProgramHeader<'_>,
         x => { panic ! ("Unexpected Segment data {:?}", x) }
     }
 
-    writeln!(Serial, "Loaded segment - VirtAddr {:#010x}, FileSize {:#010x}, MemSize {:#010x} {}{}{}",
+    let _ = writeln!(Serial, "Loaded segment - VirtAddr {:#010x}, FileSize {:#010x}, MemSize {:#010x} {}{}{}",
         segment.virtual_addr(), segment.file_size(), segment.mem_size(),
         match segment.flags().is_read()    { true => 'R', false => ' '},
         match segment.flags().is_write()   { true => 'W', false => ' '},

bootstrap/src/frame_alloc.rs

@@ -229,7 +229,7 @@ impl FrameAllocator {
 
     /// Panics if the frames bitmap was not initialized
     fn check_initialized(bitmap: &AllocatorBitmap) {
-        if bitmap.initialized == false {
+        if !bitmap.initialized {
             panic!("The frame allocator was not initialized");
         }
     }
@@ -243,7 +243,7 @@ impl FrameAllocator {
     fn mark_area_reserved(bitmap: &mut [u8],
                           start_addr: usize,
                           end_addr: usize) {
-        writeln!(Serial, "Setting {:#010x}..{:#010x} to reserved", round_to_page(start_addr), round_to_page_upper(end_addr));
+        let _ = writeln!(Serial, "Setting {:#010x}..{:#010x} to reserved", round_to_page(start_addr), round_to_page_upper(end_addr));
         bitmap.set_bits_area(
                 addr_to_frame(round_to_page(start_addr))
                     ..
@@ -259,7 +259,7 @@ impl FrameAllocator {
     fn mark_area_free(bitmap: &mut [u8],
                       start_addr: usize,
                       end_addr: usize) {
-        writeln!(Serial, "Setting {:#010x}..{:#010x} to available", round_to_page(start_addr), round_to_page_upper(end_addr));
+        let _ = writeln!(Serial, "Setting {:#010x}..{:#010x} to available", round_to_page(start_addr), round_to_page_upper(end_addr));
         bitmap.set_bits_area(
                 addr_to_frame(round_to_page_upper(start_addr))
                     ..

bootstrap/src/gdt/i386.rs

@@ -4,8 +4,6 @@
 #![cfg(any(target_arch = "x86", test))]
 #![allow(dead_code)]
 
-use core::fmt;
-
 pub mod instructions {
     //! Low level functions for special i386 instructions.
     pub mod tables {
@@ -214,7 +212,7 @@ impl TssStruct {
             ss2: (sp2.0).0,
             cr3: cr3 as u32,
             ldt_selector: ldt.0,
-            ..Default::default()
+            ..TssStruct::default()
         }
     }
 }

bootstrap/src/gdt/mod.rs

@@ -27,7 +27,7 @@ static GLOBAL_LDT: Once<DescriptorTable> = Once::new();
 pub fn init_gdt() {
     use self::i386::instructions::tables::{lldt, ltr};
 
-    let ldt = GLOBAL_LDT.call_once(|| DescriptorTable::new());
+    let ldt = GLOBAL_LDT.call_once(DescriptorTable::new);
 
     let gdt = GDT.call_once(|| {
         let mut gdt = DescriptorTable::new();
@@ -84,12 +84,12 @@ pub fn init_gdt() {
         gdt
     });
 
-    writeln!(super::Serial, "Loading GDT");
+    let _ = writeln!(super::Serial, "Loading GDT");
     gdt.load_global(0x8, 0x10, 0x18);
     unsafe { 
-        writeln!(super::Serial, "Loading LDT");
+        let _ = writeln!(super::Serial, "Loading LDT");
         lldt(SegmentSelector(7 << 3));
-        writeln!(super::Serial, "Loading Task");
+        let _ = writeln!(super::Serial, "Loading Task");
         ltr(SegmentSelector(8 << 3));
     }
 }
@@ -119,7 +119,7 @@ struct DescriptorTable {
 
 impl DescriptorTable {
     pub fn new() -> DescriptorTable {
-        let mut vec = ArrayVec::new();
+        let vec = ArrayVec::new();
         DescriptorTable {
             table: vec
         }
@@ -242,7 +242,7 @@ impl DescriptorTableEntry {
         Self::new_system(SystemDescriptorTypes::AvailableTss32, base as *const _ as u32, size_of::<TssStruct>() as u32, priv_level)
     }
 
-    fn get_limit(&self) -> u32 {
+    fn get_limit(self) -> u32 {
         (self.0.get_bits(0..16) as u32) | ((self.0.get_bits(48..52) << 16) as u32)
     }
 
@@ -252,15 +252,15 @@ impl DescriptorTableEntry {
         }
 
         if newlimit > 65536 {
-            newlimit = newlimit >> 12;
+            newlimit >>= 12;
             self.set_4k_granularity(true);
         }
 
         self.0.set_bits(0..16, newlimit.get_bits(0..16) as u64);
         self.0.set_bits(48..52, newlimit.get_bits(16..20) as u64);
     }
 
-    fn get_base(&self) -> u32 {
+    fn get_base(self) -> u32 {
         (self.0.get_bits(16..40) as u32) | ((self.0.get_bits(56..64) << 24) as u32)
     }
 
@@ -269,42 +269,42 @@ impl DescriptorTableEntry {
         self.0.set_bits(56..64, newbase.get_bits(24..32) as u64);
     }
 
-    pub fn get_accessed(&self) -> bool {
+    pub fn get_accessed(self) -> bool {
         self.0.get_bit(40)
     }
 
-    pub fn is_readwrite_allowed(&self) -> bool {
+    pub fn is_readwrite_allowed(self) -> bool {
         self.0.get_bit(41)
     }
 
     // TODO: also gets direction
-    pub fn is_comformant(&self) -> bool {
+    pub fn is_comformant(self) -> bool {
         self.0.get_bit(42)
     }
 
-    pub fn is_executable(&self) -> bool {
+    pub fn is_executable(self) -> bool {
         self.0.get_bit(43)
     }
 
     // bit 44 is unused
 
-    pub fn get_ring_level(&self) -> PrivilegeLevel {
+    pub fn get_ring_level(self) -> PrivilegeLevel {
         PrivilegeLevel::from_u16(self.0.get_bits(45..47) as u16)
     }
 
-    pub fn get_present(&self) -> bool {
+    pub fn get_present(self) -> bool {
         self.0.get_bit(47)
     }
 
-    pub fn is_4k_granularity(&self) -> bool {
+    pub fn is_4k_granularity(self) -> bool {
         self.0.get_bit(55)
     }
 
     fn set_4k_granularity(&mut self, is: bool) {
         self.0.set_bit(55, is);
     }
 
-    pub fn is_32bit(&self) -> bool {
+    pub fn is_32bit(self) -> bool {
         self.0.get_bit(54)
     }
 }