From 6cd9a141c6745cf131e82735dbf69ff5fe11289c Mon Sep 17 00:00:00 2001 From: Dylan McNamee Date: Thu, 23 Feb 2023 12:29:09 -0800 Subject: [PATCH 01/15] vector representation of mappings. Cargo test succeeds 2x and 3x doctests --- src/address_space.rs | 67 +++++++++++++++++++++++++++++++++++--------- src/lib.rs | 22 +++++++++------ 2 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index 8bbf908..3c9f90f 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -1,8 +1,10 @@ -use std::collections::LinkedList; use std::sync::Arc; use crate::data_source::DataSource; +pub const PAGE_SIZE: usize = 4096; +pub const VADDR_MAX: usize = (1 << 38) - 1; + type VirtualAddress = usize; struct MapEntry { @@ -12,10 +14,22 @@ struct MapEntry { addr: usize, } +impl MapEntry { + #[must_use] // <- not using return value of "new" doesn't make sense, so warn + pub fn new(source: Arc, offset: usize, span: usize, addr: usize) -> MapEntry { + MapEntry { + source: source.clone(), + offset, + span, + addr, + } + } +} + /// An address space. pub struct AddressSpace { name: String, - mappings: LinkedList, // see below for comments + mappings: Vec, // see below for comments } // comments about storing mappings @@ -34,21 +48,42 @@ impl AddressSpace { pub fn new(name: &str) -> Self { Self { name: name.to_string(), - mappings: LinkedList::new(), - } + mappings: Vec::new(), // <- here I changed from LinkedList, for reasons + } // I encourage you to try other sparse representations - trees, DIY linked lists, ... } /// Add a mapping from a `DataSource` into this `AddressSpace`. /// /// # Errors /// If the desired mapping is invalid. - pub fn add_mapping( - &self, - source: &D, + /// TODO: how does our test in lib.rs succeed? + // pub fn add_mapping<'a, D: DataSource + 'a>( + // &'a mut self, + pub fn add_mapping( + &mut self, + source: Arc, offset: usize, span: usize, ) -> Result { - todo!() + let mut addr_iter = PAGE_SIZE; // let's not map page 0 + let mut gap; + for mapping in &self.mappings { + gap = mapping.addr - addr_iter; + if gap > span + 2 * PAGE_SIZE { + break; + } + addr_iter = mapping.addr + mapping.span; + } + if addr_iter + span + 2 * PAGE_SIZE < VADDR_MAX { + let mapping_addr = addr_iter + PAGE_SIZE; + let new_mapping = MapEntry::new(source, offset, span, mapping_addr); + self.mappings.push(new_mapping); + self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); + // add this mapping self.mappings.Ok(addr_iter) + // then sort our vector + return Ok(mapping_addr); + } + return Err("out of address space!"); } /// Add a mapping from `DataSource` into this `AddressSpace` starting at a specific address. @@ -57,7 +92,7 @@ impl AddressSpace { /// If there is insufficient room subsequent to `start`. pub fn add_mapping_at( &self, - source: &D, + source: Arc, offset: usize, span: usize, start: VirtualAddress, @@ -71,7 +106,7 @@ impl AddressSpace { /// If the mapping could not be removed. pub fn remove_mapping( &self, - source: &D, + source: Arc, start: VirtualAddress, ) -> Result<(), &str> { todo!() @@ -79,15 +114,20 @@ impl AddressSpace { /// Look up the DataSource and offset within that DataSource for a /// VirtualAddress / AccessType in this AddressSpace - /// + /// /// # Errors /// If this VirtualAddress does not have a valid mapping in &self, /// or if this AccessType is not permitted by the mapping pub fn get_source_for_addr( &self, addr: VirtualAddress, - access_type: FlagBuilder - ) -> Result<(&D, usize), &str> { + access_type: FlagBuilder, + ) -> Result<(Arc, usize), &str> { + todo!(); + } + + /// Helper function for looking up mappings + fn get_mapping_for_addr(&self, addr: VirtualAddress) -> Result { todo!(); } } @@ -218,4 +258,3 @@ impl FlagBuilder { } } } - diff --git a/src/lib.rs b/src/lib.rs index 2e974a4..623923b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,8 +4,10 @@ mod address_space; mod cacher; mod data_source; -pub use address_space::AddressSpace; +pub use address_space::{AddressSpace, FlagBuilder}; pub use data_source::{DataSource, FileDataSource}; +use std::sync::Arc; // <- will have to make Arc ourselves for #no_std + // TODO: why does rustfmt say this is unused, but if I leave it out, undefined? #[cfg(test)] mod tests { @@ -23,18 +25,22 @@ mod tests { // test if mapping has been added #[test] fn test_add_mapping() { - let addr_space = AddressSpace::new("Test address space"); + let mut addr_space = AddressSpace::new("Test address space"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; let length: usize = 1; - let addr = addr_space.add_mapping(&data_source, offset, length).unwrap(); + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping(ds_arc.clone(), offset, length) + .unwrap(); assert!(addr != 0); - // we should move these tests into addr_space, since they access non-public internals of the structure: - // assert_eq!(addr_space.mappings.is_empty(), false); - // assert_eq!(addr_space.mappings.front().source, Some(&data_source)); - // assert_eq!(addr_space.mappings.front().offset, offset); - // assert_eq!(addr_space.mappings.front().span, length); + let addr2 = addr_space + .add_mapping(ds_arc.clone(), address_space::PAGE_SIZE, 0) + .unwrap(); + assert!(addr2 != 0); + assert!(addr2 != addr); } } From c087d6e51acc515b2e150647285baea421e5bd8b Mon Sep 17 00:00:00 2001 From: Dylan McNamee Date: Thu, 23 Feb 2023 14:47:13 -0800 Subject: [PATCH 02/15] added flags to mappings --- src/address_space.rs | 47 ++++++++++++++++++++++++++++++++++++-------- src/lib.rs | 5 +++-- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index 3c9f90f..31f9d53 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -12,16 +12,18 @@ struct MapEntry { offset: usize, span: usize, addr: usize, + flags: FlagBuilder } impl MapEntry { #[must_use] // <- not using return value of "new" doesn't make sense, so warn - pub fn new(source: Arc, offset: usize, span: usize, addr: usize) -> MapEntry { + pub fn new(source: Arc, offset: usize, span: usize, addr: usize, flags: FlagBuilder) -> MapEntry { MapEntry { source: source.clone(), offset, span, addr, + flags, } } } @@ -64,6 +66,7 @@ impl AddressSpace { source: Arc, offset: usize, span: usize, + flags: FlagBuilder, ) -> Result { let mut addr_iter = PAGE_SIZE; // let's not map page 0 let mut gap; @@ -76,28 +79,38 @@ impl AddressSpace { } if addr_iter + span + 2 * PAGE_SIZE < VADDR_MAX { let mapping_addr = addr_iter + PAGE_SIZE; - let new_mapping = MapEntry::new(source, offset, span, mapping_addr); + let new_mapping = MapEntry::new(source, offset, span, mapping_addr, flags); self.mappings.push(new_mapping); self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); - // add this mapping self.mappings.Ok(addr_iter) - // then sort our vector return Ok(mapping_addr); } - return Err("out of address space!"); + Err("out of address space!") } /// 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, + pub fn add_mapping_at( + &mut self, source: Arc, offset: usize, span: usize, start: VirtualAddress, + flags: FlagBuilder ) -> Result<(), &str> { - todo!() + for mapping in &self.mappings { + if start + span < mapping.addr + || start + span > mapping.addr + mapping.span { + continue; + } else { + return Err("overlapping mapping"); + } + } + let new_mapping = MapEntry::new(source, offset, span, start, flags); + self.mappings.push(new_mapping); + self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); + Ok(()) } /// Remove the mapping to `DataSource` that starts at the given address. @@ -156,6 +169,24 @@ pub struct FlagBuilder { shared: bool, } +impl FlagBuilder { + pub fn check_access_perms(&self, access_perms: FlagBuilder) -> bool { + if access_perms.read && !self.read || access_perms.write && !self.write || access_perms.execute && !self.execute { + return false; + } + true + } + + pub fn is_valid(&self) -> bool { + if self.private && self.shared { + return false; + } + if self.cow && self.write { // for COW to work, write needs to be off until after the copy + return false; + } + return true; + } +} /// Create a constructor and toggler for a `FlagBuilder` object. Will capture attributes, including documentation /// comments and apply them to the generated constructor. macro_rules! flag { diff --git a/src/lib.rs b/src/lib.rs index 623923b..f5499db 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,16 +29,17 @@ mod tests { let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; let length: usize = 1; + let read_flags = FlagBuilder::new().toggle_read(); let ds_arc = Arc::new(data_source); let addr = addr_space - .add_mapping(ds_arc.clone(), offset, length) + .add_mapping(ds_arc.clone(), offset, length, read_flags) .unwrap(); assert!(addr != 0); let addr2 = addr_space - .add_mapping(ds_arc.clone(), address_space::PAGE_SIZE, 0) + .add_mapping(ds_arc.clone(), address_space::PAGE_SIZE, 0, read_flags) .unwrap(); assert!(addr2 != 0); assert!(addr2 != addr); From 9515211f6d629ad023755ebd38ab0fe104be6336 Mon Sep 17 00:00:00 2001 From: Dylan McNamee Date: Thu, 23 Feb 2023 14:48:10 -0800 Subject: [PATCH 03/15] cleanup for flags push --- src/address_space.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index 31f9d53..59c8c28 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -99,18 +99,7 @@ impl AddressSpace { start: VirtualAddress, flags: FlagBuilder ) -> Result<(), &str> { - for mapping in &self.mappings { - if start + span < mapping.addr - || start + span > mapping.addr + mapping.span { - continue; - } else { - return Err("overlapping mapping"); - } - } - let new_mapping = MapEntry::new(source, offset, span, start, flags); - self.mappings.push(new_mapping); - self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); - Ok(()) + todo!() } /// Remove the mapping to `DataSource` that starts at the given address. From 7f33290698dedde2613f34b0e15ce3c6c9b5877e Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 12:01:32 -0800 Subject: [PATCH 04/15] Add comments and adjust span size --- src/address_space.rs | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index 59c8c28..2728256 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -12,12 +12,18 @@ struct MapEntry { offset: usize, span: usize, addr: usize, - flags: FlagBuilder + flags: FlagBuilder, } impl MapEntry { #[must_use] // <- not using return value of "new" doesn't make sense, so warn - pub fn new(source: Arc, offset: usize, span: usize, addr: usize, flags: FlagBuilder) -> MapEntry { + pub fn new( + source: Arc, + offset: usize, + span: usize, + addr: usize, + flags: FlagBuilder, + ) -> MapEntry { MapEntry { source: source.clone(), offset, @@ -70,16 +76,29 @@ impl AddressSpace { ) -> Result { let mut addr_iter = PAGE_SIZE; // let's not map page 0 let mut gap; + let adjusted_span: usize; + if span % PAGE_SIZE != 0 { + adjusted_span = PAGE_SIZE - (PAGE_SIZE % span); + } else { + adjusted_span = span + } for mapping in &self.mappings { + // space between end of last entry and start of this entry gap = mapping.addr - addr_iter; - if gap > span + 2 * PAGE_SIZE { + // we found a big free space! + // need to have a free page between each mapping + // for accidental overflows + if gap > adjusted_span + 2 * PAGE_SIZE { break; } + // move addr_iter to the next potentially free chunk of memory addr_iter = mapping.addr + mapping.span; } - if addr_iter + span + 2 * PAGE_SIZE < VADDR_MAX { + // if we have enough space for our new mapping + if addr_iter + adjusted_span + 2 * PAGE_SIZE < VADDR_MAX { + // add free page between mappings let mapping_addr = addr_iter + PAGE_SIZE; - let new_mapping = MapEntry::new(source, offset, span, mapping_addr, flags); + let new_mapping = MapEntry::new(source, offset, adjusted_span, mapping_addr, flags); self.mappings.push(new_mapping); self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); return Ok(mapping_addr); @@ -97,7 +116,7 @@ impl AddressSpace { offset: usize, span: usize, start: VirtualAddress, - flags: FlagBuilder + flags: FlagBuilder, ) -> Result<(), &str> { todo!() } @@ -160,17 +179,21 @@ pub struct FlagBuilder { impl FlagBuilder { pub fn check_access_perms(&self, access_perms: FlagBuilder) -> bool { - if access_perms.read && !self.read || access_perms.write && !self.write || access_perms.execute && !self.execute { + if access_perms.read && !self.read + || access_perms.write && !self.write + || access_perms.execute && !self.execute + { return false; - } - true + } + true } pub fn is_valid(&self) -> bool { if self.private && self.shared { return false; } - if self.cow && self.write { // for COW to work, write needs to be off until after the copy + if self.cow && self.write { + // for COW to work, write needs to be off until after the copy return false; } return true; From ff4e4f8224588c4e8eb7601ecd4860b0b71e2625 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 12:04:14 -0800 Subject: [PATCH 05/15] Fix adjusted span computation --- src/address_space.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index 2728256..51f184b 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -76,12 +76,7 @@ impl AddressSpace { ) -> Result { let mut addr_iter = PAGE_SIZE; // let's not map page 0 let mut gap; - let adjusted_span: usize; - if span % PAGE_SIZE != 0 { - adjusted_span = PAGE_SIZE - (PAGE_SIZE % span); - } else { - adjusted_span = span - } + let adjusted_span: usize = span + PAGE_SIZE - (span % PAGE_SIZE); for mapping in &self.mappings { // space between end of last entry and start of this entry gap = mapping.addr - addr_iter; From 46e6f545ed80cb6dc4b03239ef036fbe785244fe Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 12:28:02 -0800 Subject: [PATCH 06/15] Implement add_mapping_at --- src/address_space.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/address_space.rs b/src/address_space.rs index 51f184b..ab9fa30 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -113,7 +113,33 @@ impl AddressSpace { start: VirtualAddress, flags: FlagBuilder, ) -> Result<(), &str> { - todo!() + let adjusted_span: usize = span + PAGE_SIZE - (span % PAGE_SIZE); + let end = start + adjusted_span; + for mapping in &self.mappings { + if mapping.addr + mapping.span < start { + // we have not reached the area we need yet + continue; + } else if start + adjusted_span < mapping.addr { + // we've moved past the area we need + break; + } else if start <= mapping.addr && mapping.addr <= start + adjusted_span { + // there is a mapping that starts in the space we want to place the new mapping + return Err("out of address space!"); + } else if start <= mapping.addr + mapping.span + && mapping.addr + mapping.span <= start + adjusted_span + { + // there is a mapping that ends in the space we want to place the new mapping + return Err("out of address space!"); + } else if mapping.addr < start && start + adjusted_span < mapping.addr + mapping.span { + // there is a mapping covers the space we want to place the new mapping + return Err("out of address space!"); + } + } + // if we have enough space for our new mapping + let new_mapping = MapEntry::new(source, offset, adjusted_span, start, flags); + self.mappings.push(new_mapping); + self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); + return Ok(()); } /// Remove the mapping to `DataSource` that starts at the given address. From a72fb316f02b1cc9b6d755330581fda4508a7dd9 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 20:40:58 -0800 Subject: [PATCH 07/15] Implement remove mapping and get_source_for_addr --- src/address_space.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index ab9fa30..f36c9b7 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -147,11 +147,17 @@ impl AddressSpace { /// # Errors /// If the mapping could not be removed. pub fn remove_mapping( - &self, + &mut self, source: Arc, start: VirtualAddress, ) -> Result<(), &str> { - todo!() + // To-Do deal w source + let s = &self.mappings.len(); + let _ = &self.mappings.retain(|m| m.addr != start); + if self.mappings.len() != s - 1 { + return Err("error removing mapping"); + } + return Ok(()); } /// Look up the DataSource and offset within that DataSource for a @@ -165,7 +171,24 @@ impl AddressSpace { addr: VirtualAddress, access_type: FlagBuilder, ) -> Result<(Arc, usize), &str> { - todo!(); + for mapping in &self.mappings { + if mapping.addr + mapping.span < addr { + // we have not reached the area we need yet + continue; + } else if mapping.addr <= addr && addr <= mapping.addr + mapping.span { + // the address we are looking for is the in range of this data source + if mapping.flags.private || !mapping.flags.read { + // we do not have access + return Err("Access Type is not permitted by the mapping!"); + } else { + return Ok((mapping.source, mapping.offset)); + } + } else if addr < mapping.addr { + // we passed the address we were looking for and found nothing + return Err("VirtualAddress does not a valid mapping!"); + } + } + return return Err("VirtualAddress does not a valid mapping!"); } /// Helper function for looking up mappings From 538a9ba385db943dd2821723188ac0ed67a72738 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 20:55:02 -0800 Subject: [PATCH 08/15] Add tests for add_mapping_at --- src/lib.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index f5499db..4574ed7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,8 @@ use std::sync::Arc; // <- will have to make Arc ourselves for #no_std #[cfg(test)] mod tests { + use crate::address_space::PAGE_SIZE; + use super::*; #[test] @@ -44,4 +46,76 @@ mod tests { assert!(addr2 != 0); assert!(addr2 != addr); } + + // test running out of address space + #[test] + #[should_panic] + fn test_add_mapping_error() { + let mut addr_space = AddressSpace::new("Test address space for errors"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = (1 << 38); + let read_flags = FlagBuilder::new().toggle_read(); + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping(ds_arc.clone(), offset, length, read_flags) + .unwrap(); + } + + // test if mapping has been added + #[test] + fn test_add_mapping_at() { + let mut addr_space = AddressSpace::new("Test address space 1"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = 1; + let start: usize = 100; + let read_flags = FlagBuilder::new().toggle_read(); + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .unwrap(); + } + + // test adding multiple files to the same VA + #[test] + #[should_panic] + fn test_add_mapping_at_error() { + let mut addr_space = AddressSpace::new("Test address space for errors 1"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = 1; + let start: usize = 2 * PAGE_SIZE; + let read_flags = FlagBuilder::new().toggle_read(); + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .unwrap(); + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .unwrap(); + } + + // test adding a file without read permissions + #[test] + #[should_panic] + fn test_add_mapping_at_error_access() { + let mut addr_space = AddressSpace::new("Test address space for errors 2"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = 1; + let start: usize = 2 * PAGE_SIZE; + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .unwrap(); + } } From c4bcd4adfa0537a1743bd74dc6399d6e52753c30 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 20:56:14 -0800 Subject: [PATCH 09/15] Pass in flags to error_access --- src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 4574ed7..e7804b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,11 +111,12 @@ mod tests { let offset: usize = 0; let length: usize = 1; let start: usize = 2 * PAGE_SIZE; + let flags = FlagBuilder::new(); let ds_arc = Arc::new(data_source); let addr = addr_space - .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .add_mapping_at(ds_arc.clone(), offset, length, start, flags) .unwrap(); } } From 290bcdee02efe7a78778aca300062cc327c9dae4 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Sun, 26 Feb 2023 21:05:28 -0800 Subject: [PATCH 10/15] Add remove mapping tests --- src/lib.rs | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e7804b1..312e2bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,7 +67,7 @@ mod tests { // test if mapping has been added #[test] fn test_add_mapping_at() { - let mut addr_space = AddressSpace::new("Test address space 1"); + let mut addr_space = AddressSpace::new("Test address space"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; let length: usize = 1; @@ -85,7 +85,7 @@ mod tests { #[test] #[should_panic] fn test_add_mapping_at_error() { - let mut addr_space = AddressSpace::new("Test address space for errors 1"); + let mut addr_space = AddressSpace::new("Test address space"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; let length: usize = 1; @@ -106,7 +106,7 @@ mod tests { #[test] #[should_panic] fn test_add_mapping_at_error_access() { - let mut addr_space = AddressSpace::new("Test address space for errors 2"); + let mut addr_space = AddressSpace::new("Test address space"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; let length: usize = 1; @@ -119,4 +119,37 @@ mod tests { .add_mapping_at(ds_arc.clone(), offset, length, start, flags) .unwrap(); } + + // test remove mapping + #[test] + #[should_panic] + fn test_remove_mapping() { + let mut addr_space = AddressSpace::new("Test address space"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = 1; + let start: usize = 100; + let read_flags = FlagBuilder::new().toggle_read(); + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, read_flags) + .unwrap(); + + let res = addr_space.remove_mapping(ds_arc.clone(), start).unwrap(); + } + + // test removing a file from an empty address space + #[test] + #[should_panic] + fn test_remove_mapping_error() { + let mut addr_space = AddressSpace::new("Test address space"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let start: usize = PAGE_SIZE; + + let ds_arc = Arc::new(data_source); + + let addr = addr_space.remove_mapping(ds_arc.clone(), start).unwrap(); + } } From b0a3ae03648071e0cce38add4d27d280b5de8819 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Mon, 27 Feb 2023 12:45:07 -0800 Subject: [PATCH 11/15] write get_mapping_for_addr and fix get source and remove --- src/address_space.rs | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index f36c9b7..998019c 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -136,7 +136,7 @@ impl AddressSpace { } } // if we have enough space for our new mapping - let new_mapping = MapEntry::new(source, offset, adjusted_span, start, flags); + let new_mapping = MapEntry::new(source.clone(), offset, adjusted_span, start, flags); self.mappings.push(new_mapping); self.mappings.sort_by(|a, b| a.addr.cmp(&b.addr)); return Ok(()); @@ -151,12 +151,13 @@ impl AddressSpace { source: Arc, start: VirtualAddress, ) -> Result<(), &str> { - // To-Do deal w source + let mapping = self.get_mapping_for_addr(start).unwrap(); let s = &self.mappings.len(); - let _ = &self.mappings.retain(|m| m.addr != start); + self.mappings.retain(|m| m != mapping); if self.mappings.len() != s - 1 { return Err("error removing mapping"); } + drop(source); return Ok(()); } @@ -166,34 +167,40 @@ impl AddressSpace { /// # Errors /// If this VirtualAddress does not have a valid mapping in &self, /// or if this AccessType is not permitted by the mapping + /// access is read,write,and execute + /// private vs cow vs shared is about cache sharing pub fn get_source_for_addr( &self, addr: VirtualAddress, access_type: FlagBuilder, - ) -> Result<(Arc, usize), &str> { + ) -> Result<(Arc, usize), &str> { + let mapping = self.get_mapping_for_addr(addr).unwrap(); + if !mapping.flags.read { + // we do not have access + return Err("Access Type is not permitted by the mapping!"); + } else { + let offset = mapping.offset + addr; + return Ok((mapping.source.clone(), offset)); + } + } + + /// Helper function for looking up mappings + fn get_mapping_for_addr(&self, addr: VirtualAddress) -> Result<&MapEntry, &str> { + // tells you if there is an existsing mapping at a giving addr + // todo!(); for mapping in &self.mappings { if mapping.addr + mapping.span < addr { // we have not reached the area we need yet continue; } else if mapping.addr <= addr && addr <= mapping.addr + mapping.span { // the address we are looking for is the in range of this data source - if mapping.flags.private || !mapping.flags.read { - // we do not have access - return Err("Access Type is not permitted by the mapping!"); - } else { - return Ok((mapping.source, mapping.offset)); - } + return Ok(mapping); } else if addr < mapping.addr { // we passed the address we were looking for and found nothing return Err("VirtualAddress does not a valid mapping!"); } } - return return Err("VirtualAddress does not a valid mapping!"); - } - - /// Helper function for looking up mappings - fn get_mapping_for_addr(&self, addr: VirtualAddress) -> Result { - todo!(); + return Err("VirtualAddress does not a valid mapping!"); } } From 9ead9f02344a78badaf0f6cd67a8f3443d1837bf Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Mon, 27 Feb 2023 12:57:20 -0800 Subject: [PATCH 12/15] Add todo comment --- src/address_space.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/address_space.rs b/src/address_space.rs index 998019c..d59b2ab 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -16,6 +16,7 @@ struct MapEntry { } impl MapEntry { + // define PartialEq #[must_use] // <- not using return value of "new" doesn't make sense, so warn pub fn new( source: Arc, @@ -153,7 +154,12 @@ impl AddressSpace { ) -> Result<(), &str> { let mapping = self.get_mapping_for_addr(start).unwrap(); let s = &self.mappings.len(); - self.mappings.retain(|m| m != mapping); + + if let Some(pos) = self.mappings.iter().position(|x| x == mapping) { + self.mappings.remove(pos); + } else { + return Err("error removing mapping"); + } if self.mappings.len() != s - 1 { return Err("error removing mapping"); } From 4e77a18e0ee642eee6f2966ab7afef1c4be54cb6 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Mon, 27 Feb 2023 13:03:20 -0800 Subject: [PATCH 13/15] Change equality to mapping.addr --- src/address_space.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index d59b2ab..a2096b1 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -16,7 +16,7 @@ struct MapEntry { } impl MapEntry { - // define PartialEq + // define PartialEq, this will be addr being the same #[must_use] // <- not using return value of "new" doesn't make sense, so warn pub fn new( source: Arc, @@ -155,7 +155,7 @@ impl AddressSpace { let mapping = self.get_mapping_for_addr(start).unwrap(); let s = &self.mappings.len(); - if let Some(pos) = self.mappings.iter().position(|x| x == mapping) { + if let Some(pos) = self.mappings.iter().position(|x| x.addr == mapping.addr) { self.mappings.remove(pos); } else { return Err("error removing mapping"); From 0259d6f252b422d91a318efdbc67efbc62bc7aa6 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Mon, 27 Feb 2023 15:28:26 -0800 Subject: [PATCH 14/15] Correct remove_mapping to not expect panic, implement src_addr with error --- src/lib.rs | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 312e2bb..78560c8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,8 @@ use std::sync::Arc; // <- will have to make Arc ourselves for #no_std #[cfg(test)] mod tests { + use std::fs::File; + use crate::address_space::PAGE_SIZE; use super::*; @@ -54,7 +56,7 @@ mod tests { let mut addr_space = AddressSpace::new("Test address space for errors"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); let offset: usize = 0; - let length: usize = (1 << 38); + let length: usize = 1 << 38; let read_flags = FlagBuilder::new().toggle_read(); let ds_arc = Arc::new(data_source); @@ -102,27 +104,8 @@ mod tests { .unwrap(); } - // test adding a file without read permissions - #[test] - #[should_panic] - fn test_add_mapping_at_error_access() { - let mut addr_space = AddressSpace::new("Test address space"); - let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); - let offset: usize = 0; - let length: usize = 1; - let start: usize = 2 * PAGE_SIZE; - let flags = FlagBuilder::new(); - - let ds_arc = Arc::new(data_source); - - let addr = addr_space - .add_mapping_at(ds_arc.clone(), offset, length, start, flags) - .unwrap(); - } - // test remove mapping #[test] - #[should_panic] fn test_remove_mapping() { let mut addr_space = AddressSpace::new("Test address space"); let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); @@ -152,4 +135,26 @@ mod tests { let addr = addr_space.remove_mapping(ds_arc.clone(), start).unwrap(); } + + // test adding a file without read permissions + #[test] + #[should_panic] + fn test_get_source_for_addr_error_access() { + let mut addr_space = AddressSpace::new("Test address space"); + let data_source: FileDataSource = FileDataSource::new("Cargo.toml").unwrap(); + let offset: usize = 0; + let length: usize = 1; + let start: usize = 2 * PAGE_SIZE; + let flags = FlagBuilder::new(); + + let ds_arc = Arc::new(data_source); + + let addr = addr_space + .add_mapping_at(ds_arc.clone(), offset, length, start, flags) + .unwrap(); + + let mapping = addr_space + .get_source_for_addr::(start, flags) + .unwrap(); + } } From 884c0479e411d6ced1f62d5642f03f48d8774c99 Mon Sep 17 00:00:00 2001 From: AriaKillebrewBruehl Date: Mon, 27 Feb 2023 15:28:51 -0800 Subject: [PATCH 15/15] Check access_type parameter instead of mapping --- src/address_space.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/address_space.rs b/src/address_space.rs index a2096b1..5ca68ce 100644 --- a/src/address_space.rs +++ b/src/address_space.rs @@ -180,11 +180,11 @@ impl AddressSpace { addr: VirtualAddress, access_type: FlagBuilder, ) -> Result<(Arc, usize), &str> { - let mapping = self.get_mapping_for_addr(addr).unwrap(); - if !mapping.flags.read { + if !access_type.read { // we do not have access return Err("Access Type is not permitted by the mapping!"); } else { + let mapping = self.get_mapping_for_addr(addr).unwrap(); let offset = mapping.offset + addr; return Ok((mapping.source.clone(), offset)); } @@ -192,7 +192,7 @@ impl AddressSpace { /// Helper function for looking up mappings fn get_mapping_for_addr(&self, addr: VirtualAddress) -> Result<&MapEntry, &str> { - // tells you if there is an existsing mapping at a giving addr + // tells you if there is an existing mapping at a giving addr // todo!(); for mapping in &self.mappings { if mapping.addr + mapping.span < addr {