Better comments for set_aux_fd

Author: coolreader18

src/tools/run-make-support/Cargo.toml

@@ -13,4 +13,3 @@ gimli = "0.31.0"
 libc = "0.2"
 build_helper = { path = "../build_helper" }
 serde_json = "1.0"
-libc = "0.2"

src/tools/run-make-support/src/command.rs

@@ -152,6 +152,10 @@ impl Command {
     }
 
     /// Set an auxiliary stream passed to the process, besides the stdio streams.
+    /// Use with caution - ideally, only set one aux fd; if there are multiple,
+    /// their old `fd` may overlap with another's `newfd`, and may break.
+    //FIXME: If more than 1 auxiliary file descriptor is needed, this function
+    // should be rewritten.
     #[cfg(unix)]
     pub fn set_aux_fd<F: Into<std::os::fd::OwnedFd>>(
         &mut self,
@@ -161,18 +165,31 @@ impl Command {
         use std::os::fd::AsRawFd;
         use std::os::unix::process::CommandExt;
 
+        let cvt = |x| if x == -1 { Err(std::io::Error::last_os_error()) } else { Ok(()) };
+
         let fd = fd.into();
-        unsafe {
-            self.cmd.pre_exec(move || {
-                let fd = fd.as_raw_fd();
-                let ret = if fd == newfd {
-                    libc::fcntl(fd, libc::F_SETFD, 0)
-                } else {
-                    libc::dup2(fd, newfd)
-                };
-                if ret == -1 { Err(std::io::Error::last_os_error()) } else { Ok(()) }
-            });
+        if fd.as_raw_fd() == newfd {
+            // if the new file descriptor is already the same as fd, just turn off FD_CLOEXEC
+            // SAFETY: io-safe: fd is already owned.
+            cvt(unsafe { libc::fcntl(fd.as_raw_fd(), libc::F_SETFD, 0) })
+                .expect("disabling CLOEXEC failed");
+            // The pre_exec function should be unconditionally set, since it captures
+            // `fd`, and this ensures that it stays open until the fork
         }
+        let pre_exec = move || {
+            if fd.as_raw_fd() != newfd {
+                // SAFETY: io-"safe": newfd is not necessarily an unused fd.
+                // However, we're ensuring that newfd will now refer to the same file descriptor
+                // as fd, which is safe as long as we manage the lifecycle of both descriptors
+                // correctly. This operation will replace the file descriptor referred to by newfd
+                // with the one from fd, allowing for shared access to the same underlying file or
+                // resource.
+                cvt(unsafe { libc::dup2(fd.as_raw_fd(), newfd) })?;
+            }
+            Ok(())
+        };
+        // SAFETY: dup2 is pre-exec-safe
+        unsafe { self.cmd.pre_exec(pre_exec) };
         self
     }
 

src/tools/run-make-support/src/macros.rs

@@ -81,6 +81,10 @@ macro_rules! impl_common_helpers {
             }
 
             /// Set an auxiliary stream passed to the process, besides the stdio streams.
+            /// Use with caution - ideally, only set one aux fd; if there are multiple,
+            /// their old `fd` may overlap with another's `newfd`, and may break.
+            //FIXME: If more than 1 auxiliary file descriptor is needed, this function
+            // should be rewritten.
             #[cfg(unix)]
             pub fn set_aux_fd<F: Into<std::os::fd::OwnedFd>>(
                 &mut self,

tests/run-make/jobserver-error/rmake.rs

@@ -4,24 +4,20 @@
 // and errors are printed instead in case of a wrong jobserver.
 // See https://github.com/rust-lang/rust/issues/46981
 
-// FIXME(Oneirical): The original test included this memo:
-// # Note that by default, the compiler uses file descriptors 0 (stdin), 1 (stdout), 2 (stderr),
-// # but also 3 and 4 for either end of the ctrl-c signal handler self-pipe.
-
 // FIXME(Oneirical): only-linux ignore-cross-compile
 
-use run_make_support::{diff, rfs, rustc};
+use run_make_support::{diff, rustc};
 
 fn main() {
     let out = rustc()
-        .stdin("fn main() {}")
+        .stdin_buf(("fn main() {}").as_bytes())
         .env("MAKEFLAGS", "--jobserver-auth=5,5")
         .run_fail()
         .stderr_utf8();
     diff().expected_file("cannot_open_fd.stderr").actual_text("actual", out).run();
 
     let out = rustc()
-        .stdin("fn main() {}")
+        .stdin_buf(("fn main() {}").as_bytes())
         .input("-")
         .env("MAKEFLAGS", "--jobserver-auth=3,3")
         .set_aux_fd(3, std::fs::File::open("/dev/null").unwrap())