vinzenz/redox
0
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

Converting file handles into a new type FileHandle

Browse source 
Keeping file handles (and pids, and scheme id, ...) as usize is a
footgun. Let's remove it.
This commit is contained in:
David Teller 2016-11-13 21:47:04 +01:00
parent 37a34ab7f7
commit 9c90a8fe42
7 changed files with 53 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
kernel/context/context.rs
View file

@ -6,6 +6,7 @@ use spin::Mutex;
use arch;
use context::file::File;
use context::memory::{Grant, Memory, SharedMemory, Tls};
use scheme::FileHandle;
use syscall::data::Event;
use sync::{WaitMap, WaitQueue};
@ -184,28 +185,28 @@ impl Context {
/// Add a file to the lowest available slot.
/// Return the file descriptor number or None if no slot was found
pub fn add_file(&self, file: File) -> Option<usize> {
pub fn add_file(&self, file: File) -> Option<FileHandle> {
let mut files = self.files.lock();
for (i, mut file_option) in files.iter_mut().enumerate() {
if file_option.is_none() {
*file_option = Some(file);
return Some(i);
return Some(FileHandle::from(i));
}
}
let len = files.len();
if len < super::CONTEXT_MAX_FILES {
files.push(Some(file));
Some(len)
Some(FileHandle::from(len))
} else {
None
}
}
/// Get a file
pub fn get_file(&self, i: usize) -> Option<File> {
pub fn get_file(&self, i: FileHandle) -> Option<File> {
let files = self.files.lock();
if i < files.len() {
files[i]
if i.into() < files.len() {
files[i.into()]
} else {
None
}
@ -213,10 +214,10 @@ impl Context {
/// Remove a file
// TODO: adjust files vector to smaller size if possible
pub fn remove_file(&self, i: usize) -> Option<File> {
pub fn remove_file(&self, i: FileHandle) -> Option<File> {
let mut files = self.files.lock();
if i < files.len() {
files[i].take()
if i.into() < files.len() {
files[i.into()].take()
} else {
None
}

10
kernel/context/event.rs
View file

@ -3,7 +3,7 @@ use collections::BTreeMap;
use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard};
use context;
use scheme::SchemeId;
use scheme::{FileHandle, SchemeId};
use sync::WaitQueue;
use syscall::data::Event;
@ -18,7 +18,7 @@ pub struct RegKey {
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub struct ProcessKey {
context_id: context::context::ContextId,
fd: usize,
fd: FileHandle,
}
type Registry = BTreeMap<RegKey, BTreeMap<ProcessKey, EventList>>;
@ -40,7 +40,7 @@ pub fn registry_mut() -> RwLockWriteGuard<'static, Registry> {
REGISTRY.call_once(init_registry).write()
}
pub fn register(fd: usize, scheme_id: SchemeId, event_id: usize) -> bool {
pub fn register(fd: FileHandle, 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");
@ -67,7 +67,7 @@ pub fn register(fd: usize, scheme_id: SchemeId, event_id: usize) -> bool {
}
}
pub fn unregister(fd: usize, scheme_id: SchemeId, event_id: usize) {
pub fn unregister(fd: FileHandle, scheme_id: SchemeId, event_id: usize) {
let mut registry = registry_mut();
let mut remove = false;
@ -102,7 +102,7 @@ pub fn trigger(scheme_id: SchemeId, event_id: usize, flags: usize, data: usize)
for entry in event_lists.iter() {
if let Some(event_list) = entry.1.upgrade() {
event_list.send(Event {
id: (entry.0).fd,
id: (entry.0).fd.into(),
flags: flags,
data: data
});

7
kernel/lib.rs
View file

@ -43,6 +43,7 @@ extern crate goblin;
extern crate spin;
use core::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
use scheme::FileHandle;
#[macro_use]
#[macro_export]
@ -93,9 +94,9 @@ pub fn cpu_count() -> usize {
pub extern fn userspace_init() {
assert_eq!(syscall::chdir(b"initfs:bin"), Ok(0));
assert_eq!(syscall::open(b"debug:", 0), Ok(0));
assert_eq!(syscall::open(b"debug:", 0), Ok(1));
assert_eq!(syscall::open(b"debug:", 0), Ok(2));
assert_eq!(syscall::open(b"debug:", 0).map(FileHandle::into), Ok(0));
assert_eq!(syscall::open(b"debug:", 0).map(FileHandle::into), Ok(1));
assert_eq!(syscall::open(b"debug:", 0).map(FileHandle::into), Ok(2));
syscall::exec(b"initfs:bin/init", &[]).expect("failed to execute initfs:init");

4
kernel/scheme/mod.rs
View file

@ -67,6 +67,10 @@ int_like!(SchemeId, AtomicSchemeId, usize, AtomicUsize);
pub const ATOMIC_SCHEMEID_INIT: AtomicSchemeId = AtomicSchemeId::default();
/// Unique identifier for a file descriptor.
int_like!(FileHandle, AtomicFileHandle, usize, AtomicUsize);
/// Scheme list type
pub struct SchemeList {
map: BTreeMap<SchemeId, Arc<Box<Scheme + Send + Sync>>>,

26
kernel/syscall/fs.rs
View file

@ -2,13 +2,13 @@
use core::sync::atomic::Ordering;
use context;
use scheme;
use scheme::{self, FileHandle};
use syscall;
use syscall::data::{Packet, Stat};
use syscall::error::*;
use syscall::flag::{MODE_DIR, MODE_FILE};
pub fn file_op(a: usize, fd: usize, c: usize, d: usize) -> Result<usize> {
pub fn file_op(a: usize, fd: FileHandle, c: usize, d: usize) -> Result<usize> {
let (file, pid, uid, gid) = {
let contexts = context::contexts();
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
@ -39,11 +39,11 @@ pub fn file_op(a: usize, fd: usize, c: usize, d: usize) -> Result<usize> {
Error::demux(packet.a)
}
pub fn file_op_slice(a: usize, fd: usize, slice: &[u8]) -> Result<usize> {
pub fn file_op_slice(a: usize, fd: FileHandle, slice: &[u8]) -> Result<usize> {
file_op(a, fd, slice.as_ptr() as usize, slice.len())
}
pub fn file_op_mut_slice(a: usize, fd: usize, slice: &mut [u8]) -> Result<usize> {
pub fn file_op_mut_slice(a: usize, fd: FileHandle, slice: &mut [u8]) -> Result<usize> {
file_op(a, fd, slice.as_mut_ptr() as usize, slice.len())
}
@ -81,7 +81,7 @@ pub fn getcwd(buf: &mut [u8]) -> Result<usize> {
}
/// Open syscall
pub fn open(path: &[u8], flags: usize) -> Result<usize> {
pub fn open(path: &[u8], flags: usize) -> Result<FileHandle> {
let (path_canon, uid, gid) = {
let contexts = context::contexts();
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
@ -135,8 +135,8 @@ pub fn pipe2(fds: &mut [usize], flags: usize) -> Result<usize> {
event: None,
}).ok_or(Error::new(EMFILE))?;
fds[0] = read_fd;
fds[1] = write_fd;
fds[0] = read_fd.into();
fds[1] = write_fd.into();
Ok(0)
} else {
@ -211,7 +211,7 @@ pub fn unlink(path: &[u8]) -> Result<usize> {
}
/// Close syscall
pub fn close(fd: usize) -> Result<usize> {
pub fn close(fd: FileHandle) -> Result<usize> {
let file = {
let contexts = context::contexts();
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
@ -233,7 +233,7 @@ pub fn close(fd: usize) -> Result<usize> {
}
/// Duplicate file descriptor
pub fn dup(fd: usize, buf: &[u8]) -> Result<usize> {
pub fn dup(fd: FileHandle, buf: &[u8]) -> Result<FileHandle> {
let file = {
let contexts = context::contexts();
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
@ -262,15 +262,15 @@ pub fn dup(fd: usize, buf: &[u8]) -> Result<usize> {
}
/// Register events for file
pub fn fevent(fd: usize, flags: usize) -> Result<usize> {
pub fn fevent(fd: FileHandle, flags: usize) -> Result<usize> {
let file = {
let contexts = context::contexts();
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
let context = context_lock.read();
let mut files = context.files.lock();
let mut file = files.get_mut(fd).ok_or(Error::new(EBADF))?.ok_or(Error::new(EBADF))?;
let mut file = files.get_mut(fd.into()).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()
@ -287,7 +287,7 @@ pub fn fevent(fd: usize, flags: usize) -> Result<usize> {
let context_lock = contexts.current().ok_or(Error::new(ESRCH))?;
let context = context_lock.read();
let mut files = context.files.lock();
let mut file = files.get_mut(fd).ok_or(Error::new(EBADF))?.ok_or(Error::new(EBADF))?;
let mut file = files.get_mut(fd.into()).ok_or(Error::new(EBADF))?.ok_or(Error::new(EBADF))?;
file.event = Some(event_id);
}
context::event::register(fd, file.scheme, event_id);

24
kernel/syscall/mod.rs
View file

@ -15,6 +15,7 @@ use self::error::{Error, Result, ENOSYS};
use self::number::*;
use context::ContextId;
use scheme::FileHandle;
/// Filesystem syscalls
pub mod fs;
@ -36,19 +37,22 @@ pub extern fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize
#[inline(always)]
fn inner(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, stack: usize) -> Result<usize> {
match a & SYS_CLASS {
SYS_CLASS_FILE => match a & SYS_ARG {
SYS_ARG_SLICE => file_op_slice(a, b, validate_slice(c as *const u8, d)?),
SYS_ARG_MSLICE => file_op_mut_slice(a, b, validate_slice_mut(c as *mut u8, d)?),
_ => match a {
SYS_CLOSE => close(b),
SYS_DUP => dup(b, validate_slice(c as *const u8, d)?),
SYS_FEVENT => fevent(b, c),
SYS_FUNMAP => funmap(b),
_ => file_op(a, b, c, d)
SYS_CLASS_FILE => {
let fd = FileHandle::from(b);
match a & SYS_ARG {
SYS_ARG_SLICE => file_op_slice(a, fd, validate_slice(c as *const u8, d)?),
SYS_ARG_MSLICE => file_op_mut_slice(a, fd, validate_slice_mut(c as *mut u8, d)?),
_ => match a {
SYS_CLOSE => close(fd),
SYS_DUP => dup(fd, validate_slice(c as *const u8, d)?).map(FileHandle::into),
SYS_FEVENT => fevent(fd, c),
SYS_FUNMAP => funmap(b),
_ => file_op(a, fd, c, d)
}
}
},
SYS_CLASS_PATH => match a {
SYS_OPEN => open(validate_slice(b as *const u8, c)?, d),
SYS_OPEN => open(validate_slice(b as *const u8, c)?, d).map(FileHandle::into),
SYS_MKDIR => mkdir(validate_slice(b as *const u8, c)?, d as u16),
SYS_RMDIR => rmdir(validate_slice(b as *const u8, c)?),
SYS_UNLINK => unlink(validate_slice(b as *const u8, c)?),

6
kernel/syscall/process.rs
View file

@ -16,7 +16,7 @@ use context;
use context::ContextId;
use context::memory::Grant;
use elf::{self, program_header};
use scheme;
use scheme::{self, FileHandle};
use syscall;
use syscall::data::Stat;
use syscall::error::*;
@ -695,7 +695,7 @@ pub fn exit(status: usize) -> ! {
let mut close_files = Vec::new();
let (pid, ppid) = {
let mut context = context_lock.write();
if Arc::strong_count(&context.files) == 1 {
if Arc::strong_count(&context.files) == 1 { // FIXME: Looks like a race condition.
mem::swap(context.files.lock().deref_mut(), &mut close_files);
}
context.files = Arc::new(Mutex::new(Vec::new()));
@ -706,7 +706,7 @@ pub fn exit(status: usize) -> ! {
for (fd, file_option) in close_files.drain(..).enumerate() {
if let Some(file) = file_option {
if let Some(event_id) = file.event {
context::event::unregister(fd, file.scheme, event_id);
context::event::unregister(FileHandle::from(fd), file.scheme, event_id);
}
let scheme_option = {