Improved how descriptor/file flags are stored in Rust

All flags were being stored in the descriptor object, but only the
FD_CLOEXEC should be. The rest should be stored in the posix file
object itself. Also improved the comments.

src/main/host/descriptor/mod.rs

@@ -39,7 +39,27 @@ pub enum SyscallReturn {
 }
 
 bitflags::bitflags! {
-    // file modes: https://github.com/torvalds/linux/blob/master/include/linux/fs.h#L111
+    /// These are flags that can potentially be changed from the plugin (analagous to the Linux
+    /// `filp->f_flags` status flags). Not all `O_` flags are valid here. For example file access
+    /// mode flags (ex: `O_RDWR`) are stored elsewhere, and file creation flags (ex: `O_CREAT`)
+    /// are not stored anywhere. Many of these can be represented in different ways, for example:
+    /// `O_NONBLOCK`, `SOCK_NONBLOCK`, `EFD_NONBLOCK`, `GRND_NONBLOCK`, etc, and not all have the
+    /// same value.
+    pub struct FileFlags: libc::c_int {
+        const NONBLOCK = libc::O_NONBLOCK;
+        const APPEND = libc::O_APPEND;
+        const ASYNC = libc::O_ASYNC;
+        const DIRECT = libc::O_DIRECT;
+        const NOATIME = libc::O_NOATIME;
+    }
+}
+
+bitflags::bitflags! {
+    /// These are flags that should generally not change (analagous to the Linux `filp->f_mode`).
+    /// Since the plugin will never see these values and they're not exposed by the kernel, we
+    /// don't match the kernel `FMODE_` values here.
+    ///
+    /// Examples: https://github.com/torvalds/linux/blob/master/include/linux/fs.h#L111
     pub struct FileMode: u32 {
         const READ = 0b00000001;
         const WRITE = 0b00000010;
@@ -210,6 +230,18 @@ impl PosixFile {
         }
     }
 
+    pub fn get_flags(&self) -> FileFlags {
+        match self {
+            Self::Pipe(f) => f.get_flags(),
+        }
+    }
+
+    pub fn set_flags(&mut self, flags: FileFlags) {
+        match self {
+            Self::Pipe(f) => f.set_flags(flags),
+        }
+    }
+
     pub fn add_legacy_listener(&mut self, ptr: *mut c::StatusListener) {
         match self {
             Self::Pipe(f) => f.add_legacy_listener(ptr),
@@ -223,32 +255,39 @@ impl PosixFile {
     }
 }
 
+bitflags::bitflags! {
+    // Linux only supports a single descriptor flag:
+    // https://www.gnu.org/software/libc/manual/html_node/Descriptor-Flags.html
+    pub struct DescriptorFlags: u32 {
+        const CLOEXEC = libc::FD_CLOEXEC as u32;
+    }
+}
+
 #[derive(Clone)]
 pub struct Descriptor {
     file: Arc<AtomicRefCell<PosixFile>>,
-    flags: i32,
+    flags: DescriptorFlags,
 }
 
 impl Descriptor {
     pub fn new(file: Arc<AtomicRefCell<PosixFile>>) -> Self {
-        Self { file, flags: 0 }
+        Self {
+            file,
+            flags: DescriptorFlags::empty(),
+        }
     }
 
     pub fn get_file(&self) -> &Arc<AtomicRefCell<PosixFile>> {
         &self.file
     }
 
-    pub fn get_flags(&self) -> i32 {
+    pub fn get_flags(&self) -> DescriptorFlags {
         self.flags
     }
 
-    pub fn set_flags(&mut self, flags: i32) {
+    pub fn set_flags(&mut self, flags: DescriptorFlags) {
         self.flags = flags;
     }
-
-    pub fn add_flags(&mut self, flags: i32) {
-        self.flags |= flags;
-    }
 }
 
 // don't implement copy or clone without considering the legacy descriptor's ref count

src/main/host/descriptor/pipe.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use crate::cshadow as c;
 use crate::host::descriptor::{
-    FileMode, NewStatusListenerFilter, PosixFile, StatusEventSource, SyscallReturn,
+    FileFlags, FileMode, NewStatusListenerFilter, PosixFile, StatusEventSource, SyscallReturn,
 };
 use crate::utility::byte_queue::ByteQueue;
 use crate::utility::event_queue::{EventQueue, Handle};
@@ -13,16 +13,18 @@ pub struct PipeFile {
     event_source: StatusEventSource,
     status: c::Status,
     mode: FileMode,
+    flags: FileFlags,
     buffer_event_handle: Option<Handle<(c::Status, c::Status)>>,
 }
 
 impl PipeFile {
-    pub fn new(buffer: Arc<AtomicRefCell<SharedBuf>>, mode: FileMode) -> Self {
+    pub fn new(buffer: Arc<AtomicRefCell<SharedBuf>>, mode: FileMode, flags: FileFlags) -> Self {
         let mut rv = Self {
             buffer,
             event_source: StatusEventSource::new(),
             status: c::_Status_STATUS_NONE,
             mode,
+            flags,
             buffer_event_handle: None,
         };
 
@@ -31,6 +33,14 @@ impl PipeFile {
         rv
     }
 
+    pub fn get_flags(&self) -> FileFlags {
+        self.flags
+    }
+
+    pub fn set_flags(&mut self, flags: FileFlags) {
+        self.flags = flags;
+    }
+
     pub fn read(&mut self, bytes: &mut [u8], event_queue: &mut EventQueue) -> SyscallReturn {
         // if the file is not open for reading, return EBADF
         if !self.mode.contains(FileMode::READ) {

src/main/host/syscall/unistd.rs

@@ -1,6 +1,8 @@
 use crate::cshadow as c;
 use crate::host::descriptor::pipe;
-use crate::host::descriptor::{CompatDescriptor, Descriptor, FileMode, PosixFile, SyscallReturn};
+use crate::host::descriptor::{
+    CompatDescriptor, Descriptor, DescriptorFlags, FileFlags, FileMode, PosixFile, SyscallReturn,
+};
 use crate::host::syscall;
 use crate::utility::event_queue::EventQueue;
 
@@ -90,17 +92,20 @@ fn read_helper(
         unsafe { c::process_getWriteablePtr(sys.process, sys.thread, buf_ptr, size_needed as u64) };
     let mut buf = unsafe { std::slice::from_raw_parts_mut(buf_ptr as *mut u8, size_needed) };
 
+    let posix_file = desc.get_file();
+    let file_flags = posix_file.borrow().get_flags();
+
     // call the file's read(), and run any resulting events
     let result = EventQueue::queue_and_run(|event_queue| {
-        desc.get_file().borrow_mut().read(&mut buf, event_queue)
+        posix_file.borrow_mut().read(&mut buf, event_queue)
     });
 
     // if the syscall would block and it's a blocking descriptor
     if result == SyscallReturn::Error(nix::errno::EWOULDBLOCK)
-        && (desc.get_flags() & libc::O_NONBLOCK) == 0
+        && !file_flags.contains(FileFlags::NONBLOCK)
     {
         let trigger =
-            c::Trigger::from_posix_file(desc.get_file(), c::_Status_STATUS_DESCRIPTOR_READABLE);
+            c::Trigger::from_posix_file(posix_file, c::_Status_STATUS_DESCRIPTOR_READABLE);
 
         return c::SysCallReturn {
             state: c::SysCallReturnState_SYSCALL_BLOCK,
@@ -161,17 +166,19 @@ fn write_helper(
         unsafe { c::process_getReadablePtr(sys.process, sys.thread, buf_ptr, size_needed as u64) };
     let buf = unsafe { std::slice::from_raw_parts(buf_ptr as *const u8, size_needed) };
 
+    let posix_file = desc.get_file();
+    let file_flags = posix_file.borrow().get_flags();
+
     // call the file's write(), and run any resulting events
-    let result = EventQueue::queue_and_run(|event_queue| {
-        desc.get_file().borrow_mut().write(&buf, event_queue)
-    });
+    let result =
+        EventQueue::queue_and_run(|event_queue| posix_file.borrow_mut().write(&buf, event_queue));
 
     // if the syscall would block and it's a blocking descriptor
     if result == SyscallReturn::Error(nix::errno::EWOULDBLOCK)
-        && (desc.get_flags() & libc::O_NONBLOCK) == 0
+        && !file_flags.contains(FileFlags::NONBLOCK)
     {
         let trigger =
-            c::Trigger::from_posix_file(desc.get_file(), c::_Status_STATUS_DESCRIPTOR_WRITABLE);
+            c::Trigger::from_posix_file(posix_file, c::_Status_STATUS_DESCRIPTOR_WRITABLE);
 
         return c::SysCallReturn {
             state: c::SysCallReturnState_SYSCALL_BLOCK,
@@ -205,14 +212,26 @@ fn pipe_helper(sys: &mut c::SysCallHandler, fd_ptr: c::PluginPtr, flags: i32) ->
         return c::SysCallReturn::from_errno(nix::errno::Errno::EFAULT);
     }
 
-    // pipe flags that we support
-    let supported_flags = libc::O_NONBLOCK & libc::O_CLOEXEC;
-    let flags_to_set = flags & supported_flags;
+    let mut file_flags = FileFlags::empty();
+    let mut descriptor_flags = DescriptorFlags::empty();
+
+    // keep track of which flags we use
+    let mut remaining_flags = flags;
+
+    if flags & libc::O_NONBLOCK != 0 {
+        file_flags.insert(FileFlags::NONBLOCK);
+        remaining_flags &= !libc::O_NONBLOCK;
+    }
+
+    if flags & libc::O_CLOEXEC != 0 {
+        descriptor_flags.insert(DescriptorFlags::CLOEXEC);
+        remaining_flags &= !libc::O_CLOEXEC;
+    }
 
     // the user requested flags that we don't support
-    if flags != flags_to_set {
+    if remaining_flags != 0 {
         // exit early if the O_DIRECT flag was set
-        if flags & libc::O_DIRECT != 0 {
+        if remaining_flags & libc::O_DIRECT != 0 {
             warn!("We don't currently support pipes in 'O_DIRECT' mode");
             return c::SysCallReturn::from_errno(nix::errno::Errno::EOPNOTSUPP);
         }
@@ -224,11 +243,11 @@ fn pipe_helper(sys: &mut c::SysCallHandler, fd_ptr: c::PluginPtr, flags: i32) ->
     let buffer = Arc::new(AtomicRefCell::new(buffer));
 
     // reference-counted file object for read end of the pipe
-    let reader = pipe::PipeFile::new(Arc::clone(&buffer), FileMode::READ);
+    let reader = pipe::PipeFile::new(Arc::clone(&buffer), FileMode::READ, file_flags);
     let reader = Arc::new(AtomicRefCell::new(PosixFile::Pipe(reader)));
 
     // reference-counted file object for write end of the pipe
-    let writer = pipe::PipeFile::new(Arc::clone(&buffer), FileMode::WRITE);
+    let writer = pipe::PipeFile::new(Arc::clone(&buffer), FileMode::WRITE, file_flags);
     let writer = Arc::new(AtomicRefCell::new(PosixFile::Pipe(writer)));
 
     // set the file objects to listen for events on the buffer
@@ -240,8 +259,8 @@ fn pipe_helper(sys: &mut c::SysCallHandler, fd_ptr: c::PluginPtr, flags: i32) ->
     let mut writer_desc = Descriptor::new(writer);
 
     // set the file descriptor flags
-    reader_desc.set_flags(flags_to_set);
-    writer_desc.set_flags(flags_to_set);
+    reader_desc.set_flags(descriptor_flags);
+    writer_desc.set_flags(descriptor_flags);
 
     // register the file descriptors and return them to the caller
     let num_items = 2;