diff --git a/src/bin/fish.rs b/src/bin/fish.rs index 2f7cb0ddc..ac5369db2 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -42,7 +42,7 @@ history::{self, start_private_mode}, io::IoChain, locale::set_libc_locales, - nix::{RUsage, getpid, getrusage, isatty}, + nix::{RUsage, getrusage, isatty}, panic::panic_handler, parse_constants::{ParseErrorList, ParseTreeFlags}, parse_tree::ParsedSource, @@ -63,7 +63,7 @@ }; use fish_wcstringutil::wcs2bytes; use libc::STDIN_FILENO; -use nix::unistd::AccessFlags; +use nix::unistd::{AccessFlags, getpid}; use std::ffi::{OsStr, OsString}; use std::fs::File; use std::os::unix::prelude::*; @@ -605,7 +605,10 @@ fn throwing_main() -> i32 { parser.get_last_status() }; - event::fire(parser, Event::process_exit(Pid::new(getpid()), exit_status)); + event::fire( + parser, + Event::process_exit(Pid::from_nix_pid_unchecked(getpid()), exit_status), + ); // Trigger any exit handlers. event::fire_generic( diff --git a/src/builtins/function.rs b/src/builtins/function.rs index cb4cf7ce3..3e5cc636d 100644 --- a/src/builtins/function.rs +++ b/src/builtins/function.rs @@ -7,12 +7,12 @@ use crate::event::{self, EventDescription, EventHandler}; use crate::function; use crate::global_safety::RelaxedAtomicBool; -use crate::nix::getpid; use crate::parse_execution::varname_error; use crate::parse_tree::NodeRef; use crate::parser_keywords::parser_keywords_is_reserved; use crate::proc::Pid; use crate::signal::Signal; +use nix::unistd::getpid; use std::sync::Arc; struct FunctionCmdOpts { @@ -171,7 +171,7 @@ fn add_named_argument( } e = EventDescription::CallerExit { caller_id }; } else if opt == 'p' && woptarg == "%self" { - let pid = Pid::new(getpid()); + let pid = Pid::from_nix_pid_unchecked(getpid()); e = EventDescription::ProcessExit { pid: Some(pid) }; } else { let pid = parse_pid_may_be_zero(streams, cmd, woptarg)?; diff --git a/src/env/environment.rs b/src/env/environment.rs index 06e6a0736..4ea48f951 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -16,7 +16,6 @@ use crate::global_safety::RelaxedAtomicBool; use crate::input::{FISH_BIND_MODE_VAR, init_input}; use crate::localization::wgettext; -use crate::nix::getpid; use crate::null_terminated_array::OwningNullTerminatedArray; use crate::path::{ path_emit_config_directory_messages, path_get_cache, path_get_config, path_get_data, @@ -31,7 +30,7 @@ use libc::c_int; use nix::{ NixPath as _, - unistd::{Uid, User, gethostname}, + unistd::{Uid, User, gethostname, getpid}, }; use std::collections::HashMap; use std::ffi::CStr; @@ -662,7 +661,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool vars.set_one(L!("FISH_VERSION"), global_mode, version); // Set the $fish_pid variable. - vars.set_one(L!("fish_pid"), global_mode, getpid().to_wstring()); + vars.set_one(L!("fish_pid"), global_mode, getpid().as_raw().to_wstring()); // Set the $hostname variable let hostname: WString = gethostname().map_or("fish".into(), osstr2wcstring); diff --git a/src/exec.rs b/src/exec.rs index 3ece4cffe..e6081aebe 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -17,26 +17,26 @@ BorrowedFdFile, PIPE_ERROR, make_autoclose_pipes, make_fd_blocking, open_cloexec, }; use crate::flog::{flog, flogf}; -use crate::fork_exec::PATH_BSHELL; -use crate::fork_exec::blocked_signals_for_job; -use crate::fork_exec::postfork::{ - child_setup_process, execute_fork, execute_setpgid, report_setpgid_error, - safe_report_exec_error, -}; #[cfg(have_posix_spawn)] use crate::fork_exec::spawn::PosixSpawner; +use crate::fork_exec::{ + PATH_BSHELL, blocked_signals_for_job, + postfork::{ + child_setup_process, execute_fork, execute_setpgid, report_setpgid_error, + safe_report_exec_error, + }, +}; use crate::function::{self, FunctionProperties}; use crate::io::{ BufferedOutputStream, FdOutputStream, IoBufferfill, IoChain, IoClose, IoMode, IoPipe, IoStreams, OutputStream, SeparatedBuffer, StringOutputStream, }; -use crate::nix::{getpid, isatty}; +use crate::nix::isatty; use crate::null_terminated_array::OwningNullTerminatedArray; use crate::parser::{Block, BlockId, BlockType, EvalRes, Parser, ParserEnvSetMode}; use crate::prelude::*; -use crate::proc::Pid; use crate::proc::{ - InternalProc, Job, JobGroupRef, ProcStatus, Process, ProcessType, hup_jobs, + InternalProc, Job, JobGroupRef, Pid, ProcStatus, Process, ProcessType, hup_jobs, is_interactive_session, jobs_requiring_warning_on_exit, no_exec, print_exit_warning_for_jobs, }; use crate::reader::{reader_run_count, safe_restore_term_mode}; @@ -52,18 +52,22 @@ EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO, }; -use nix::fcntl::OFlag; -use nix::sys::stat; -use nix::unistd::getpgrp; -use std::ffi::CStr; -use std::io::{Read as _, Write as _}; -use std::mem::MaybeUninit; -use std::num::NonZeroU32; -use std::os::fd::{AsRawFd as _, FromRawFd as _, OwnedFd, RawFd}; -use std::slice; -use std::sync::{ - Arc, OnceLock, - atomic::{AtomicUsize, Ordering}, +use nix::{ + fcntl::OFlag, + sys::stat, + unistd::{getpgrp, getpid}, +}; +use std::{ + ffi::CStr, + io::{Read as _, Write as _}, + mem::MaybeUninit, + num::NonZeroU32, + os::fd::{AsRawFd as _, FromRawFd as _, OwnedFd, RawFd}, + slice, + sync::{ + Arc, OnceLock, + atomic::{AtomicUsize, Ordering}, + }, }; /// The singleton shared exec thread pool. @@ -734,7 +738,11 @@ fn fork_child_for_process( // Determine the child pid. let is_parent = fork_res > 0; - let pid: libc::pid_t = if is_parent { fork_res } else { getpid() }; + let pid: libc::pid_t = if is_parent { + fork_res + } else { + getpid().as_raw() + }; // Send the process to a new pgroup if requested. // Do this in BOTH the parent and child, to resolve the well-known race. diff --git a/src/expand.rs b/src/expand.rs index 84311e446..392f3b312 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -29,7 +29,7 @@ use fish_util::wcsfilecmp_glob; use fish_wcstringutil::{join_strings, trim}; use fish_widestring::char_offset; -use nix::unistd::User; +use nix::unistd::{User, getpid}; bitflags! { /// Set of flags controlling expansions. @@ -1167,7 +1167,7 @@ fn expand_home_directory(input: &mut WString, vars: &dyn Environment) { /// Expand the %self escape. Note this can only come at the beginning of the string. fn expand_percent_self(input: &mut WString) { if input.as_char_slice().first() == Some(&PROCESS_EXPAND_SELF) { - input.replace_range(0..1, &crate::nix::getpid().to_wstring()); + input.replace_range(0..1, &getpid().as_raw().to_wstring()); } } diff --git a/src/fork_exec/postfork.rs b/src/fork_exec/postfork.rs index 564e1d10c..085532e9e 100644 --- a/src/fork_exec/postfork.rs +++ b/src/fork_exec/postfork.rs @@ -2,12 +2,12 @@ // Everything in this module must be async-signal safe. // That means no locking, no allocating, no freeing memory, etc! use super::flog_safe::flog_safe; -use crate::nix::getpid; use crate::null_terminated_array::OwningNullTerminatedArray; use crate::redirection::Dup2List; use crate::signal::signal_reset_handlers; use crate::{common::exit_without_destructors, wutil::fstat}; use libc::{O_RDONLY, pid_t}; +use nix::unistd::getpid; use std::ffi::CStr; use std::num::NonZeroU32; use std::os::unix::fs::MetadataExt as _; @@ -175,7 +175,7 @@ pub fn child_setup_process( unsafe { libc::signal(libc::SIGTTIN, libc::SIG_IGN); libc::signal(libc::SIGTTOU, libc::SIG_IGN); - let _ = libc::tcsetpgrp(libc::STDIN_FILENO, getpid()); + let _ = libc::tcsetpgrp(libc::STDIN_FILENO, getpid().as_raw()); } } if let Some(sigmask) = sigmask { diff --git a/src/nix.rs b/src/nix.rs index bac50cebe..8268e855c 100644 --- a/src/nix.rs +++ b/src/nix.rs @@ -24,9 +24,6 @@ fn as_duration(&self) -> Duration { } } -pub fn getpid() -> i32 { - unsafe { libc::getpid() } -} pub fn isatty(fd: i32) -> bool { // This returns false if the fd is valid but not a tty, or is invalid. // No place we currently call it really cares about the difference. diff --git a/src/proc.rs b/src/proc.rs index f261ecd14..a30124ef1 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -302,6 +302,14 @@ pub fn as_pid_t(&self) -> libc::pid_t { pub fn as_nix_pid(&self) -> nix::unistd::Pid { nix::unistd::Pid::from_raw(self.as_pid_t()) } + + #[inline(always)] + // The nix Pid type does not guarantee non-zero values. + // It is safe to use this on the result of nix's getpid, since getpid does not fail, and the ID + // of the calling process is never 0. + pub fn from_nix_pid_unchecked(pid: nix::unistd::Pid) -> Self { + Self::new(pid.as_raw()) + } } impl std::fmt::Display for Pid { diff --git a/src/reader/reader.rs b/src/reader/reader.rs index 62b075aaf..a2916d3c1 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -68,7 +68,7 @@ use crate::io::IoChain; use crate::key::ViewportPosition; use crate::kill::{kill_add, kill_replace, kill_yank, kill_yank_rotate}; -use crate::nix::{getpid, isatty}; +use crate::nix::isatty; use crate::operation_context::{OperationContext, get_bg_context}; use crate::pager::{PageRendering, Pager, SelectionMotion}; use crate::panic::AT_EXIT; @@ -128,17 +128,18 @@ }; use fish_widestring::ELLIPSIS_CHAR; use libc::{ - _POSIX_VDISABLE, EIO, EISDIR, ENOTTY, EPERM, ESRCH, O_NONBLOCK, O_RDONLY, SIGINT, - STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO, VMIN, VQUIT, VSUSP, VTIME, c_char, + _POSIX_VDISABLE, EIO, EISDIR, ENOTTY, ESRCH, O_NONBLOCK, O_RDONLY, SIGINT, STDERR_FILENO, + STDIN_FILENO, STDOUT_FILENO, VMIN, VQUIT, VSUSP, VTIME, c_char, }; -use nix::sys::termios::{self, SetArg, Termios, tcgetattr, tcsetattr}; +use nix::unistd::setpgid; use nix::{ fcntl::OFlag, sys::{ signal::{Signal, killpg}, stat::Mode, + termios::{self, SetArg, Termios, tcgetattr, tcsetattr}, }, - unistd::getpgrp, + unistd::{getpgrp, getpid}, }; use std::{ borrow::Cow, @@ -4877,7 +4878,7 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) { // In some strange cases the tty may be come preassigned to fish's pid, but not its pgroup. // In that case we simply attempt to claim our own pgroup. // See #7388. - if owner == getpid() { + if owner == getpid().as_raw() { unsafe { libc::setpgid(owner, owner) }; return; } @@ -4937,7 +4938,7 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) { warning, sprintf!( "I appear to be an orphaned process, so I am quitting politely. My pid is %d.", - pid + pid.as_raw() ) ); exit_without_destructors(1); @@ -4956,36 +4957,36 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) { fn reader_interactive_init() { assert_is_main_thread(); - let mut shell_pgid = getpgrp().as_raw(); + let mut shell_pgid = getpgrp(); let shell_pid = getpid(); // Ensure interactive signal handling is enabled. signal_set_handlers_once(true); // Wait until we own the terminal. - acquire_tty_or_exit(shell_pgid); + acquire_tty_or_exit(shell_pgid.as_raw()); // If fish has no valid pgroup (possible with firejail, see #5295) or is interactive, // ensure it owns the terminal. Also see #5909, #7060. - if shell_pgid == 0 || (is_interactive_session() && shell_pgid != shell_pid) { + if shell_pgid.as_raw() == 0 || (is_interactive_session() && shell_pgid != shell_pid) { shell_pgid = shell_pid; - if unsafe { libc::setpgid(shell_pgid, shell_pgid) } < 0 { + if let Err(e) = setpgid(shell_pgid, shell_pgid) { // If we're session leader setpgid returns EPERM. The other cases where we'd get EPERM // don't apply as we passed our own pid. // // This should be harmless, so we ignore it. - if errno().0 != EPERM { + if e != nix::errno::Errno::EPERM { flog!( error, wgettext!("Failed to assign shell to its own process group") ); - perror("setpgid"); + perror_nix("setpgid", e); exit_without_destructors(1); } } // Take control of the terminal - if unsafe { libc::tcsetpgrp(STDIN_FILENO, shell_pgid) } == -1 { + if unsafe { libc::tcsetpgrp(STDIN_FILENO, shell_pgid.as_raw()) } == -1 { flog!(error, wgettext!("Failed to take control of the terminal")); perror("tcsetpgrp"); exit_without_destructors(1); diff --git a/src/signal.rs b/src/signal.rs index 224f5ccd9..4837ec432 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -1,6 +1,5 @@ use crate::common::exit_without_destructors; use crate::event::{enqueue_signal, is_signal_observed}; -use crate::nix::getpid; use crate::prelude::*; use crate::reader::{reader_handle_sigint, reader_sighup, safe_restore_term_mode}; use crate::termsize::safe_termsize_invalidate_tty; @@ -9,7 +8,11 @@ use crate::wutil::fish_wcstoi; use errno::{errno, set_errno}; use fish_util::perror; -use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, SigmaskHow, sigprocmask}; +use nix::sys::signal::kill; +use nix::{ + sys::signal::{SaFlags, SigAction, SigHandler, SigSet, SigmaskHow, sigprocmask}, + unistd::getpid, +}; use std::{ mem::MaybeUninit, num::NonZeroI32, @@ -28,7 +31,7 @@ /// and re-raise the signal. Return whether we re-raised the signal. fn reraise_if_forked_child(sig: i32) -> bool { // Don't use is_forked_child: it relies on atfork handlers which may have not yet run. - if getpid() == MAIN_PID.load(Ordering::Relaxed) { + if getpid().as_raw() == MAIN_PID.load(Ordering::Relaxed) { return false; } @@ -199,7 +202,7 @@ fn set_interactive_handlers() { /// Set signal handlers to fish default handlers. pub fn signal_set_handlers(interactive: bool) { // Mark our main pid. - MAIN_PID.store(getpid(), Ordering::Relaxed); + MAIN_PID.store(getpid().as_raw(), Ordering::Relaxed); use libc::SIG_IGN; let nullptr = std::ptr::null_mut(); @@ -243,7 +246,7 @@ pub fn signal_set_handlers(interactive: bool) { // The workaround is to send ourselves a SIGCHLD signal now, to force the allocation to happen. // As no child is associated with this signal, it is OK if it is dropped, so long as the // allocation happens. - unsafe { libc::kill(getpid(), libc::SIGCHLD) }; + let _ = kill(getpid(), nix::sys::signal::Signal::SIGCHLD); } }