Merge pull request #3 from nihilistkitten/fix-clippy

Fix clippy and rustfmt errors - thanks Riley!

.github/workflows/lints.yml

@@ -0,0 +1,54 @@
+name: lints
+
+on:
+  pull_request:
+  push:
+    branches:
+      - "*"
+
+jobs:
+  clippy:
+    name: clippy
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout source
+        uses: actions/checkout@v2
+
+      - name: install nightly toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          override: true
+          components: rustfmt, clippy
+
+      - name: run cargo fmt
+        uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+
+      - name: run cargo clippy
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+
+  docs:
+    name: docs
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout source
+        uses: actions/checkout@v2
+
+      - name: install nightly toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          override: true
+
+      - name: run cargo doc
+        uses: actions-rs/cargo@v1
+        with:
+          command: doc
+          args: --all --no-deps

src/address_space.rs

@@ -8,11 +8,13 @@ type VirtualAddress = usize;
 struct MapEntry {
     source: Arc<dyn DataSource>,
     offset: usize,
-    span:   usize,
+    span: usize,
 }
+
+/// An address space.
 pub struct AddressSpace {
     name: String,
-    mappings: LinkedList<MapEntry>,  // see below for comments
+    mappings: LinkedList<MapEntry>, // see below for comments
 }
 
 // comments about storing mappings
