From d16515ea122b3126f67422289474912265598354 Mon Sep 17 00:00:00 2001 From: David Teller Date: Fri, 4 Nov 2016 13:49:44 +0100 Subject: [PATCH] Converting scheme ids into a new type SchemeId Keeping scheme ids (and pids, and file handles, ...) as usize is a footgun. Let's remove it. --- kernel/context/event.rs | 9 +++++---- kernel/context/file.rs | 4 +++- kernel/scheme/debug.rs | 5 +++-- kernel/scheme/irq.rs | 5 +++-- kernel/scheme/mod.rs | 26 +++++++++++++++----------- kernel/scheme/pipe.rs | 3 ++- kernel/scheme/root.rs | 6 +++--- kernel/scheme/user.rs | 7 ++++--- kernel/syscall/fs.rs | 2 +- kernel/syscall/process.rs | 1 + 10 files changed, 40 insertions(+), 28 deletions(-) diff --git a/kernel/context/event.rs b/kernel/context/event.rs index 2ece466..b41a692 100644 --- a/kernel/context/event.rs +++ b/kernel/context/event.rs @@ -3,6 +3,7 @@ use collections::BTreeMap; use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; use context; +use scheme::SchemeId; use sync::WaitQueue; use syscall::data::Event; @@ -10,7 +11,7 @@ type EventList = Weak>; #[derive(PartialEq, Eq, PartialOrd, Ord)] pub struct RegKey { - scheme_id: usize, + scheme_id: SchemeId, event_id: usize, } @@ -39,7 +40,7 @@ pub fn registry_mut() -> RwLockWriteGuard<'static, Registry> { REGISTRY.call_once(init_registry).write() } -pub fn register(fd: usize, scheme_id: usize, event_id: usize) -> bool { +pub fn register(fd: usize, scheme_id: SchemeId, event_id: usize) -> bool { let (context_id, events) = { let contexts = context::contexts(); let context_lock = contexts.current().expect("event::register: No context"); @@ -66,7 +67,7 @@ pub fn register(fd: usize, scheme_id: usize, event_id: usize) -> bool { } } -pub fn unregister(fd: usize, scheme_id: usize, event_id: usize) { +pub fn unregister(fd: usize, scheme_id: SchemeId, event_id: usize) { let mut registry = registry_mut(); let mut remove = false; @@ -91,7 +92,7 @@ pub fn unregister(fd: usize, scheme_id: usize, event_id: usize) { } } -pub fn trigger(scheme_id: usize, event_id: usize, flags: usize, data: usize) { +pub fn trigger(scheme_id: SchemeId, event_id: usize, flags: usize, data: usize) { let registry = registry(); let key = RegKey { scheme_id: scheme_id, diff --git a/kernel/context/file.rs b/kernel/context/file.rs index d9238d6..26e0863 100644 --- a/kernel/context/file.rs +++ b/kernel/context/file.rs @@ -1,11 +1,13 @@ //! File struct +use scheme::SchemeId; + /// A file //TODO: Close on exec #[derive(Copy, Clone, Debug)] pub struct File { /// The scheme that this file refers to - pub scheme: usize, + pub scheme: SchemeId, /// The number the scheme uses to refer to this file pub number: usize, /// If events are on, this is the event ID diff --git a/kernel/scheme/debug.rs b/kernel/scheme/debug.rs index 8a8f556..f4b88db 100644 --- a/kernel/scheme/debug.rs +++ b/kernel/scheme/debug.rs @@ -1,14 +1,15 @@ use core::str; -use core::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; +use core::sync::atomic::Ordering; use spin::Once; use context; +use scheme::*; use sync::WaitQueue; use syscall::error::*; use syscall::flag::EVENT_READ; use syscall::scheme::Scheme; -pub static DEBUG_SCHEME_ID: AtomicUsize = ATOMIC_USIZE_INIT; +pub static DEBUG_SCHEME_ID: AtomicSchemeId = ATOMIC_SCHEMEID_INIT; /// Input queue static INPUT: Once> = Once::new(); diff --git a/kernel/scheme/irq.rs b/kernel/scheme/irq.rs index a20aca7..bde521d 100644 --- a/kernel/scheme/irq.rs +++ b/kernel/scheme/irq.rs @@ -1,14 +1,15 @@ use core::{mem, str}; -use core::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; +use core::sync::atomic::Ordering; use spin::Mutex; use arch::interrupt::irq::acknowledge; use context; +use scheme::{AtomicSchemeId, ATOMIC_SCHEMEID_INIT}; use syscall::error::*; use syscall::flag::EVENT_READ; use syscall::scheme::Scheme; -pub static IRQ_SCHEME_ID: AtomicUsize = ATOMIC_USIZE_INIT; +pub static IRQ_SCHEME_ID: AtomicSchemeId = ATOMIC_SCHEMEID_INIT; /// IRQ queues static ACKS: Mutex<[usize; 16]> = Mutex::new([0; 16]); diff --git a/kernel/scheme/mod.rs b/kernel/scheme/mod.rs index 58aaa8e..45a9ae2 100644 --- a/kernel/scheme/mod.rs +++ b/kernel/scheme/mod.rs @@ -9,7 +9,7 @@ use alloc::arc::Arc; use alloc::boxed::Box; use collections::BTreeMap; -use core::sync::atomic::Ordering; +use core::sync::atomic::{AtomicUsize, Ordering}; use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; use syscall::error::*; @@ -62,10 +62,15 @@ pub mod zero; /// Limit on number of schemes pub const SCHEME_MAX_SCHEMES: usize = 65536; +/// Unique identifier for a scheme. +int_like!(SchemeId, AtomicSchemeId, usize, AtomicUsize); + +pub const ATOMIC_SCHEMEID_INIT: AtomicSchemeId = AtomicSchemeId::default(); + /// Scheme list type pub struct SchemeList { - map: BTreeMap>>, - names: BTreeMap, usize>, + map: BTreeMap>>, + names: BTreeMap, SchemeId>, next_id: usize } @@ -79,20 +84,20 @@ impl SchemeList { } } - pub fn iter(&self) -> ::collections::btree_map::Iter>> { + pub fn iter(&self) -> ::collections::btree_map::Iter>> { self.map.iter() } - pub fn iter_name(&self) -> ::collections::btree_map::Iter, usize> { + pub fn iter_name(&self) -> ::collections::btree_map::Iter, SchemeId> { self.names.iter() } /// Get the nth scheme. - pub fn get(&self, id: usize) -> Option<&Arc>> { + pub fn get(&self, id: SchemeId) -> Option<&Arc>> { self.map.get(&id) } - pub fn get_name(&self, name: &[u8]) -> Option<(usize, &Arc>)> { + pub fn get_name(&self, name: &[u8]) -> Option<(SchemeId, &Arc>)> { if let Some(&id) = self.names.get(name) { self.get(id).map(|scheme| (id, scheme)) } else { @@ -101,7 +106,7 @@ impl SchemeList { } /// Create a new scheme. - pub fn insert(&mut self, name: Box<[u8]>, scheme: Arc>) -> Result { + pub fn insert(&mut self, name: Box<[u8]>, scheme: Arc>) -> Result { if self.names.contains_key(&name) { return Err(Error::new(EEXIST)); } @@ -110,7 +115,7 @@ impl SchemeList { self.next_id = 1; } - while self.map.contains_key(&self.next_id) { + while self.map.contains_key(&SchemeId(self.next_id)) { self.next_id += 1; } @@ -118,12 +123,11 @@ impl SchemeList { return Err(Error::new(EAGAIN)); } - let id = self.next_id; + let id = SchemeId(self.next_id); self.next_id += 1; assert!(self.map.insert(id, scheme).is_none()); assert!(self.names.insert(name, id).is_none()); - Ok(id) } } diff --git a/kernel/scheme/pipe.rs b/kernel/scheme/pipe.rs index 49d4895..e4568db 100644 --- a/kernel/scheme/pipe.rs +++ b/kernel/scheme/pipe.rs @@ -2,6 +2,7 @@ use alloc::arc::{Arc, Weak}; use collections::{BTreeMap, VecDeque}; use core::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; use spin::{Mutex, Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use scheme::{AtomicSchemeId, ATOMIC_SCHEMEID_INIT}; use sync::WaitCondition; use syscall::error::{Error, Result, EBADF, EPIPE}; @@ -9,7 +10,7 @@ use syscall::flag::O_NONBLOCK; use syscall::scheme::Scheme; /// Pipes list -pub static PIPE_SCHEME_ID: AtomicUsize = ATOMIC_USIZE_INIT; +pub static PIPE_SCHEME_ID: AtomicSchemeId = ATOMIC_SCHEMEID_INIT; static PIPE_NEXT_ID: AtomicUsize = ATOMIC_USIZE_INIT; static PIPES: Once, BTreeMap)>> = Once::new(); diff --git a/kernel/scheme/root.rs b/kernel/scheme/root.rs index 737e74b..bc1ec42 100644 --- a/kernel/scheme/root.rs +++ b/kernel/scheme/root.rs @@ -1,16 +1,16 @@ use alloc::arc::Arc; use alloc::boxed::Box; use collections::BTreeMap; -use core::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; +use core::sync::atomic::{AtomicUsize, Ordering}; use spin::RwLock; use context; use syscall::error::*; use syscall::scheme::Scheme; -use scheme; +use scheme::{self, AtomicSchemeId, ATOMIC_SCHEMEID_INIT}; use scheme::user::{UserInner, UserScheme}; -pub static ROOT_SCHEME_ID: AtomicUsize = ATOMIC_USIZE_INIT; +pub static ROOT_SCHEME_ID: AtomicSchemeId = ATOMIC_SCHEMEID_INIT; pub struct RootScheme { next_id: AtomicUsize, diff --git a/kernel/scheme/user.rs b/kernel/scheme/user.rs index ef2bd22..c918d9e 100644 --- a/kernel/scheme/user.rs +++ b/kernel/scheme/user.rs @@ -1,6 +1,6 @@ use alloc::arc::{Arc, Weak}; use collections::BTreeMap; -use core::sync::atomic::{AtomicUsize, AtomicU64, Ordering}; +use core::sync::atomic::{AtomicU64, Ordering}; use core::{mem, slice, usize}; use spin::{Mutex, RwLock}; @@ -10,6 +10,7 @@ use arch::paging::temporary_page::TemporaryPage; use context::{self, Context}; use context::memory::Grant; use scheme::root::ROOT_SCHEME_ID; +use scheme::{AtomicSchemeId, ATOMIC_SCHEMEID_INIT}; use sync::{WaitQueue, WaitMap}; use syscall::data::{Packet, Stat}; use syscall::error::*; @@ -20,7 +21,7 @@ use syscall::scheme::Scheme; pub struct UserInner { handle_id: usize, flags: usize, - pub scheme_id: AtomicUsize, + pub scheme_id: AtomicSchemeId, next_id: AtomicU64, context: Weak>, todo: WaitQueue, @@ -33,7 +34,7 @@ impl UserInner { UserInner { handle_id: handle_id, flags: flags, - scheme_id: AtomicUsize::new(0), + scheme_id: ATOMIC_SCHEMEID_INIT, next_id: AtomicU64::new(1), context: context, todo: WaitQueue::new(), diff --git a/kernel/syscall/fs.rs b/kernel/syscall/fs.rs index c40ec8d..9aae871 100644 --- a/kernel/syscall/fs.rs +++ b/kernel/syscall/fs.rs @@ -270,7 +270,7 @@ pub fn fevent(fd: usize, flags: usize) -> Result { let mut files = context.files.lock(); let mut file = files.get_mut(fd).ok_or(Error::new(EBADF))?.ok_or(Error::new(EBADF))?; if let Some(event_id) = file.event.take() { - println!("{}: {}:{}: events already registered: {}", fd, file.scheme, file.number, event_id); + println!("{}: {:?}:{}: events already registered: {}", fd, file.scheme, file.number, event_id); context::event::unregister(fd, file.scheme, event_id); } file.clone() diff --git a/kernel/syscall/process.rs b/kernel/syscall/process.rs index 87053af..9a65f6b 100644 --- a/kernel/syscall/process.rs +++ b/kernel/syscall/process.rs @@ -1062,3 +1062,4 @@ pub fn waitpid(pid: usize, status_ptr: usize, flags: usize) -> Result { } } } +