Merge #1229

1229: Add clippy::missing_safety_doc lint to wasi, misc clean up r=MarkMcCaskey a=MarkMcCaskey

Part of #1219 

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>

Author: bors[bot]

lib/wasi/src/lib.rs

@@ -5,7 +5,8 @@
     unused_mut,
     unused_variables,
     unused_unsafe,
-    unreachable_patterns
+    unreachable_patterns,
+    clippy::missing_safety_doc
 )]
 #![doc(html_favicon_url = "https://wasmer.io/static/icons/favicon.ico")]
 #![doc(html_logo_url = "https://avatars3.githubusercontent.com/u/44205449?s=200&v=4")]

lib/wasi/src/state/mod.rs

@@ -36,7 +36,7 @@ use wasmer_runtime_core::vm::Ctx;
 /// the fd value of the virtual root
 pub const VIRTUAL_ROOT_FD: __wasi_fd_t = 3;
 /// all the rights enabled
-pub const ALL_RIGHTS: __wasi_rights_t = 0x1FFFFFFF;
+pub const ALL_RIGHTS: __wasi_rights_t = 0x1FFF_FFFF;
 const STDIN_DEFAULT_RIGHTS: __wasi_rights_t = __WASI_RIGHT_FD_DATASYNC
     | __WASI_RIGHT_FD_READ
     | __WASI_RIGHT_FD_SYNC
@@ -52,7 +52,10 @@ const STDOUT_DEFAULT_RIGHTS: __wasi_rights_t = __WASI_RIGHT_FD_DATASYNC
 const STDERR_DEFAULT_RIGHTS: __wasi_rights_t = STDOUT_DEFAULT_RIGHTS;
 
 /// Get WasiState from a Ctx
-/// This function is unsafe because it must be called on a WASI Ctx
+///
+/// # Safety
+/// - This function must be called on a `Ctx` that was created with `WasiState`
+///   in the data field
 pub unsafe fn get_wasi_state(ctx: &mut Ctx) -> &mut WasiState {
     &mut *(ctx.data as *mut WasiState)
 }
@@ -186,7 +189,7 @@ impl WasiFs {
         for dir in preopened_dirs {
             debug!("Attempting to preopen {}", &dir.to_string_lossy());
             // TODO: think about this
-            let default_rights = 0x1FFFFFFF; // all rights
+            let default_rights = ALL_RIGHTS;
             let cur_dir_metadata = dir.metadata().map_err(|e| {
                 format!(
                     "Could not get metadata for file {:?}: {}",
@@ -236,7 +239,7 @@ impl WasiFs {
         for (alias, real_dir) in mapped_dirs {
             debug!("Attempting to open {:?} at {}", real_dir, alias);
             // TODO: think about this
-            let default_rights = 0x1FFFFFFF; // all rights
+            let default_rights = ALL_RIGHTS;
             let cur_dir_metadata = real_dir.metadata().map_err(|e| {
                 format!(
                     "Could not get metadata for file {:?}: {}",
@@ -428,7 +431,7 @@ impl WasiFs {
 
         // create virtual root
         let root_inode = {
-            let all_rights = 0x1FFFFFFF;
+            let all_rights = ALL_RIGHTS;
             // TODO: make this a list of positive rigths instead of negative ones
             // root gets all right for now
             let root_rights = all_rights
@@ -525,10 +528,15 @@ impl WasiFs {
         next
     }
 
-    /// like create dir all, but it also opens it
+    /// This function is like create dir all, but it also opens it.
     /// Function is unsafe because it may break invariants and hasn't been tested.
     /// This is an experimental function and may be removed
-    // dead code because this is an API for external use
+    ///
+    /// # Safety
+    /// - Virtual directories created with this function must not conflict with
+    ///   the standard operation of the WASI filesystem.  This is vague and
+    ///   unlikely in pratice.  Join the discussion at https://github.com/wasmerio/wasmer/issues/1219
+    ///   for what the newer, safer WASI FS APIs should look like.
     #[allow(dead_code)]
     pub unsafe fn open_dir_all(
         &mut self,
@@ -1161,7 +1169,7 @@ impl WasiFs {
         stat.st_ino = self.get_next_inode_index();
 
         Ok(self.inodes.insert(InodeVal {
-            stat: stat,
+            stat,
             is_preopened,
             name,
             kind,
@@ -1210,10 +1218,14 @@ impl WasiFs {
         Ok(idx)
     }
 
-    /// This function is unsafe because it's the caller's responsibility to ensure that
-    /// all refences to the given inode have been removed from the filesystem
+    /// Low level function to remove an inode, that is it deletes the WASI FS's
+    /// knowledge of a file.
+    ///
+    /// This function returns the inode if it existed and was removed.
     ///
-    /// returns the inode if it existed and was removed
+    /// # Safety
+    /// - The caller must ensure that all references to the specified inode have
+    ///   been removed from the filesystem.
     pub unsafe fn remove_inode(&mut self, inode: Inode) -> Option<InodeVal> {
         self.inodes.remove(inode)
     }

lib/wasi/src/state/types.rs

@@ -434,7 +434,7 @@ impl<'de> Deserialize<'de> for HostFile {
             }
         }
 
-        const FIELDS: &'static [&'static str] = &["host_path", "flags"];
+        const FIELDS: &[&str] = &["host_path", "flags"];
         deserializer.deserialize_struct("HostFile", FIELDS, HostFileVisitor)
     }
 }

lib/wasi/src/syscalls/types.rs

@@ -1,4 +1,4 @@
-#![allow(non_camel_case_types)]
+#![allow(non_camel_case_types, clippy::identity_op)]
 
 use crate::ptr::{Array, WasmPtr};
 use byteorder::{ReadBytesExt, WriteBytesExt, LE};

lib/wasi/src/utils.rs

@@ -30,10 +30,10 @@ pub enum WasiVersion {
 }
 
 /// Namespace for the `Snapshot0` version.
-const SNAPSHOT0_NAMESPACE: &'static str = "wasi_unstable";
+const SNAPSHOT0_NAMESPACE: &str = "wasi_unstable";
 
 /// Namespace for the `Snapshot1` version.
-const SNAPSHOT1_NAMESPACE: &'static str = "wasi_snapshot_preview1";
+const SNAPSHOT1_NAMESPACE: &str = "wasi_snapshot_preview1";
 
 /// Detect the version of WASI being used based on the import
 /// namespaces.