Extract exec_command function out of no_pty and use_pty

Author: bjorn3

src/exec/mod.rs

@@ -18,16 +18,17 @@ use std::{
 };
 
 use crate::{
+    common::bin_serde::BinPipe,
     exec::no_pty::exec_no_pty,
     log::{dev_info, dev_warn, user_error},
     system::{
+        _exit,
         interface::ProcessId,
-        killpg,
-        signal::{consts::*, signal_name},
+        kill, killpg, set_target_user,
+        signal::{consts::*, signal_name, SignalNumber, SignalSet},
+        term::UserTerm,
         wait::{Wait, WaitError, WaitOptions},
-    },
-    system::{
-        kill, set_target_user, signal::SignalNumber, term::UserTerm, FileCloser, Group, User,
+        FileCloser, Group, User,
     },
 };
 
@@ -143,6 +144,45 @@ pub enum ExitReason {
     Signal(i32),
 }
 
+fn exec_command(
+    file_closer: FileCloser,
+    mut command: Command,
+    original_set: Option<SignalSet>,
+    mut errpipe_tx: BinPipe<i32>,
+) -> ! {
+    // Restore the signal mask now that the handlers have been setup.
+    if let Some(set) = original_set {
+        if let Err(err) = set.set_mask() {
+            dev_warn!("cannot restore signal mask: {err}");
+        }
+    }
+
+    // SAFETY: We immediately exec after this call and if the exec fails we only access stderr
+    // and errpipe before exiting without running atexit handlers using _exit
+    if let Err(err) = unsafe { file_closer.close_the_universe() } {
+        dev_warn!("failed to close the universe: {err}");
+        // Send the error to the monitor using the pipe.
+        if let Some(error_code) = err.raw_os_error() {
+            errpipe_tx.write(&error_code).ok();
+        }
+
+        // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
+        _exit(1);
+    }
+
+    let err = command.exec();
+
+    dev_warn!("failed to execute command: {err}");
+    // If `exec` returns, it means that executing the command failed. Send the error to the
+    // monitor using the pipe.
+    if let Some(error_code) = err.raw_os_error() {
+        errpipe_tx.write(&error_code).ok();
+    }
+
+    // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
+    _exit(1);
+}
+
 // Kill the process with increasing urgency.
 //
 // Based on `terminate_command`.

src/exec/no_pty.rs

@@ -1,4 +1,4 @@
-use std::{ffi::c_int, io, os::unix::process::CommandExt, process::Command};
+use std::{ffi::c_int, io, process::Command};
 
 use super::{
     event::PollEvent,
@@ -14,10 +14,10 @@ use crate::{
     },
 };
 use crate::{
-    exec::{handle_sigchld, signal_fmt},
+    exec::{exec_command, handle_sigchld, signal_fmt},
     log::{dev_error, dev_info, dev_warn},
     system::{
-        _exit, fork, getpgid, getpgrp,
+        fork, getpgid, getpgrp,
         interface::ProcessId,
         kill, killpg,
         term::{Terminal, UserTerm},
@@ -29,7 +29,7 @@ use crate::{
 pub(super) fn exec_no_pty(
     sudo_pid: ProcessId,
     mut file_closer: FileCloser,
-    mut command: Command,
+    command: Command,
 ) -> io::Result<ExitReason> {
     // FIXME (ogsudo): Initialize the policy plugin's session here.
 
@@ -46,7 +46,7 @@ pub(super) fn exec_no_pty(
     // FIXME (ogsudo): Some extra config happens here if selinux is available.
 
     // Use a pipe to get the IO error if `exec` fails.
-    let (mut errpipe_tx, errpipe_rx) = BinPipe::pair()?;
+    let (errpipe_tx, errpipe_rx) = BinPipe::pair()?;
 
     // Don't close the error pipe as we need it to retrieve the error code if the command execution
     // fails.
@@ -58,37 +58,7 @@ pub(super) fn exec_no_pty(
         err
     })?
     else {
-        // Restore the signal mask now that the handlers have been setup.
-        if let Some(set) = original_set {
-            if let Err(err) = set.set_mask() {
-                dev_warn!("cannot restore signal mask: {err}");
-            }
-        }
-
-        // SAFETY: We immediately exec after this call and if the exec fails we only access stderr
-        // and errpipe before exiting without running atexit handlers using _exit
-        if let Err(err) = unsafe { file_closer.close_the_universe() } {
-            dev_warn!("failed to close the universe: {err}");
-            // Send the error to the monitor using the pipe.
-            if let Some(error_code) = err.raw_os_error() {
-                errpipe_tx.write(&error_code).ok();
-            }
-
-            // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-            _exit(1);
-        }
-
-        let err = command.exec();
-
-        dev_warn!("failed to execute command: {err}");
-        // If `exec` returns, it means that executing the command failed. Send the error to the
-        // monitor using the pipe.
-        if let Some(error_code) = err.raw_os_error() {
-            errpipe_tx.write(&error_code).ok();
-        }
-
-        // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-        _exit(1);
+        exec_command(file_closer, command, original_set, errpipe_tx);
     };
 
     dev_info!("executed command with pid {command_pid}");

src/exec/use_pty/monitor.rs

@@ -1,4 +1,4 @@
-use std::{convert::Infallible, ffi::c_int, io, os::unix::process::CommandExt, process::Command};
+use std::{convert::Infallible, ffi::c_int, io, process::Command};
 
 use crate::exec::{opt_fmt, signal_fmt};
 use crate::system::signal::{
@@ -189,11 +189,11 @@ pub(super) fn exec_monitor(
 }
 
 fn exec_command(
-    mut command: Command,
+    command: Command,
     foreground: bool,
     pty_follower: PtyFollower,
     file_closer: FileCloser,
-    mut errpipe_tx: BinPipe<i32, i32>,
+    errpipe_tx: BinPipe<i32, i32>,
     original_set: Option<SignalSet>,
 ) -> ! {
     // FIXME (ogsudo): Do any additional configuration that needs to be run after `fork` but before `exec`
@@ -212,37 +212,7 @@ fn exec_command(
     // Done with the pty follower.
     drop(pty_follower);
 
-    // Restore the signal mask now that the handlers have been setup.
-    if let Some(set) = original_set {
-        if let Err(err) = set.set_mask() {
-            dev_warn!("cannot restore signal mask: {err}");
-        }
-    }
-
-    // SAFETY: We immediately exec after this call and if the exec fails we only access stderr
-    // and errpipe before exiting without running atexit handlers using _exit
-    if let Err(err) = unsafe { file_closer.close_the_universe() } {
-        dev_warn!("failed to close the universe: {err}");
-        // Send the error to the monitor using the pipe.
-        if let Some(error_code) = err.raw_os_error() {
-            errpipe_tx.write(&error_code).ok();
-        }
-
-        // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-        _exit(1);
-    }
-
-    let err = command.exec();
-
-    dev_warn!("failed to execute command: {err}");
-    // If `exec_command` returns, it means that executing the command failed. Send the error to
-    // the monitor using the pipe.
-    if let Some(error_code) = err.raw_os_error() {
-        errpipe_tx.write(&error_code).ok();
-    }
-
-    // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-    _exit(1);
+    crate::exec::exec_command(file_closer, command, original_set, errpipe_tx)
 }
 
 struct MonitorClosure<'a> {