feat: implement perror_nix

Similar to `perror_io`, we don't need to make a libc call for `nix`
results, since the error variant contains the errno, from which a static
mapping to an error message exists. Avoid using `perror` and instead use
`perror_io` or `perror_nix` as appropriate where possible.

The `perror_io` and `perror_nix` functions could be combined by
implementing `fish_printf::ToArg` for `nix::errno::Errno`, but such a
function would violate type safety, as it would allow passing any
formattable argument, not necessarily limited to functions with a `%s`
formatting.

Part of #12502

src/builtins/ulimit.rs

@@ -4,7 +4,7 @@
 use nix::errno::Errno;
 use nix::sys::resource::Resource as ResourceEnum;
 
-use crate::wutil::perror;
+use crate::wutil::perror_nix;
 use fish_fallback::{fish_wcswidth, wcscasecmp};
 
 use super::prelude::*;
@@ -99,7 +99,7 @@ fn convert_resource(resource: c_uint) -> ResourceEnum {
 /// Calls getrlimit.
 fn getrlimit(resource: c_uint) -> Option<(rlim_t, rlim_t)> {
     nix::sys::resource::getrlimit(convert_resource(resource))
-        .map_err(|_| perror("getrlimit"))
+        .map_err(|e| perror_nix("getrlimit", e))
         .ok()
 }
 

src/exec.rs

@@ -44,7 +44,7 @@
 use crate::threads::{ThreadPool, is_forked_child};
 use crate::trace::trace_if_enabled_with_args;
 use crate::tty_handoff::TtyHandoff;
-use crate::wutil::{fish_wcstol, perror};
+use crate::wutil::{fish_wcstol, perror_io};
 use errno::{errno, set_errno};
 use fish_wcstringutil::{wcs2bytes, wcs2zstring};
 use fish_widestring::ToWString as _;
@@ -625,7 +625,7 @@ fn skip_err(&self) -> bool {
         if !f.skip_out() {
             if let Err(err) = write_loop(&f.src_outfd, &f.outdata) {
                 if err.raw_os_error() != Some(EPIPE) {
-                    perror("write");
+                    perror_io("write", &err);
                 }
                 if status.is_success() {
                     status = ProcStatus::from_exit_code(1);
@@ -635,7 +635,7 @@ fn skip_err(&self) -> bool {
         if !f.skip_err() {
             if let Err(err) = write_loop(&f.src_errfd, &f.errdata) {
                 if err.raw_os_error() != Some(EPIPE) {
-                    perror("write");
+                    perror_io("write", &err);
                 }
                 if status.is_success() {
                     status = ProcStatus::from_exit_code(1);

src/fd_monitor.rs

@@ -10,7 +10,7 @@
 use crate::fd_readable_set::{FdReadableSet, Timeout};
 use crate::flog::flog;
 use crate::threads::assert_is_background_thread;
-use crate::wutil::perror;
+use crate::wutil::{perror, perror_nix};
 use errno::errno;
 use libc::{EAGAIN, EINTR, EWOULDBLOCK};
 
@@ -126,7 +126,7 @@ pub fn post(&self) {
         if let Err(err) = ret {
             // EAGAIN occurs if either the pipe buffer is full or the eventfd overflows (very unlikely).
             if ![nix::Error::EAGAIN, nix::Error::EWOULDBLOCK].contains(&err) {
-                perror("write");
+                perror_nix("write", err);
             }
         }
     }

src/fds.rs

@@ -2,6 +2,7 @@
 use crate::prelude::*;
 use crate::signal::signal_check_cancel;
 use crate::wutil::perror;
+use crate::wutil::perror_nix;
 use cfg_if::cfg_if;
 use fish_wcstringutil::wcs2zstring;
 use libc::{EINTR, F_GETFD, F_GETFL, F_SETFD, F_SETFL, FD_CLOEXEC, O_NONBLOCK, c_int};
@@ -96,13 +97,10 @@ fn heightenize_fd(fd: OwnedFd, input_has_cloexec: bool) -> nix::Result<OwnedFd>
     }
 
     // Here we are asking the kernel to give us a cloexec fd.
-    let newfd = match nix::fcntl::fcntl(&fd, FcntlArg::F_DUPFD_CLOEXEC(FIRST_HIGH_FD)) {
+    let newfd =
-        Ok(newfd) => newfd,
+        nix::fcntl::fcntl(&fd, FcntlArg::F_DUPFD_CLOEXEC(FIRST_HIGH_FD)).inspect_err(|&err| {
-        Err(err) => {
+            perror_nix("fcntl", err);
-            perror("fcntl");
+        })?;
-            return Err(err);
-        }
-    };
 
     Ok(unsafe { OwnedFd::from_raw_fd(newfd) })
 }
@@ -218,7 +216,7 @@ pub fn exec_close(fd: RawFd) {
 }
 
 /// Mark an fd as nonblocking
-pub fn make_fd_nonblocking(fd: RawFd) -> Result<(), io::Error> {
+pub fn make_fd_nonblocking(fd: RawFd) -> std::io::Result<()> {
     let flags = unsafe { libc::fcntl(fd, F_GETFL, 0) };
     let nonblocking = (flags & O_NONBLOCK) == O_NONBLOCK;
     if !nonblocking {

src/parser.rs

@@ -32,7 +32,7 @@
 use crate::proc::{JobGroupRef, JobList, JobRef, Pid, ProcStatus, job_reap};
 use crate::signal::{Signal, signal_check_cancel, signal_clear_cancel};
 use crate::wait_handle::WaitHandleStore;
-use crate::wutil::perror;
+use crate::wutil::perror_nix;
 use crate::{flog, flogf, function};
 use assert_matches::assert_matches;
 use fish_util::get_time;
@@ -479,8 +479,8 @@ pub fn new(variables: EnvStack, cancel_behavior: CancelBehavior) -> Parser {
             Ok(fd) => {
                 result.libdata_mut().cwd_fd = Some(Arc::new(fd));
             }
-            Err(_) => {
+            Err(err) => {
-                perror("Unable to open the current working directory");
+                perror_nix("Unable to open the current working directory", err);
             }
         }
 

src/proc.rs

@@ -19,7 +19,7 @@
 use crate::signal::{Signal, signal_set_handlers_once};
 use crate::topic_monitor::{GenerationsList, Topic, topic_monitor_principal};
 use crate::wait_handle::{InternalJobId, WaitHandle, WaitHandleRef, WaitHandleStore};
-use crate::wutil::{perror, wbasename};
+use crate::wutil::{perror_nix, wbasename};
 use cfg_if::cfg_if;
 use fish_widestring::ToWString;
 use libc::{
@@ -853,8 +853,8 @@ pub fn resume(&self) -> bool {
     /// Return true on success, false on failure.
     pub fn signal(&self, signal: NixSignal) -> bool {
         if let Some(pgid) = self.group().get_pgid() {
-            if killpg(pgid.as_nix_pid(), signal).is_err() {
+            if let Err(err) = killpg(pgid.as_nix_pid(), signal) {
-                perror(&format!("killpg({pgid}, {})", signal.as_str()));
+                perror_nix(&format!("killpg({pgid}, {})", signal.as_str()), err);
                 return false;
             }
         } else {

src/reader/reader.rs

@@ -113,7 +113,7 @@
     TtyHandoff, get_tty_protocols_active, initialize_tty_protocols, safe_deactivate_tty_protocols,
 };
 use crate::wildcard::wildcard_has;
-use crate::wutil::{fstat, perror, wstat};
+use crate::wutil::{fstat, perror, perror_nix, wstat};
 use crate::{abbrs, event, function};
 use assert_matches::assert_matches;
 use errno::{Errno, errno};
@@ -2650,15 +2650,14 @@ fn readline(
             // The order of the two conditions below is important. Try to restore the mode
             // in all cases, but only complain if interactive.
             if let Some(old_modes) = old_modes {
-                if tcsetattr(
+                if let Err(err) = tcsetattr(
                     unsafe { BorrowedFd::borrow_raw(self.conf.inputfd) },
                     SetArg::TCSANOW,
                     &old_modes,
-                )
+                ) {
-                .is_err()
+                    if is_interactive_session() {
-                    && is_interactive_session()
+                        perror_nix("tcsetattr", err);
-                {
+                    }
-                    perror("tcsetattr");
                 }
             }
             Outputter::stdoutput().borrow_mut().reset_text_face();
@@ -4786,13 +4785,13 @@ fn term_donate(quiet: bool /* = false */) {
         ) {
             Ok(_) => (),
             Err(nix::Error::EINTR) => continue,
-            Err(_) => {
+            Err(err) => {
                 if !quiet {
                     flog!(
                         warning,
                         wgettext!("Could not set terminal mode for new job")
                     );
-                    perror("tcsetattr");
+                    perror_nix("tcsetattr", err);
                 }
                 break;
             }
@@ -4818,25 +4817,24 @@ pub fn term_copy_modes() {
 }
 
 pub fn set_shell_modes(fd: RawFd, whence: &str) -> bool {
-    let ok = loop {
+    loop {
         match tcsetattr(
             unsafe { BorrowedFd::borrow_raw(fd) },
             SetArg::TCSANOW,
             &shell_modes(),
         ) {
-            Ok(_) => break true,
+            Ok(_) => return true,
             Err(nix::Error::EINTR) => continue,
-            Err(_) => break false,
+            Err(err) => {
+                perror_nix("tcsetattr", err);
+                flog!(
+                    warning,
+                    wgettext_fmt!("Failed to set terminal mode (%s)", whence)
+                );
+                return false;
+            }
         }
-    };
-    if !ok {
-        perror("tcsetattr");
-        flog!(
-            warning,
-            wgettext_fmt!("Failed to set terminal mode (%s)", whence)
-        );
     }
-    ok
 }
 
 pub fn set_shell_modes_temporarily(inputfd: RawFd) -> Option<Termios> {
@@ -4946,8 +4944,8 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) {
             }
 
             // Try stopping us.
-            if killpg(nix::unistd::Pid::from_raw(shell_pgid), Signal::SIGTTIN).is_err() {
+            if let Err(err) = killpg(nix::unistd::Pid::from_raw(shell_pgid), Signal::SIGTTIN) {
-                perror("killpg(shell_pgid, SIGTTIN)");
+                perror_nix("killpg(shell_pgid, SIGTTIN)", err);
                 exit_without_destructors(1);
             }
         }

src/tty_handoff.rs

@@ -17,7 +17,7 @@
     KittyKeyboardProgressiveEnhancementsEnable, ModifyOtherKeysDisable, ModifyOtherKeysEnable,
 };
 use crate::threads::assert_is_main_thread;
-use crate::wutil::{perror, wcstoi};
+use crate::wutil::{perror, perror_nix, wcstoi};
 use libc::{EINVAL, ENOTTY, EPERM, STDIN_FILENO, WNOHANG};
 use nix::sys::termios::tcgetattr;
 use nix::unistd::getpgrp;
@@ -417,7 +417,7 @@ pub fn save_tty_modes(&mut self) {
             }
             Err(err) => {
                 if err != nix::Error::ENOTTY {
-                    perror("tcgetattr");
+                    perror_nix("tcgetattr", err);
                 }
             }
         }

src/wutil/mod.rs

@@ -81,6 +81,10 @@ pub fn perror(s: &str) {
     let _ = stderr.write_all(b"\n");
 }
 
+pub fn perror_nix(s: &str, e: nix::errno::Errno) {
+    eprintf!("%s: %s\n", s, e.desc());
+}
+
 pub fn perror_io(s: &str, e: &io::Error) {
     eprintf!("%s: %s\n", s, e);
 }