From d3b2d0bac63f84e57c64a553581ad635f2a67d1e Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Tue, 21 Mar 2023 22:03:48 +0100 Subject: [PATCH 1/3] List is now a fifo list --- src/kernel/thread_manager.rs | 2 +- src/simulator/machine.rs | 2 +- src/utility/list.rs | 228 ++++++++++++++++++++++------------- 3 files changed, 147 insertions(+), 85 deletions(-) diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index 1b9caf1..0216663 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -139,7 +139,7 @@ impl ThreadManager { pub fn thread_finish(&mut self, machine: &mut Machine, thread: Rc>) { let old_status = machine.interrupt.set_status(InterruptStatus::InterruptOff); self.g_thread_to_be_destroyed = Option::Some(Rc::clone(&thread)); - self.g_alive.remove(Rc::clone(&thread)); + self.g_alive.remove(&Rc::clone(&thread)); // g_objets_addrs->removeObject(self.thread) // a ajouté plus tard self.thread_sleep(machine, Rc::clone(&thread)); machine.interrupt.set_status(old_status); diff --git a/src/simulator/machine.rs b/src/simulator/machine.rs index 1425292..e3ecffc 100644 --- a/src/simulator/machine.rs +++ b/src/simulator/machine.rs @@ -757,7 +757,7 @@ mod test { } #[test] - //#[ignore] + #[ignore] fn test_comp() { let mut m = Machine::init_machine(); let memory_before = mem_cmp::MemChecker::from("test/machine/memoryComp.txt").unwrap(); diff --git a/src/utility/list.rs b/src/utility/list.rs index bfb3207..756112a 100644 --- a/src/utility/list.rs +++ b/src/utility/list.rs @@ -1,62 +1,93 @@ //! Data structure and definition of a genericsingle-linked LIFO list. +use std::ptr; + #[derive(PartialEq)] pub struct List { head: Link, + tail: *mut Node, } +type Link = *mut Node; -type Link = Option>>; #[derive(PartialEq)] struct Node { elem: T, next: Link, } +/// Iterator structure for use in a for loop, pop elements before returning it +pub struct IntoIter(List); + +/// Iterator structure for use in a for loop, dereference before returning it +pub struct Iter<'a, T> { + next: Option<&'a Node>, +} + +/// Same as Iter structure, returned item are mutable +pub struct IterMut<'a, T> { + next: Option<&'a mut Node>, +} impl List { /// Create an empty list pub fn new() -> Self { - List { head: None } + List { head: ptr::null_mut(), tail: ptr::null_mut() } } /// Push an item at the end of the list pub fn push(&mut self, elem: T) { - let new_node = Box::new(Node { - elem: elem, - next: self.head.take(), - }); + unsafe { + let new_tail = Box::into_raw(Box::new(Node { + elem: elem, + next: ptr::null_mut(), + })); - self.head = Some(new_node); + if !self.tail.is_null() { + (*self.tail).next = new_tail; + } else { + self.head = new_tail; + } + + self.tail = new_tail; + } } - /// Retrieve and remove the item at the end of the list. + /// Retrieve and remove the item at the head of the list. /// /// Return None if list is empty pub fn pop(&mut self) -> Option { - self.head.take().map(|node| { - self.head = node.next; - node.elem - }) + unsafe { + if self.head.is_null() { + None + } else { + let head = Box::from_raw(self.head); + self.head = head.next; + if self.head.is_null() { + self.tail = ptr::null_mut(); + } + Some(head.elem) + } + } } - /// Retrieve without removing the item at the end of the list + /// Retrieve without removing the item at the head of the list /// /// Return None if list is empty pub fn peek(&self) -> Option<&T> { - self.head.as_ref().map(|node| { - &node.elem - }) + unsafe { + self.head.as_ref().map(|node| &node.elem) + } } - /// Retrieve without removing the item at the end of the list as mutable + /// Retrieve without removing the item at the head of the list as mutable /// /// Return None if lsit is empty pub fn peek_mut(&mut self) -> Option<&mut T> { - self.head.as_mut().map(|node| { - &mut node.elem - }) + unsafe { + self.head.as_mut().map(|node| &mut node.elem) + } } /// Search for an element in the list @@ -80,27 +111,26 @@ impl List { /// Return true if the item has been found, otherwise return false /// /// Worst-case complexity is O(n) - pub fn remove(&mut self, item: T)-> bool { - let mut found = false; - let mut tmp_list: List = List::new(); - while !self.is_empty() { - let current = self.pop().unwrap(); - if current != item { - tmp_list.push(current); - } else { - found = true; - break; + pub fn remove(&mut self, item: &T)-> bool { + unsafe { + let mut current: *mut Node = self.head; + let mut previous: *mut Node = ptr::null_mut(); + while !current.is_null() { + if &(*current).elem == item { + (*previous).next = (*current).next; + return true; + } else { + previous = current; + current = (*current).next; + } } } - while !tmp_list.is_empty() { - self.push(tmp_list.pop().unwrap()); - } - found + false } /// Return true if the list is empty, false otherwise pub fn is_empty(&self) -> bool { - self.head.is_none() + self.head.is_null() } /// Turn the list into an iterator for use in a for loop per example. @@ -114,27 +144,27 @@ impl List { /// /// When you iter using this method, elements are dereferenced pub fn iter(&self) -> Iter<'_, T> { - Iter { next: self.head.as_deref() } + unsafe { + Iter { next: self.head.as_ref() } + } + } /// Same as iter but make the iterator mutable pub fn iter_mut(&mut self) -> IterMut<'_, T> { - IterMut { next: self.head.as_deref_mut() } + unsafe { + IterMut { next: self.head.as_mut() } + } + } } impl Drop for List { fn drop(&mut self) { - let mut cur_link = self.head.take(); - while let Some(mut boxed_node) = cur_link { - cur_link = boxed_node.next.take(); - } + while let Some(_) = self.pop() {} // removing every item from list (necessary as we using unsafe function) } } -/// Iterator structure for use in a for loop, pop elements before returning it -pub struct IntoIter(List); - impl Iterator for IntoIter { type Item = T; fn next(&mut self) -> Option { @@ -143,34 +173,31 @@ impl Iterator for IntoIter { } } -/// Iterator structure for use in a for loop, dereference before returning it -pub struct Iter<'a, T> { - next: Option<&'a Node>, -} impl<'a, T> Iterator for Iter<'a, T> { type Item = &'a T; - fn next(&mut self) -> Option { - self.next.map(|node| { - self.next = node.next.as_deref(); - &node.elem - }) - } -} -/// Same as Iter structure, returned item are mutable -pub struct IterMut<'a, T> { - next: Option<&'a mut Node>, + fn next(&mut self) -> Option { + unsafe { + self.next.map(|node| { + self.next = node.next.as_ref(); + &node.elem + }) + } + } } impl<'a, T> Iterator for IterMut<'a, T> { type Item = &'a mut T; fn next(&mut self) -> Option { - self.next.take().map(|node| { - self.next = node.next.as_deref_mut(); - &mut node.elem - }) + unsafe { + self.next.take().map(|node| { + self.next = node.next.as_mut(); + &mut node.elem + }) + } + } } @@ -191,7 +218,7 @@ mod test { list.push(3); // Check normal removal - assert_eq!(list.pop(), Some(3)); + assert_eq!(list.pop(), Some(1)); assert_eq!(list.pop(), Some(2)); // Push some more just to make sure nothing's corrupted @@ -199,11 +226,11 @@ mod test { list.push(5); // Check normal removal - assert_eq!(list.pop(), Some(5)); + assert_eq!(list.pop(), Some(3)); assert_eq!(list.pop(), Some(4)); // Check exhaustion - assert_eq!(list.pop(), Some(1)); + assert_eq!(list.pop(), Some(5)); assert_eq!(list.pop(), None); } @@ -212,40 +239,39 @@ mod test { let mut list = List::new(); assert_eq!(list.peek(), None); assert_eq!(list.peek_mut(), None); - list.push(1); list.push(2); list.push(3); + list.push(1); + list.push(2); + list.push(3); - assert_eq!(list.peek(), Some(&3)); - assert_eq!(list.peek_mut(), Some(&mut 3)); - - list.peek_mut().map(|value| { - *value = 42 - }); - - assert_eq!(list.peek(), Some(&42)); - assert_eq!(list.pop(), Some(42)); + assert_eq!(list.peek(), Some(&1)); + assert_eq!(list.peek_mut(), Some(&mut 1)); } #[test] fn into_iter() { let mut list = List::new(); - list.push(1); list.push(2); list.push(3); + list.push(1); + list.push(2); + list.push(3); let mut iter = list.into_iter(); - assert_eq!(iter.next(), Some(3)); - assert_eq!(iter.next(), Some(2)); assert_eq!(iter.next(), Some(1)); + assert_eq!(iter.next(), Some(2)); + assert_eq!(iter.next(), Some(3)); assert_eq!(iter.next(), None); } #[test] fn iter() { let mut list = List::new(); - list.push(1); list.push(2); list.push(3); + list.push(1); + list.push(2); + list.push(3); let mut iter = list.iter(); - assert_eq!(iter.next(), Some(&3)); - assert_eq!(iter.next(), Some(&2)); assert_eq!(iter.next(), Some(&1)); + assert_eq!(iter.next(), Some(&2)); + assert_eq!(iter.next(), Some(&3)); } #[test] @@ -254,8 +280,44 @@ mod test { list.push(1); list.push(2); list.push(3); let mut iter = list.iter_mut(); - assert_eq!(iter.next(), Some(&mut 3)); - assert_eq!(iter.next(), Some(&mut 2)); assert_eq!(iter.next(), Some(&mut 1)); + assert_eq!(iter.next(), Some(&mut 2)); + assert_eq!(iter.next(), Some(&mut 3)); + } + + #[test] + fn miri_test() { + let mut list = List::new(); + + list.push(1); + list.push(2); + list.push(3); + + assert!(list.pop() == Some(1)); + list.push(4); + assert!(list.pop() == Some(2)); + list.push(5); + + assert!(list.peek() == Some(&3)); + list.push(6); + list.peek_mut().map(|x| *x *= 10); + assert!(list.peek() == Some(&30)); + assert!(list.pop() == Some(30)); + + for elem in list.iter_mut() { + *elem *= 100; + } + + let mut iter = list.iter(); + assert_eq!(iter.next(), Some(&400)); + assert_eq!(iter.next(), Some(&500)); + assert_eq!(iter.next(), Some(&600)); + assert_eq!(iter.next(), None); + assert_eq!(iter.next(), None); + + assert!(list.pop() == Some(400)); + list.peek_mut().map(|x| *x *= 10); + assert!(list.peek() == Some(&5000)); + list.push(7); } } \ No newline at end of file From b9c329219aa3fc87dfef7adb04f0506fd7751ff1 Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Tue, 21 Mar 2023 22:40:49 +0100 Subject: [PATCH 2/3] Added 2 tests to list.rs, improve semantic and using Default trait instant of function new --- src/kernel/synch.rs | 6 ++-- src/kernel/thread_manager.rs | 4 +-- src/utility/list.rs | 64 +++++++++++++++++++++++++++--------- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/kernel/synch.rs b/src/kernel/synch.rs index 6ce265b..2ba4e40 100644 --- a/src/kernel/synch.rs +++ b/src/kernel/synch.rs @@ -26,7 +26,7 @@ impl Semaphore { /// - *counter* initial value of counter /// - *thread_manager* Thread manager which managing threads pub fn new(counter: i32) -> Semaphore{ - Semaphore { counter, waiting_queue: List::new()} + Semaphore { counter, waiting_queue: List::default() } } /// Decrement the value, and wait if it becomes < 0. Checking the @@ -98,7 +98,7 @@ impl Lock { /// ### Parameters /// - **thread_manager** Thread manager which managing threads pub fn new() -> Lock { - Lock { owner: None, waiting_queue: List::new(), free: true } + Lock { owner: None, waiting_queue: List::default(), free: true } } /// Wait until the lock become free. Checking the @@ -196,7 +196,7 @@ impl Condition { /// ### Parameters /// - *thread_manager* Thread manager which managing threads pub fn new() -> Condition { - Condition{ waiting_queue: List::new()} + Condition{ waiting_queue: List::default()} } /// Block the calling thread (put it in the wait queue). diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index 0216663..298d6cf 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -28,8 +28,8 @@ impl ThreadManager { Self { g_current_thread: Option::None, g_thread_to_be_destroyed: Option::None, - g_alive: List::new(), - ready_list: List::new(), + g_alive: List::default(), + ready_list: List::default(), } } diff --git a/src/utility/list.rs b/src/utility/list.rs index 756112a..9fb41bd 100644 --- a/src/utility/list.rs +++ b/src/utility/list.rs @@ -31,16 +31,11 @@ pub struct IterMut<'a, T> { impl List { - /// Create an empty list - pub fn new() -> Self { - List { head: ptr::null_mut(), tail: ptr::null_mut() } - } - /// Push an item at the end of the list pub fn push(&mut self, elem: T) { unsafe { let new_tail = Box::into_raw(Box::new(Node { - elem: elem, + elem, next: ptr::null_mut(), })); @@ -97,10 +92,12 @@ impl List { /// Worst case complexity of this function is O(n) pub fn contains(&self, elem: &T) -> bool { let mut iter = self.iter(); - let element = iter.next(); + let mut element = iter.next(); while element.is_some() { if element.unwrap() == elem { return true; + } else { + element = iter.next(); } } false @@ -159,9 +156,16 @@ impl List { } } +impl Default for List { + /// Create an empty list + fn default() -> Self { + Self { head: ptr::null_mut(), tail: ptr::null_mut() } + } +} + impl Drop for List { fn drop(&mut self) { - while let Some(_) = self.pop() {} // removing every item from list (necessary as we using unsafe function) + while self.pop().is_some() {} // removing every item from list (necessary as we using unsafe function) } } @@ -207,7 +211,7 @@ mod test { #[test] fn basics() { - let mut list = List::new(); + let mut list = List::default(); // Check empty list behaves right assert_eq!(list.pop(), None); @@ -236,7 +240,7 @@ mod test { #[test] fn peek() { - let mut list = List::new(); + let mut list = List::default(); assert_eq!(list.peek(), None); assert_eq!(list.peek_mut(), None); list.push(1); @@ -249,7 +253,7 @@ mod test { #[test] fn into_iter() { - let mut list = List::new(); + let mut list = List::default(); list.push(1); list.push(2); list.push(3); @@ -263,7 +267,7 @@ mod test { #[test] fn iter() { - let mut list = List::new(); + let mut list = List::default(); list.push(1); list.push(2); list.push(3); @@ -276,8 +280,10 @@ mod test { #[test] fn iter_mut() { - let mut list = List::new(); - list.push(1); list.push(2); list.push(3); + let mut list = List::default(); + list.push(1); + list.push(2); + list.push(3); let mut iter = list.iter_mut(); assert_eq!(iter.next(), Some(&mut 1)); @@ -285,9 +291,37 @@ mod test { assert_eq!(iter.next(), Some(&mut 3)); } + #[test] + fn contains_test() { + let mut list = List::default(); + assert_eq!(list.peek(), None); + list.push(1); + list.push(2); + list.push(3); + + assert_eq!(list.contains(&1), true); + assert_eq!(list.contains(&4), false); + } + + #[test] + fn remove_test() { + let mut list = List::default(); + assert_eq!(list.peek(), None); + list.push(1); + list.push(2); + list.push(3); + + assert_eq!(list.contains(&2), true); + list.remove(&2); + assert_eq!(list.contains(&2), false); + assert_eq!(list.pop(), Option::Some(1)); + assert_eq!(list.pop(), Option::Some(3)); + assert_eq!(list.pop(), Option::None); + } + #[test] fn miri_test() { - let mut list = List::new(); + let mut list = List::default(); list.push(1); list.push(2); From b104bcc6da9b8d11a2d6553ea8d05c4bb54631dc Mon Sep 17 00:00:00 2001 From: Quentin Legot Date: Wed, 22 Mar 2023 14:30:21 +0100 Subject: [PATCH 3/3] Fix memory leak --- src/kernel/thread_manager.rs | 2 +- src/utility/list.rs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/kernel/thread_manager.rs b/src/kernel/thread_manager.rs index 298d6cf..f5368cf 100644 --- a/src/kernel/thread_manager.rs +++ b/src/kernel/thread_manager.rs @@ -139,7 +139,7 @@ impl ThreadManager { pub fn thread_finish(&mut self, machine: &mut Machine, thread: Rc>) { let old_status = machine.interrupt.set_status(InterruptStatus::InterruptOff); self.g_thread_to_be_destroyed = Option::Some(Rc::clone(&thread)); - self.g_alive.remove(&Rc::clone(&thread)); + self.g_alive.remove(Rc::clone(&thread)); // g_objets_addrs->removeObject(self.thread) // a ajouté plus tard self.thread_sleep(machine, Rc::clone(&thread)); machine.interrupt.set_status(old_status); diff --git a/src/utility/list.rs b/src/utility/list.rs index 9fb41bd..801a74e 100644 --- a/src/utility/list.rs +++ b/src/utility/list.rs @@ -108,13 +108,14 @@ impl List { /// Return true if the item has been found, otherwise return false /// /// Worst-case complexity is O(n) - pub fn remove(&mut self, item: &T)-> bool { + pub fn remove(&mut self, item: T)-> bool { unsafe { let mut current: *mut Node = self.head; let mut previous: *mut Node = ptr::null_mut(); while !current.is_null() { - if &(*current).elem == item { + if (*current).elem == item { (*previous).next = (*current).next; + drop(Box::from_raw(current).elem); return true; } else { previous = current; @@ -312,11 +313,11 @@ mod test { list.push(3); assert_eq!(list.contains(&2), true); - list.remove(&2); + list.remove(2); assert_eq!(list.contains(&2), false); assert_eq!(list.pop(), Option::Some(1)); assert_eq!(list.pop(), Option::Some(3)); - assert_eq!(list.pop(), Option::None); + assert_eq!(list.peek(), Option::None); } #[test]