@@ -26,27 +28,49 @@ pub struct AddressSpace {
 // from a crate (but remember it needs to be #no_std compatible), or even write your own.
 
 impl AddressSpace {
+    #[must_use]
     pub fn new(name: &str) -> Self {
-        AddressSpace {
+        Self {
             name: name.to_string(),
-            mappings : LinkedList::new(),
+            mappings: LinkedList::new(),
         }
     }
 
-    // add a mapping from DataSource into this AddressSpace
-    // return VirtualAddress, or an error
-    pub fn add_mapping(&self, source: &dyn DataSource, offset: usize, span: usize) -> Result<VirtualAddress, &str> {
-        panic!("add mapping not yet implemented!");
+    /// Add a mapping from a `DataSource` into this `AddressSpace`.
+    ///
+    /// # Errors
+    /// If the desired mapping is invalid.
+    pub fn add_mapping(
+        &self,
+        source: &dyn DataSource,
+        offset: usize,
+        span: usize,
+    ) -> Result<VirtualAddress, &str> {
+        todo!()
     }
 
-    // add a mapping from DataSource into this AddressSpace starting at start
-    // returns Ok(), or an error if start + span doesn't have room for this mapping
-    pub fn add_mapping_at(&self, source: &dyn DataSource, offset: usize, span: usize, start: VirtualAddress) -> Result<(), &str> {
-        panic!("add mapping not yet implemented!");
+    /// Add a mapping from `DataSource` into this `AddressSpace` starting at a specific address.
+    ///
+    /// # Errors
+    /// If there is insufficient room subsequent to `start`.
+    pub fn add_mapping_at(
+        &self,
+        source: &dyn DataSource,
+        offset: usize,
+        span: usize,
+        start: VirtualAddress,
+    ) -> Result<(), &str> {
+        todo!()
     }
 
-    // remove the mapping to DataSource that starts at VirtualAddress
-    pub fn remove_mapping(&self, source: &dyn DataSource, start: VirtualAddress) -> Result<(), &str> {
-        panic!("remove_mapping not yet implemented!");
+    /// Remove the mapping to `DataSource` that starts at the given address.
+    /// # Errors
+    /// If the mapping could not be removed.
+    pub fn remove_mapping(
+        &self,
+        source: &dyn DataSource,
+        start: VirtualAddress,
+    ) -> Result<(), &str> {
+        todo!()
     }
-}
+}

src/cacher.rs

@@ -1,4 +1,3 @@
-
 // This is the one that needs the most design.
 // It coordinates allocating physical pages to cache data from DataSources
 // It asks DataSources to fetch data, in response to a page fault, and when in arrives, it adds

src/data_source.rs

@@ -3,55 +3,80 @@ use std::fs::File;
 
 pub trait DataSource {
     // constructors are left to each implementation, once you have one, you can:
-    fn read(&self, offset: usize, length: usize, buffer: &mut Vec<u8> ) -> Result<(), &str>;
-    fn write(&self, offset: usize, length: usize, buffer: &mut Vec<u8> ) -> Result<(), &str>;
+    fn read(&self, offset: usize, length: usize, buffer: &mut Vec<u8>) -> Result<(), &str>;
+    fn write(&self, offset: usize, length: usize, buffer: &mut Vec<u8>) -> Result<(), &str>;
     fn flush(&self, offset: usize, length: usize) -> Result<(), &str>;
-    fn add_map(&self, with_flag: Flags, into_address_space: &mut AddressSpace, offset: usize, length: usize) -> Result<usize, &str>;
-    fn del_map(&self, from_address_space: &mut AddressSpace, offset: usize, length: usize) -> Result<(), &str>;
+    fn add_map(
+        &self,
+        with_flag: Flags,
+        into_address_space: &mut AddressSpace,
+        offset: usize,
+        length: usize,
+    ) -> Result<usize, &str>;
+    fn del_map(
+        &self,
+        from_address_space: &mut AddressSpace,
+        offset: usize,
+        length: usize,
+    ) -> Result<(), &str>;
 }
 
-enum Flags {
+pub enum Flags {
     // TODO: do we need more flags?
-    read,
-    write,
-    execute,
-    copy_on_write,
-    private,
-    shared
+    Read,
+    Write,
+    Execute,
+    CopyOnWrite,
+    Private,
+    Shared,
 }
 
+#[allow(clippy::module_name_repetitions)]
 pub struct FileDataSource {
     file_handle: File,
     name: String,
 }
 
 impl FileDataSource {
+    /// Create a new `FileDataSource`.
+    ///
+    /// # Errors
+    /// If the file can't be opened.
     pub fn new(name: &str) -> Result<Self, &str> {
-        if let Ok(f) = File::open(name){
-            Ok(FileDataSource {
-                file_handle: f,
+        File::open(name).map_or(Err("couldn't open {name}"), |file_handle| {
+            Ok(Self {
+                file_handle,
                 name: name.to_string(),
             })
-        } else {
-            Err("couldn't open {name}")
-        }
+        })
     }
 }
 
 impl DataSource for FileDataSource {
-    fn read(&self, offset: usize, length: usize, buffer: &mut Vec<u8> ) -> Result<(), &str> {
+    fn read(&self, offset: usize, length: usize, buffer: &mut Vec<u8>) -> Result<(), &str> {
         panic!("not yet done");
     }
-    fn write(&self, offset: usize, length: usize, buffer: &mut Vec<u8> ) -> Result<(), &str>{
+    fn write(&self, offset: usize, length: usize, buffer: &mut Vec<u8>) -> Result<(), &str> {
         panic!("not yet done");
     }
-    fn flush(&self, offset: usize, length: usize) -> Result<(), &str>{
+    fn flush(&self, offset: usize, length: usize) -> Result<(), &str> {
         panic!("not yet done");
     }
-    fn add_map(&self, with_flag: Flags, into_address_space: &mut AddressSpace, offset: usize, length: usize) -> Result<usize, &str>{
+    fn add_map(
+        &self,
+        with_flag: Flags,
+        into_address_space: &mut AddressSpace,
+        offset: usize,
+        length: usize,
+    ) -> Result<usize, &str> {
         panic!("not yet done");
     }
-    fn del_map(&self, from_address_space: &mut AddressSpace, offset: usize, length: usize) -> Result<(), &str>{
+    fn del_map(
+        &self,
+        from_address_space: &mut AddressSpace,
+        offset: usize,
+        length: usize,
+    ) -> Result<(), &str> {
         panic!("not yet done");
     }
 }

src/lib.rs

@@ -1,10 +1,11 @@
+#![allow(dead_code, unused_variables)]
 
 mod address_space;
-mod data_source;
 mod cacher;
+mod data_source;
 
-use address_space::AddressSpace;
-use data_source::FileDataSource;
+pub use address_space::AddressSpace;
+pub use data_source::FileDataSource;
 
 #[cfg(test)]
 mod tests {