From 919190e0620e5271413599c861983d77190c6b54 Mon Sep 17 00:00:00 2001
From: Marcin Mielniczuk <marmistrz.dev@zoho.eu>
Date: Thu, 9 Jan 2020 21:14:20 +0100
Subject: [PATCH] Document the behavior of some rights-related functions.

cf. #770
---
 crates/wasi-common/src/hostcalls_impl/fs.rs        | 6 ++----
 crates/wasi-common/src/old/snapshot_0/fdentry.rs   | 3 +++
 crates/wasi-common/src/sys/unix/fdentry_impl.rs    | 5 +++++
 crates/wasi-common/src/sys/windows/fdentry_impl.rs | 9 +++++++--
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/crates/wasi-common/src/hostcalls_impl/fs.rs b/crates/wasi-common/src/hostcalls_impl/fs.rs
index 3d2e732433..f4ed12822a 100644
--- a/crates/wasi-common/src/hostcalls_impl/fs.rs
+++ b/crates/wasi-common/src/hostcalls_impl/fs.rs
@@ -610,10 +610,8 @@ pub(crate) unsafe fn path_open(
     let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?;
 
     let mut fe = FdEntry::from(fd)?;
-    // We need to manually deny the rights which are not explicitly requested.
-    // This should not be needed, but currently determine_type_and_access_rights,
-    // which is used by FdEntry::from, may grant extra rights while inferring it
-    // from the open mode.
+    // We need to manually deny the rights which are not explicitly requested
+    // because FdEntry::from will assign maximal consistent rights.
     fe.rights_base &= fs_rights_base;
     fe.rights_inheriting &= fs_rights_inheriting;
     let guest_fd = wasi_ctx.insert_fd_entry(fe)?;
diff --git a/crates/wasi-common/src/old/snapshot_0/fdentry.rs b/crates/wasi-common/src/old/snapshot_0/fdentry.rs
index 44cc4ce403..aab0178b93 100644
--- a/crates/wasi-common/src/old/snapshot_0/fdentry.rs
+++ b/crates/wasi-common/src/old/snapshot_0/fdentry.rs
@@ -61,6 +61,9 @@ pub(crate) struct FdEntry {
 }
 
 impl FdEntry {
+    /// Create an FdEntry with *maximal* possible rights from a given `File`.
+    /// If this is not desired, the rights of the resulting `FdEntry` should
+    /// be manually restricted.
     pub(crate) fn from(file: fs::File) -> Result<Self> {
         unsafe { determine_type_and_access_rights(&file) }.map(
             |(file_type, rights_base, rights_inheriting)| Self {
diff --git a/crates/wasi-common/src/sys/unix/fdentry_impl.rs b/crates/wasi-common/src/sys/unix/fdentry_impl.rs
index e44f54c076..6efd681cb2 100644
--- a/crates/wasi-common/src/sys/unix/fdentry_impl.rs
+++ b/crates/wasi-common/src/sys/unix/fdentry_impl.rs
@@ -26,6 +26,9 @@ pub(crate) fn descriptor_as_oshandle<'lifetime>(
     })))
 }
 
+/// Returns the set of all possible rights that are both relevant for the file
+/// type and consistent with the open mode.
+///
 /// This function is unsafe because it operates on a raw file descriptor.
 pub(crate) unsafe fn determine_type_and_access_rights<Fd: AsRawFd>(
     fd: &Fd,
@@ -48,6 +51,8 @@ pub(crate) unsafe fn determine_type_and_access_rights<Fd: AsRawFd>(
     Ok((file_type, rights_base, rights_inheriting))
 }
 
+/// Returns the set of all possible rights that are relevant for file type.
+///
 /// This function is unsafe because it operates on a raw file descriptor.
 pub(crate) unsafe fn determine_type_rights<Fd: AsRawFd>(
     fd: &Fd,
diff --git a/crates/wasi-common/src/sys/windows/fdentry_impl.rs b/crates/wasi-common/src/sys/windows/fdentry_impl.rs
index bb7b4263fc..8700638004 100644
--- a/crates/wasi-common/src/sys/windows/fdentry_impl.rs
+++ b/crates/wasi-common/src/sys/windows/fdentry_impl.rs
@@ -54,7 +54,10 @@ pub(crate) fn descriptor_as_oshandle<'lifetime>(
     })))
 }
 
-/// This function is unsafe because it operates on a raw file handle.
+/// Returns the set of all possible rights that are both relevant for the file
+/// type and consistent with the open mode.
+///
+/// This function is unsafe because it operates on a raw file descriptor.
 pub(crate) unsafe fn determine_type_and_access_rights<Handle: AsRawHandle>(
     handle: &Handle,
 ) -> Result<(
@@ -85,7 +88,9 @@ pub(crate) unsafe fn determine_type_and_access_rights<Handle: AsRawHandle>(
     Ok((file_type, rights_base, rights_inheriting))
 }
 
-/// This function is unsafe because it operates on a raw file handle.
+/// Returns the set of all possible rights that are relevant for file type.
+///
+/// This function is unsafe because it operates on a raw file descriptor.
 pub(crate) unsafe fn determine_type_rights<Handle: AsRawHandle>(
     handle: &Handle,
 ) -> Result<(