From c005f94a5519432f738d2a28fc29065e84c7f33a Mon Sep 17 00:00:00 2001 From: Mark McCaskey Date: Mon, 17 Feb 2020 15:15:04 -0800 Subject: [PATCH] Add clippy::missing_safety_doc lint to wasi, misc clean up --- lib/wasi/src/lib.rs | 3 ++- lib/wasi/src/state/mod.rs | 34 +++++++++++++++++++++++----------- lib/wasi/src/state/types.rs | 2 +- lib/wasi/src/syscalls/types.rs | 2 +- lib/wasi/src/utils.rs | 4 ++-- 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/wasi/src/lib.rs b/lib/wasi/src/lib.rs index 6d9dd4fce..167888e0e 100644 --- a/lib/wasi/src/lib.rs +++ b/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")] diff --git a/lib/wasi/src/state/mod.rs b/lib/wasi/src/state/mod.rs index f7d022aa0..f05c44b91 100644 --- a/lib/wasi/src/state/mod.rs +++ b/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. /// - /// returns the inode if it existed and was removed + /// This function 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 { self.inodes.remove(inode) } diff --git a/lib/wasi/src/state/types.rs b/lib/wasi/src/state/types.rs index b352669c7..3308887de 100644 --- a/lib/wasi/src/state/types.rs +++ b/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) } } diff --git a/lib/wasi/src/syscalls/types.rs b/lib/wasi/src/syscalls/types.rs index 5f5559ce4..9890fb76d 100644 --- a/lib/wasi/src/syscalls/types.rs +++ b/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}; diff --git a/lib/wasi/src/utils.rs b/lib/wasi/src/utils.rs index 8a72a215a..f1395ced2 100644 --- a/lib/wasi/src/utils.rs +++ b/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.