Skip to content

Commit

Permalink
Merge #231
Browse files Browse the repository at this point in the history
231: Remove `Allowed::replace` and `Allowed::take`. r=jrvanwhy a=jrvanwhy

When I originally implemented `Allowed`, I made it work with non-`Copy` types. `Allowed::get` cannot with with non-`Copy` types, so I added `Allowed::replace` to allow client code to read a non-`Copy` type out of an `Allowed` buffer. However, during the PR review we changed `Allowed` to only contain `Copy` types. Because of this change, I don't anticipate `Allowed::replace` (and `Allowed::take` which is built on top of `Allowed::replace`) seeing much use. This PR removes `Allowed::replace` and `Allowed::take` to reduce the amount of `unsafe` in `libtock_platform`.

Co-authored-by: Johnathan Van Why <jrvanwhy@google.com>
  • Loading branch information
bors[bot] and jrvanwhy authored Aug 19, 2020
2 parents 4a584df + 9e00ef6 commit d399b9d
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 40 deletions.
14 changes: 0 additions & 14 deletions core/platform/src/allows/allowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,7 @@ impl<'b, T: Copy + 'b> Allowed<'b, T> {
}

impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> {
pub fn replace(&self, value: T) -> T {
let current = unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) };
unsafe {
core::ptr::write_volatile(self.buffer.as_ptr(), value);
}
current
}

pub fn get(&self) -> T {
unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) }
}
}

impl<'b, T: crate::AllowReadable + Copy + Default + 'b> Allowed<'b, T> {
pub fn take(&self) -> T {
self.replace(T::default())
}
}
26 changes: 0 additions & 26 deletions core/platform/src/allows/allowed_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,6 @@ fn set() {
assert_eq!(kernel_ptr.get(), 3);
}

#[test]
fn replace() {
let mut buffer = 1;
let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer);
assert_eq!(kernel_ptr.get(), 1);

// Simulate the kernel replacing the value in buffer.
kernel_ptr.set(2);
let returned = allowed.replace(3);
assert_eq!(returned, 2);
assert_eq!(kernel_ptr.get(), 3);
}

#[test]
fn get() {
let mut buffer = 1;
Expand All @@ -109,16 +96,3 @@ fn get() {
assert_eq!(allowed.get(), 2);
assert_eq!(kernel_ptr.get(), 2);
}

#[test]
fn take() {
let mut buffer = 1;
let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer);
assert_eq!(kernel_ptr.get(), 1);

// Simulate the kernel replacing the value in buffer.
kernel_ptr.set(2);
let returned = allowed.take();
assert_eq!(returned, 2);
assert_eq!(kernel_ptr.get(), 0);
}

0 comments on commit d399b9d

Please sign in to comment.