From 21399914b08addbcab41acac1824d5ee53c099fe Mon Sep 17 00:00:00 2001
From: Jesse Luehrs <doy@tozt.net>
Date: Wed, 5 Jan 2022 00:36:02 -0500
Subject: some safety comments

---
 src/builtins/command.rs  | 2 ++
 src/pipeline/command.rs  | 2 ++
 src/pipeline/mod.rs      | 7 ++++---
 src/state/history/mod.rs | 6 ++++++
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/builtins/command.rs b/src/builtins/command.rs
index 2ea7f18..f3ec598 100644
--- a/src/builtins/command.rs
+++ b/src/builtins/command.rs
@@ -32,6 +32,8 @@ impl Command {
         self.io.set_stderr(fh);
     }
 
+    // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this
+    // is just a wrapper)
     pub unsafe fn pre_exec<F>(&mut self, f: F)
     where
         F: 'static + FnMut() -> std::io::Result<()> + Send + Sync,
diff --git a/src/pipeline/command.rs b/src/pipeline/command.rs
index 408f845..2de1ff4 100644
--- a/src/pipeline/command.rs
+++ b/src/pipeline/command.rs
@@ -74,6 +74,8 @@ impl Command {
         }
     }
 
+    // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this
+    // is just a wrapper)
     pub unsafe fn pre_exec<F>(&mut self, f: F)
     where
         F: 'static + FnMut() -> std::io::Result<()> + Send + Sync,
diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs
index ba69f52..62e246f 100644
--- a/src/pipeline/mod.rs
+++ b/src/pipeline/mod.rs
@@ -16,6 +16,7 @@ mod command;
 pub use command::{Child, Command};
 
 pub async fn run() -> anyhow::Result<i32> {
+    // Safety: we don't create File instances for fd 3 or 4 anywhere else
     let shell_read = unsafe { async_std::fs::File::from_raw_fd(3) };
     let shell_write = unsafe { async_std::fs::File::from_raw_fd(4) };
 
@@ -234,9 +235,9 @@ async fn wait_children(
 
 fn pipe() -> anyhow::Result<(std::fs::File, std::fs::File)> {
     let (r, w) = nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC)?;
-    // Safety: these file descriptors were just returned by pipe2 above, which
-    // means they must be valid otherwise that call would have returned an
-    // error
+    // Safety: these file descriptors were just returned by pipe2 above, and
+    // are only available in this function, so nothing else can be accessing
+    // them
     Ok((unsafe { std::fs::File::from_raw_fd(r) }, unsafe {
         std::fs::File::from_raw_fd(w)
     }))
diff --git a/src/state/history/mod.rs b/src/state/history/mod.rs
index 9f2d254..8b15916 100644
--- a/src/state/history/mod.rs
+++ b/src/state/history/mod.rs
@@ -622,6 +622,7 @@ async fn run_pipeline(
         nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC).unwrap();
     let (from_r, from_w) =
         nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC).unwrap();
+    // Safety: dup2 is an async-signal-safe function
     unsafe {
         cmd.pre_exec(move || {
             nix::unistd::dup2(to_r, 3)?;
@@ -633,6 +634,8 @@ async fn run_pipeline(
     nix::unistd::close(to_r).unwrap();
     nix::unistd::close(from_w).unwrap();
 
+    // Safety: to_w was just opened above, was not used until now, and can't
+    // be used after this because we rebound the variable
     let mut to_w = unsafe { async_std::fs::File::from_raw_fd(to_w) };
     to_w.write_all(&env.as_bytes()).await.unwrap();
     drop(to_w);
@@ -646,6 +649,9 @@ async fn run_pipeline(
         let read = async move {
             Res::Read(
                 blocking::unblock(move || {
+                    // Safety: from_r was just opened above and is only
+                    // referenced in this closure, which takes ownership of it
+                    // at the start and returns ownership of it at the end
                     let fh = unsafe { std::fs::File::from_raw_fd(from_r) };
                     let event = bincode::deserialize_from(&fh);
                     let _ = fh.into_raw_fd();
-- 
cgit v1.2.3-54-g00ecf