From 3756fb5606089a97d0a9ca1ff7d736019d502878 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 28 Jul 2017 13:59:31 -0700 Subject: [PATCH] Use file description alongside file descriptior, matching posix --- src/context/context.rs | 12 +-- src/context/file.rs | 46 ++++++++-- src/scheme/sys/iostat.rs | 12 +-- src/syscall/fs.rs | 184 ++++++++++++++++++++++++--------------- src/syscall/process.rs | 117 +++++-------------------- 5 files changed, 191 insertions(+), 180 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 5bd18f5..56e3e10 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -5,7 +5,7 @@ use core::mem; use spin::Mutex; use context::arch; -use context::file::File; +use context::file::FileDescriptor; use context::memory::{Grant, Memory, SharedMemory, Tls}; use device; use scheme::{SchemeNamespace, FileHandle}; @@ -92,7 +92,7 @@ pub struct Context { /// The process environment pub env: Arc, Arc>>>>>, /// The open files in the scheme - pub files: Arc>>>, + pub files: Arc>>>, /// Singal actions pub actions: Arc>>, } @@ -243,7 +243,7 @@ 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 { + pub fn add_file(&self, file: FileDescriptor) -> Option { let mut files = self.files.lock(); for (i, mut file_option) in files.iter_mut().enumerate() { if file_option.is_none() { @@ -261,7 +261,7 @@ impl Context { } /// Get a file - pub fn get_file(&self, i: FileHandle) -> Option { + pub fn get_file(&self, i: FileHandle) -> Option { let files = self.files.lock(); if i.into() < files.len() { files[i.into()].clone() @@ -272,7 +272,7 @@ impl Context { /// Insert a file with a specific handle number. This is used by dup2 /// Return the file descriptor number or None if the slot was not empty, or i was invalid - pub fn insert_file(&self, i: FileHandle, file: File) -> Option { + pub fn insert_file(&self, i: FileHandle, file: FileDescriptor) -> Option { let mut files = self.files.lock(); if i.into() < super::CONTEXT_MAX_FILES { while i.into() >= files.len() { @@ -291,7 +291,7 @@ impl Context { /// Remove a file // TODO: adjust files vector to smaller size if possible - pub fn remove_file(&self, i: FileHandle) -> Option { + pub fn remove_file(&self, i: FileHandle) -> Option { let mut files = self.files.lock(); if i.into() < files.len() { files[i.into()].take() diff --git a/src/context/file.rs b/src/context/file.rs index 4fea00f..a5fd277 100644 --- a/src/context/file.rs +++ b/src/context/file.rs @@ -1,17 +1,51 @@ -//! File struct +//! File structs -use scheme::SchemeId; +use alloc::arc::Arc; +use spin::RwLock; +use scheme::{self, SchemeId}; +use core::mem; +use syscall::error::{Result, Error, EBADF}; +use scheme::FileHandle; +use context; -/// A file -//TODO: Close on exec -#[derive(Clone, Debug)] -pub struct File { +/// A file description +#[derive(Debug)] +pub struct FileDescription { /// The scheme that this file refers to pub scheme: SchemeId, /// The number the scheme uses to refer to this file pub number: usize, /// The flags passed to open or fcntl(SETFL) pub flags: usize, +} + +/// A file descriptor +#[derive(Clone, Debug)] +pub struct FileDescriptor { + /// Corresponding file description + pub description: Arc>, /// If events are on, this is the event ID pub event: Option, + /// Cloexec flag + pub cloexec: bool, +} + +impl FileDescriptor { + pub fn close(self, fd: FileHandle) -> Result { + if let Some(event_id) = self.event { + context::event::unregister(fd, self.description.read().scheme, event_id); + } + + if let Ok(file) = Arc::try_unwrap(self.description) { + let file = file.into_inner(); + let scheme = { + let schemes = scheme::schemes(); + let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; + scheme.clone() + }; + scheme.close(file.number) + } else { + Ok(0) + } + } } diff --git a/src/scheme/sys/iostat.rs b/src/scheme/sys/iostat.rs index 1f2610b..c365acd 100644 --- a/src/scheme/sys/iostat.rs +++ b/src/scheme/sys/iostat.rs @@ -31,25 +31,27 @@ pub fn resource() -> Result> { Some(ref file) => file.clone() }; + let description = file.description.read(); + let scheme = { let schemes = scheme::schemes(); - match schemes.get(file.scheme) { + match schemes.get(description.scheme) { Some(scheme) => scheme.clone(), None => { - let _ = writeln!(string, " {:>4}: {:>8} {:>8} {:>08X}: no scheme", fd, file.scheme.into(), file.number, file.flags); + let _ = writeln!(string, " {:>4}: {:>8} {:>8} {:>08X}: no scheme", fd, description.scheme.into(), description.number, description.flags); continue; } } }; let mut fpath = [0; 4096]; - match scheme.fpath(file.number, &mut fpath) { + match scheme.fpath(description.number, &mut fpath) { Ok(path_len) => { let fname = str::from_utf8(&fpath[..path_len]).unwrap_or("?"); - let _ = writeln!(string, "{:>6}: {:>8} {:>8} {:>08X}: {}", fd, file.scheme.into(), file.number, file.flags, fname); + let _ = writeln!(string, "{:>6}: {:>8} {:>8} {:>08X}: {}", fd, description.scheme.into(), description.number, description.flags, fname); }, Err(err) => { - let _ = writeln!(string, "{:>6}: {:>8} {:>8} {:>08X}: {}", fd, file.scheme.into(), file.number, file.flags, err); + let _ = writeln!(string, "{:>6}: {:>8} {:>8} {:>08X}: {}", fd, description.scheme.into(), description.number, description.flags, err); } } } diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index 5388c0a..5c693bd 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -1,5 +1,7 @@ //! Filesystem syscalls use core::sync::atomic::Ordering; +use alloc::arc::Arc; +use spin::RwLock; use context; use scheme::{self, FileHandle}; @@ -7,6 +9,7 @@ use syscall; use syscall::data::{Packet, Stat}; use syscall::error::*; use syscall::flag::{F_GETFL, F_SETFL, O_ACCMODE, O_RDONLY, O_WRONLY, MODE_DIR, MODE_FILE, O_CLOEXEC}; +use context::file::{FileDescriptor, FileDescription}; pub fn file_op(a: usize, fd: FileHandle, c: usize, d: usize) -> Result { let (file, pid, uid, gid) = { @@ -19,7 +22,7 @@ pub fn file_op(a: usize, fd: FileHandle, c: usize, d: usize) -> Result { let scheme = { let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; + let scheme = schemes.get(file.description.read().scheme).ok_or(Error::new(EBADF))?; scheme.clone() }; @@ -29,7 +32,7 @@ pub fn file_op(a: usize, fd: FileHandle, c: usize, d: usize) -> Result { uid: uid, gid: gid, a: a, - b: file.number, + b: file.description.read().number, c: c, d: d }; @@ -109,11 +112,14 @@ pub fn open(path: &[u8], flags: usize) -> Result { let contexts = context::contexts(); let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let context = context_lock.read(); - context.add_file(::context::file::File { - scheme: scheme_id, - number: file_id, - flags: flags, + context.add_file(FileDescriptor { + description: Arc::new(RwLock::new(FileDescription { + scheme: scheme_id, + number: file_id, + flags: flags & !O_CLOEXEC, + })), event: None, + cloexec: flags & O_CLOEXEC == O_CLOEXEC, }).ok_or(Error::new(EMFILE)) } @@ -126,18 +132,24 @@ pub fn pipe2(fds: &mut [usize], flags: usize) -> Result { let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let context = context_lock.read(); - let read_fd = context.add_file(::context::file::File { - scheme: scheme_id, - number: read_id, - flags: O_RDONLY | flags & !O_ACCMODE, + let read_fd = context.add_file(FileDescriptor { + description: Arc::new(RwLock::new(FileDescription { + scheme: scheme_id, + number: read_id, + flags: O_RDONLY | flags & !O_ACCMODE & !O_CLOEXEC, + })), event: None, + cloexec: flags & O_CLOEXEC == O_CLOEXEC, }).ok_or(Error::new(EMFILE))?; - let write_fd = context.add_file(::context::file::File { - scheme: scheme_id, - number: write_id, - flags: O_WRONLY | flags & !O_ACCMODE, + let write_fd = context.add_file(FileDescriptor { + description: Arc::new(RwLock::new(FileDescription { + scheme: scheme_id, + number: write_id, + flags: O_WRONLY | flags & !O_ACCMODE & !O_CLOEXEC, + })), event: None, + cloexec: flags & O_CLOEXEC == O_CLOEXEC, }).ok_or(Error::new(EMFILE))?; fds[0] = read_fd.into(); @@ -225,16 +237,7 @@ pub fn close(fd: FileHandle) -> Result { file }; - if let Some(event_id) = file.event { - context::event::unregister(fd, file.scheme, event_id); - } - - let scheme = { - let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; - scheme.clone() - }; - scheme.close(file.number) + file.close(fd) } /// Duplicate file descriptor @@ -247,24 +250,43 @@ pub fn dup(fd: FileHandle, buf: &[u8]) -> Result { file }; - let new_id = { - let scheme = { - let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; - scheme.clone() - }; - scheme.dup(file.number, buf)? - }; + if buf.is_empty() { + let contexts = context::contexts(); + let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; + let context = context_lock.read(); - let contexts = context::contexts(); - let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; - let context = context_lock.read(); - context.add_file(::context::file::File { - scheme: file.scheme, - number: new_id, - flags: file.flags & !O_CLOEXEC, - event: None, - }).ok_or(Error::new(EMFILE)) + context.add_file(FileDescriptor { + description: Arc::clone(&file.description), + event: None, + cloexec: false, + }).ok_or(Error::new(EMFILE)) + } else { + let description = file.description.read(); + + let new_id = { + let scheme = { + let schemes = scheme::schemes(); + let scheme = schemes.get(description.scheme).ok_or(Error::new(EBADF))?; + + scheme.clone() + }; + scheme.dup(description.number, buf)? + }; + + let contexts = context::contexts(); + let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; + let context = context_lock.read(); + + context.add_file(FileDescriptor { + description: Arc::new(RwLock::new(FileDescription { + scheme: description.scheme, + number: new_id, + flags: description.flags, + })), + event: None, + cloexec: false, + }).ok_or(Error::new(EMFILE)) + } } /// Duplicate file descriptor, replacing another @@ -278,28 +300,46 @@ pub fn dup2(fd: FileHandle, new_fd: FileHandle, buf: &[u8]) -> Result Result { file }; + let description = file.description.read(); + // Communicate fcntl with scheme let _res = { let scheme = { let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; + let scheme = schemes.get(description.scheme).ok_or(Error::new(EBADF))?; scheme.clone() }; - scheme.fcntl(file.number, cmd, arg)? + scheme.fcntl(description.number, cmd, arg)? }; // Perform kernel operation if scheme agrees @@ -332,11 +374,12 @@ pub fn fcntl(fd: FileHandle, cmd: usize, arg: usize) -> Result { match *files.get_mut(fd.into()).ok_or(Error::new(EBADF))? { Some(ref mut file) => match cmd { F_GETFL => { - Ok(file.flags) + Ok(description.flags) }, F_SETFL => { - let new_flags = (file.flags & O_ACCMODE) | (arg & ! O_ACCMODE); - file.flags = new_flags; + let new_flags = (description.flags & O_ACCMODE) | (arg & ! O_ACCMODE); + drop(description); + file.description.write().flags = new_flags; Ok(0) }, _ => { @@ -357,9 +400,10 @@ pub fn fevent(fd: FileHandle, flags: usize) -> Result { let mut files = context.files.lock(); match *files.get_mut(fd.into()).ok_or(Error::new(EBADF))? { Some(ref mut file) => { + let description = file.description.read(); if let Some(event_id) = file.event.take() { - println!("{:?}: {:?}:{}: events already registered: {}", fd, file.scheme, file.number, event_id); - context::event::unregister(fd, file.scheme, event_id); + println!("{:?}: {:?}:{}: events already registered: {}", fd, description.scheme, description.number, event_id); + context::event::unregister(fd, description.scheme, event_id); } file.clone() }, @@ -367,12 +411,14 @@ pub fn fevent(fd: FileHandle, flags: usize) -> Result { } }; + let description = file.description.read(); + let scheme = { let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; + let scheme = schemes.get(description.scheme).ok_or(Error::new(EBADF))?; scheme.clone() }; - let event_id = scheme.fevent(file.number, flags)?; + let event_id = scheme.fevent(description.number, flags)?; { let contexts = context::contexts(); let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; @@ -383,7 +429,7 @@ pub fn fevent(fd: FileHandle, flags: usize) -> Result { None => return Err(Error::new(EBADF)), } } - context::event::register(fd, file.scheme, event_id); + context::event::register(fd, description.scheme, event_id); Ok(0) } diff --git a/src/syscall/process.rs b/src/syscall/process.rs index c0c0e96..3823486 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -1,4 +1,3 @@ -///! Process syscalls use alloc::allocator::{Alloc, Layout}; use alloc::arc::Arc; use alloc::boxed::Box; @@ -15,6 +14,7 @@ use start::usermode; use interrupt; use context; use context::ContextId; +use context::file::FileDescriptor; use elf::{self, program_header}; use scheme::{self, FileHandle}; use syscall; @@ -284,27 +284,11 @@ pub fn clone(flags: usize, stack_base: usize) -> Result { if flags & CLONE_FILES == 0 { for (_fd, mut file_option) in files.lock().iter_mut().enumerate() { let new_file_option = if let Some(ref file) = *file_option { - let result = { - let scheme = { - let schemes = scheme::schemes(); - let scheme = schemes.get(file.scheme).ok_or(Error::new(EBADF))?; - scheme.clone() - }; - scheme.dup(file.number, b"") - }; - match result { - Ok(new_number) => { - Some(context::file::File { - scheme: file.scheme, - number: new_number, - flags: file.flags, - event: None, - }) - }, - Err(_err) => { - None - } - } + Some(FileDescriptor { + description: Arc::clone(&file.description), + event: None, + cloexec: file.cloexec, + }) } else { None }; @@ -617,7 +601,7 @@ pub fn exec(path: &[u8], arg_ptrs: &[[usize; 2]]) -> Result { drop(path); // Drop so that usage is not allowed after unmapping context drop(arg_ptrs); // Drop so that usage is not allowed after unmapping context - let (vfork, ppid, files) = { + let (vfork, ppid) = { let contexts = context::contexts(); let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let mut context = context_lock.write(); @@ -777,9 +761,6 @@ pub fn exec(path: &[u8], arg_ptrs: &[[usize; 2]]) -> Result { context.image.push(memory.to_shared()); } - let files = Arc::new(Mutex::new(context.files.lock().clone())); - context.files = files.clone(); - context.actions = Arc::new(Mutex::new(vec![( SigAction { sa_handler: unsafe { mem::transmute(SIG_DFL) }, @@ -791,65 +772,23 @@ pub fn exec(path: &[u8], arg_ptrs: &[[usize; 2]]) -> Result { let vfork = context.vfork; context.vfork = false; - (vfork, context.ppid, files) + + for (fd, file_option) in context.files.lock().iter_mut().enumerate() { + let mut cloexec = false; + if let Some(ref file) = *file_option { + if file.cloexec { + cloexec = true; + } + } + + if cloexec { + let _ = file_option.take().unwrap().close(FileHandle::from(fd)); + } + } + + (vfork, context.ppid) }; - // Duplicate current files, close previous - for (fd, mut file_option) in files.lock().iter_mut().enumerate() { - let new_file_option = if let Some(ref file) = *file_option { - // Duplicate - let result = { - if file.flags & O_CLOEXEC == O_CLOEXEC { - Err(Error::new(EBADF)) - } else { - let scheme_option = { - let schemes = scheme::schemes(); - schemes.get(file.scheme).map(|scheme| scheme.clone()) - }; - if let Some(scheme) = scheme_option { - scheme.dup(file.number, b"") - } else { - Err(Error::new(EBADF)) - } - } - }; - - // Close - { - if let Some(event_id) = file.event { - context::event::unregister(FileHandle::from(fd), file.scheme, event_id); - } - - let scheme_option = { - let schemes = scheme::schemes(); - schemes.get(file.scheme).map(|scheme| scheme.clone()) - }; - if let Some(scheme) = scheme_option { - let _ = scheme.close(file.number); - } - } - - // Return new descriptor - match result { - Ok(new_number) => { - Some(context::file::File { - scheme: file.scheme, - number: new_number, - flags: file.flags, - event: None, - }) - }, - Err(_err) => { - None - } - } - } else { - None - }; - - *file_option = new_file_option; - } - if vfork { let contexts = context::contexts(); if let Some(context_lock) = contexts.get(ppid) { @@ -897,17 +836,7 @@ pub fn exit(status: usize) -> ! { /// Files must be closed while context is valid so that messages can be passed 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(FileHandle::from(fd), file.scheme, event_id); - } - - let scheme_option = { - let schemes = scheme::schemes(); - schemes.get(file.scheme).map(|scheme| scheme.clone()) - }; - if let Some(scheme) = scheme_option { - let _ = scheme.close(file.number); - } + let _ = file.close(FileHandle::from(fd)); } }