Replace UnsafeCell in DateServiceInner with Cell

The previous API was extremely dangerous - calling `get_ref()`
followed by `reset()` would trigger instant UB, without requiring
any `unsafe` blocks in the caller.

By making DateInner `Copy`, we can use a normal `Cell` instead
of an `UnsafeCell`. This makes it impossible to cause UB (or even panic)
with the API.
This commit is contained in:
Aaron Hill 2019-07-09 21:01:27 -04:00
parent f410f3330f
commit 50397fb63b
No known key found for this signature in database
GPG Key ID: B4087E510E98B164
1 changed files with 12 additions and 12 deletions

View File

@ -1,4 +1,4 @@
use std::cell::UnsafeCell; use std::cell::Cell;
use std::fmt; use std::fmt;
use std::fmt::Write; use std::fmt::Write;
use std::rc::Rc; use std::rc::Rc;
@ -172,6 +172,7 @@ impl ServiceConfig {
} }
} }
#[derive(Copy, Clone)]
struct Date { struct Date {
bytes: [u8; DATE_VALUE_LENGTH], bytes: [u8; DATE_VALUE_LENGTH],
pos: usize, pos: usize,
@ -205,28 +206,28 @@ impl fmt::Write for Date {
struct DateService(Rc<DateServiceInner>); struct DateService(Rc<DateServiceInner>);
struct DateServiceInner { struct DateServiceInner {
current: UnsafeCell<Option<(Date, Instant)>>, current: Cell<Option<(Date, Instant)>>,
} }
impl DateServiceInner { impl DateServiceInner {
fn new() -> Self { fn new() -> Self {
DateServiceInner { DateServiceInner {
current: UnsafeCell::new(None), current: Cell::new(None),
} }
} }
fn get_ref(&self) -> &Option<(Date, Instant)> { fn get(&self) -> Option<(Date, Instant)> {
unsafe { &*self.current.get() } self.current.get()
} }
fn reset(&self) { fn reset(&self) {
unsafe { (&mut *self.current.get()).take() }; self.current.set(None);
} }
fn update(&self) { fn update(&self) {
let now = Instant::now(); let now = Instant::now();
let date = Date::new(); let date = Date::new();
*(unsafe { &mut *self.current.get() }) = Some((date, now)); self.current.set(Some((date, now)));
} }
} }
@ -236,7 +237,7 @@ impl DateService {
} }
fn check_date(&self) { fn check_date(&self) {
if self.0.get_ref().is_none() { if self.0.get().is_none() {
self.0.update(); self.0.update();
// periodic date update // periodic date update
@ -252,14 +253,13 @@ impl DateService {
fn now(&self) -> Instant { fn now(&self) -> Instant {
self.check_date(); self.check_date();
self.0.get_ref().as_ref().unwrap().1 self.0.get().unwrap().1
} }
fn date(&self) -> &Date { fn date(&self) -> Date {
self.check_date(); self.check_date();
let item = self.0.get_ref().as_ref().unwrap(); self.0.get().unwrap().0
&item.0
} }
} }