Pass Fd by reference.

Make handles non-`Copy` and non-`Clone`. Pass them by reference, and
store them instructs as raw `u32` values. And make `Fd::from_raw` unsafe
so that we're careful about where we get handles from.

This also obviates the `Fd` trait. With `EntryTable`, it's now
straightforward to let `FdPool` just operate on `u32` values.

Author: sunfishcode

crates/wasi-common/src/ctx.rs

@@ -381,7 +381,7 @@ impl WasiCtxBuilder {
 #[derive(Debug)]
 struct EntryTable {
     fd_pool: FdPool,
-    entries: HashMap<types::Fd, Entry>,
+    entries: HashMap<u32, Entry>,
 }
 
 impl EntryTable {
@@ -393,30 +393,30 @@ impl EntryTable {
     }
 
     fn contains(&self, fd: &types::Fd) -> bool {
-        self.entries.contains_key(fd)
+        self.entries.contains_key(&fd.as_raw())
     }
 
     fn insert(&mut self, entry: Entry) -> Option<types::Fd> {
         let fd = self.fd_pool.allocate()?;
         self.entries.insert(fd, entry);
-        Some(fd)
+        Some(unsafe { types::Fd::from_raw(fd) })
     }
 
     fn insert_at(&mut self, fd: &types::Fd, entry: Entry) {
-        self.entries.insert(*fd, entry);
+        self.entries.insert(fd.as_raw(), entry);
     }
 
     fn get(&self, fd: &types::Fd) -> Option<&Entry> {
-        self.entries.get(fd)
+        self.entries.get(&fd.as_raw())
     }
 
     fn get_mut(&mut self, fd: &types::Fd) -> Option<&mut Entry> {
-        self.entries.get_mut(fd)
+        self.entries.get_mut(&fd.as_raw())
     }
 
-    fn remove(&mut self, fd: types::Fd) -> Option<Entry> {
-        let entry = self.entries.remove(&fd)?;
-        self.fd_pool.deallocate(fd);
+    fn remove(&mut self, fd: &types::Fd) -> Option<Entry> {
+        let entry = self.entries.remove(&fd.as_raw())?;
+        self.fd_pool.deallocate(fd.as_raw());
         Some(entry)
     }
 }
@@ -444,13 +444,13 @@ impl WasiCtx {
             .build()
     }
 
-    /// Check if `WasiCtx` contains the specified raw WASI `fd`.
-    pub(crate) fn contains_entry(&self, fd: types::Fd) -> bool {
+    /// Check if `WasiCtx` contains the specified WASI `Fd`.
+    pub(crate) fn contains_entry(&self, fd: &types::Fd) -> bool {
         self.entries.borrow().contains(&fd)
     }
 
-    /// Get an immutable `Entry` corresponding to the specified raw WASI `fd`.
-    pub(crate) fn get_entry(&self, fd: types::Fd) -> Result<Ref<Entry>> {
+    /// Get an immutable `Entry` corresponding to the specified WASI `Fd`.
+    pub(crate) fn get_entry(&self, fd: &types::Fd) -> Result<Ref<Entry>> {
         if !self.contains_entry(fd) {
             return Err(Errno::Badf);
         }
@@ -459,9 +459,9 @@ impl WasiCtx {
         }))
     }
 
-    /// Get a mutable `Entry` corresponding to the specified raw WASI `fd`.
+    /// Get a mutable `Entry` corresponding to the specified WASI `Fd`.
     // TODO This runs the risk of a potential difficult-to-predict panic down-the-line.
-    pub(crate) fn get_entry_mut(&self, fd: types::Fd) -> Result<RefMut<Entry>> {
+    pub(crate) fn get_entry_mut(&self, fd: &types::Fd) -> Result<RefMut<Entry>> {
         if !self.contains_entry(fd) {
             return Err(Errno::Badf);
         }
@@ -472,20 +472,20 @@ impl WasiCtx {
 
     /// Insert the specified `Entry` into the `WasiCtx` object.
     ///
-    /// The `Entry` will automatically get another free raw WASI `fd` assigned. Note that
-    /// the two subsequent free raw WASI `fd`s do not have to be stored contiguously.
+    /// The `Entry` will automatically get another free WASI `Fd` assigned. Note that
+    /// the two subsequent free WASI `Fd`s do not have to be stored contiguously.
     pub(crate) fn insert_entry(&self, entry: Entry) -> Result<types::Fd> {
         self.entries.borrow_mut().insert(entry).ok_or(Errno::Mfile)
     }
 
-    /// Insert the specified `Entry` with the specified raw WASI `fd` key into the `WasiCtx`
+    /// Insert the specified `Entry` with the specified WASI `Fd` key into the `WasiCtx`
     /// object.
-    pub(crate) fn insert_entry_at(&self, fd: types::Fd, entry: Entry) {
+    pub(crate) fn insert_entry_at(&self, fd: &types::Fd, entry: Entry) {
         self.entries.borrow_mut().insert_at(&fd, entry)
     }
 
-    /// Remove `Entry` corresponding to the specified raw WASI `fd` from the `WasiCtx` object.
-    pub(crate) fn remove_entry(&self, fd: types::Fd) -> Result<Entry> {
+    /// Remove `Entry` corresponding to the specified WASI `Fd` from the `WasiCtx` object.
+    pub(crate) fn remove_entry(&self, fd: &types::Fd) -> Result<Entry> {
         self.entries.borrow_mut().remove(fd).ok_or(Errno::Badf)
     }
 }

crates/wasi-common/src/fdpool.rs

@@ -2,25 +2,8 @@
 //! pool. It's intended to be mainly used within the `WasiCtx`
 //! object(s).
 
-/// Any type wishing to be treated as a valid WASI file descriptor
-/// should implement this trait.
-///
-/// This trait is required as internally we use `u32` to represent
-/// and manage raw file descriptors.
-pub(crate) trait Fd {
-    /// Convert to `u32`.
-    fn as_raw(&self) -> u32;
-    /// Convert from `u32`.
-    fn from_raw(raw_fd: u32) -> Self;
-}
-
 /// This container tracks and manages all file descriptors that
 /// were already allocated.
-/// Internally, we use `u32` to represent the file descriptors;
-/// however, the caller may supply any type `T` such that it
-/// implements the `Fd` trait when requesting a new descriptor
-/// via the `allocate` method, or when returning one back via
-/// the `deallocate` method.
 #[derive(Debug)]
 pub(crate) struct FdPool {
     next_alloc: Option<u32>,
@@ -41,11 +24,11 @@ impl FdPool {
     /// descriptors (which would be equal to `2^32 + 1` accounting for `0`),
     /// then this method will return `None` to signal that case.
     /// Otherwise, a new file descriptor is return as `Some(fd)`.
-    pub fn allocate<T: Fd>(&mut self) -> Option<T> {
+    pub fn allocate(&mut self) -> Option<u32> {
         if let Some(fd) = self.available.pop() {
             // Since we've had free, unclaimed handle in the pool,
             // simply claim it and return.
-            return Some(T::from_raw(fd));
+            return Some(fd);
         }
         // There are no free handles available in the pool, so try
         // allocating an additional one into the pool. If we've
@@ -55,16 +38,15 @@ impl FdPool {
         // It's OK to not unpack the result of `fd.checked_add()` here which
         // can fail since we check for `None` in the snippet above.
         self.next_alloc = fd.checked_add(1);
-        Some(T::from_raw(fd))
+        Some(fd)
     }
 
     /// Return a file descriptor back to the pool.
     ///
     /// If the caller tries to return a file descriptor that was
     /// not yet allocated (via spoofing, etc.), this method
     /// will panic.
-    pub fn deallocate<T: Fd>(&mut self, fd: T) {
-        let fd = fd.as_raw();
+    pub fn deallocate(&mut self, fd: u32) {
         if let Some(next_alloc) = self.next_alloc {
             assert!(fd < next_alloc);
         }
@@ -76,44 +58,24 @@ impl FdPool {
 #[cfg(test)]
 mod test {
     use super::FdPool;
-    use std::ops::Deref;
-
-    #[derive(Debug)]
-    struct Fd(u32);
-
-    impl super::Fd for Fd {
-        fn as_raw(&self) -> u32 {
-            self.0
-        }
-        fn from_raw(raw_fd: u32) -> Self {
-            Self(raw_fd)
-        }
-    }
-
-    impl Deref for Fd {
-        type Target = u32;
-        fn deref(&self) -> &Self::Target {
-            &self.0
-        }
-    }
 
     #[test]
     fn basics() {
         let mut fd_pool = FdPool::new();
-        let mut fd: Fd = fd_pool.allocate().expect("success allocating 0");
-        assert_eq!(*fd, 0);
+        let mut fd = fd_pool.allocate().expect("success allocating 0");
+        assert_eq!(fd, 0);
         fd = fd_pool.allocate().expect("success allocating 1");
-        assert_eq!(*fd, 1);
+        assert_eq!(fd, 1);
         fd = fd_pool.allocate().expect("success allocating 2");
-        assert_eq!(*fd, 2);
+        assert_eq!(fd, 2);
         fd_pool.deallocate(1u32);
         fd_pool.deallocate(0u32);
         fd = fd_pool.allocate().expect("success reallocating 0");
-        assert_eq!(*fd, 0);
+        assert_eq!(fd, 0);
         fd = fd_pool.allocate().expect("success reallocating 1");
-        assert_eq!(*fd, 1);
+        assert_eq!(fd, 1);
         fd = fd_pool.allocate().expect("success allocating 3");
-        assert_eq!(*fd, 3);
+        assert_eq!(fd, 3);
     }
 
     #[test]
@@ -128,6 +90,6 @@ mod test {
         let mut fd_pool = FdPool::new();
         // Spoof reaching the limit of allocs.
         fd_pool.next_alloc = None;
-        assert!(fd_pool.allocate::<Fd>().is_none());
+        assert!(fd_pool.allocate().is_none());
     }
 }

crates/wasi-common/src/fs/dir.rs

@@ -18,12 +18,12 @@ use std::{io, path::Path};
 /// don't interoperate well with the capability-oriented security model.
 pub struct Dir<'ctx> {
     ctx: &'ctx WasiCtx,
-    fd: types::Fd,
+    fd: &'ctx types::Fd,
 }
 
 impl<'ctx> Dir<'ctx> {
     /// Constructs a new instance of `Self` from the given raw WASI file descriptor.
-    pub unsafe fn from_raw_wasi_fd(ctx: &'ctx WasiCtx, fd: types::Fd) -> Self {
+    pub unsafe fn from_raw_wasi_fd(ctx: &'ctx WasiCtx, fd: &'ctx types::Fd) -> Self {
         Self { ctx, fd }
     }
 
@@ -38,9 +38,6 @@ impl<'ctx> Dir<'ctx> {
     ///
     /// [`std::fs::File::open`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.open
     pub fn open_file<P: AsRef<Path>>(&mut self, path: P) -> io::Result<File> {
-        let path = path.as_ref();
-        let mut fd = types::Fd::from(0);
-
         // TODO: Refactor the hostcalls functions to split out the encoding/decoding
         // parts from the underlying functionality, so that we can call into the
         // underlying functionality directly.
@@ -51,6 +48,9 @@ impl<'ctx> Dir<'ctx> {
         // on `OsStrExt`.
         unimplemented!("Dir::open_file");
         /*
+        let path = path.as_ref();
+        let mut fd = types::Fd { raw: types::RawFd::from(0) };
+
         wasi_errno_to_io_error(hostcalls::path_open(
             self.ctx,
             self.fd,
@@ -63,10 +63,10 @@ impl<'ctx> Dir<'ctx> {
             0,
             &mut fd,
         ))?;
-        */
 
         let ctx = self.ctx;
         Ok(unsafe { File::from_raw_wasi_fd(ctx, fd) })
+        */
     }
 
     /// Opens a file at `path` with the options specified by `self`.
@@ -91,12 +91,12 @@ impl<'ctx> Dir<'ctx> {
     ///
     /// TODO: Not yet implemented. See the comment in `open_file`.
     pub fn open_dir<P: AsRef<Path>>(&mut self, path: P) -> io::Result<Self> {
-        let path = path.as_ref();
-        let mut fd = types::Fd::from(0);
-
         // TODO: See the comment in `open_file`.
         unimplemented!("Dir::open_dir");
         /*
+        let path = path.as_ref();
+        let mut fd = types::Fd { raw: types::RawFd::from(0) };
+
         wasi_errno_to_io_error(hostcalls::path_open(
             self.ctx,
             self.fd,
@@ -108,10 +108,10 @@ impl<'ctx> Dir<'ctx> {
             0,
             &mut fd,
         ))?;
-        */
 
         let ctx = self.ctx;
         Ok(unsafe { Dir::from_raw_wasi_fd(ctx, fd) })
+        */
     }
 
     /// Opens a file in write-only mode.
@@ -123,14 +123,14 @@ impl<'ctx> Dir<'ctx> {
     ///
     /// [`std::fs::File::create`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.create
     pub fn create_file<P: AsRef<Path>>(&mut self, path: P) -> io::Result<File> {
-        let path = path.as_ref();
-        let mut fd = types::Fd::from(0);
-
         // TODO: See the comments in `open_file`.
         //
         // TODO: Set the requested rights to be read+write.
         unimplemented!("Dir::create_file");
         /*
+        let path = path.as_ref();
+        let mut fd = types::Fd { raw: types::RawFd::from(0) };
+
         wasi_errno_to_io_error(hostcalls::path_open(
             self.ctx,
             self.fd,
@@ -143,10 +143,10 @@ impl<'ctx> Dir<'ctx> {
             0,
             &mut fd,
         ))?;
-        */
 
         let ctx = self.ctx;
         Ok(unsafe { File::from_raw_wasi_fd(ctx, fd) })
+        */
     }
 
     /// Returns an iterator over the entries within a directory.
@@ -159,13 +159,13 @@ impl<'ctx> Dir<'ctx> {
     /// now, use `into_read` instead.
     ///
     /// [`std::fs::read_dir`]: https://doc.rust-lang.org/std/fs/fn.read_dir.html
-    pub fn read(&mut self) -> io::Result<ReadDir> {
+    pub fn read(&mut self) -> io::Result<ReadDir<'ctx>> {
         unimplemented!("Dir::read")
     }
 
     /// Consumes self and returns an iterator over the entries within a directory
     /// in the manner of `read`.
-    pub fn into_read(self) -> ReadDir {
+    pub fn into_read(self) -> ReadDir<'ctx> {
         unsafe { ReadDir::from_raw_wasi_fd(self.fd) }
     }
 
@@ -189,7 +189,7 @@ impl<'ctx> Dir<'ctx> {
     /// relative to and within `self`.
     ///
     /// [`std::fs::read_dir`]: https://doc.rust-lang.org/std/fs/fn.read_dir.html
-    pub fn read_dir<P: AsRef<Path>>(&mut self, path: P) -> io::Result<ReadDir> {
+    pub fn read_dir<P: AsRef<Path>>(&mut self, path: P) -> io::Result<ReadDir<'ctx>> {
         self.open_dir(path)?.read()
     }
 }

crates/wasi-common/src/fs/file.rs

@@ -18,7 +18,7 @@ use std::io;
 /// [`Dir::create_file`]: struct.Dir.html#method.create_file
 pub struct File<'ctx> {
     ctx: &'ctx WasiCtx,
-    fd: types::Fd,
+    fd: &'ctx types::Fd,
 }
 
 impl<'ctx> File<'ctx> {
@@ -27,7 +27,7 @@ impl<'ctx> File<'ctx> {
     /// This corresponds to [`std::fs::File::from_raw_fd`].
     ///
     /// [`std::fs::File::from_raw_fd`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.from_raw_fd
-    pub unsafe fn from_raw_wasi_fd(ctx: &'ctx WasiCtx, fd: types::Fd) -> Self {
+    pub unsafe fn from_raw_wasi_fd(ctx: &'ctx WasiCtx, fd: &'ctx types::Fd) -> Self {
         Self { ctx, fd }
     }
 

crates/wasi-common/src/fs/readdir.rs

@@ -8,19 +8,19 @@ use crate::wasi::types;
 /// TODO: Not yet implemented.
 ///
 /// [`std::fs::ReadDir`]: https://doc.rust-lang.org/std/fs/struct.ReadDir.html
-pub struct ReadDir {
-    fd: types::Fd,
+pub struct ReadDir<'ctx> {
+    fd: &'ctx types::Fd,
 }
 
-impl ReadDir {
+impl<'ctx> ReadDir<'ctx> {
     /// Constructs a new instance of `Self` from the given raw WASI file descriptor.
-    pub unsafe fn from_raw_wasi_fd(fd: types::Fd) -> Self {
+    pub unsafe fn from_raw_wasi_fd(fd: &'ctx types::Fd) -> Self {
         Self { fd }
     }
 }
 
 /// TODO: Not yet implemented.
-impl Iterator for ReadDir {
+impl<'ctx> Iterator for ReadDir<'ctx> {
     type Item = DirEntry;
 
     /// TODO: Not yet implemented.