Remove some uses of unsafe

libc::getpid and getpgrp *cannot fail*, so an unsafe declaration here
is just noise.

I mean sure, if your libc is broken and these fail, but at that point
I'm comfortable declaring your computer a platypus that we do not need
to support.
This commit is contained in:
Fabian Boehm 2025-01-18 09:47:03 +01:00
parent 79320e3ba7
commit 1123169bbd
6 changed files with 16 additions and 17 deletions

View File

@ -1491,7 +1491,7 @@ pub fn restore_term_foreground_process_group_for_exit() {
// failure because doing so is unlikely to be noticed. // failure because doing so is unlikely to be noticed.
// Safety: All of getpgrp, signal, and tcsetpgrp are async-signal-safe. // Safety: All of getpgrp, signal, and tcsetpgrp are async-signal-safe.
let initial_fg_process_group = INITIAL_FG_PROCESS_GROUP.load(Ordering::Relaxed); let initial_fg_process_group = INITIAL_FG_PROCESS_GROUP.load(Ordering::Relaxed);
if initial_fg_process_group > 0 && initial_fg_process_group != unsafe { libc::getpgrp() } { if initial_fg_process_group > 0 && initial_fg_process_group != crate::nix::getpgrp() {
unsafe { unsafe {
libc::signal(SIGTTOU, SIG_IGN); libc::signal(SIGTTOU, SIG_IGN);
libc::tcsetpgrp(STDIN_FILENO, initial_fg_process_group); libc::tcsetpgrp(STDIN_FILENO, initial_fg_process_group);

View File

@ -675,7 +675,7 @@ fn fork_child_for_process(
// Claim the tty from fish, if the job wants it and we are the pgroup leader. // Claim the tty from fish, if the job wants it and we are the pgroup leader.
let claim_tty_from = if p.leads_pgrp && job.group().wants_terminal() { let claim_tty_from = if p.leads_pgrp && job.group().wants_terminal() {
// getpgrp(2) cannot fail and always returns the (positive) caller's pgid // getpgrp(2) cannot fail and always returns the (positive) caller's pgid
Some(NonZeroU32::new(unsafe { libc::getpgrp() } as u32).unwrap()) Some(NonZeroU32::new(crate::nix::getpgrp() as u32).unwrap())
} else { } else {
None None
}; };
@ -702,11 +702,7 @@ fn fork_child_for_process(
// Record the pgroup if this is the leader. // Record the pgroup if this is the leader.
// Both parent and child attempt to send the process to its new group, to resolve the race. // Both parent and child attempt to send the process to its new group, to resolve the race.
let pid = if is_parent { let pid = if is_parent { pid } else { crate::nix::getpid() };
pid
} else {
unsafe { libc::getpid() }
};
p.set_pid(pid); p.set_pid(pid);
if p.leads_pgrp { if p.leads_pgrp {
job.group().set_pgid(pid); job.group().set_pgid(pid);

View File

@ -1181,7 +1181,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. /// Expand the %self escape. Note this can only come at the beginning of the string.
fn expand_percent_self(input: &mut WString) { fn expand_percent_self(input: &mut WString) {
if input.as_char_slice().first() == Some(&PROCESS_EXPAND_SELF) { if input.as_char_slice().first() == Some(&PROCESS_EXPAND_SELF) {
input.replace_range(0..1, &unsafe { libc::getpid() }.to_wstring()); input.replace_range(0..1, &crate::nix::getpid().to_wstring());
} }
} }

View File

@ -31,6 +31,9 @@ pub fn geteuid() -> u32 {
pub fn getegid() -> u32 { pub fn getegid() -> u32 {
unsafe { libc::getegid() } unsafe { libc::getegid() }
} }
pub fn getpgrp() -> i32 {
unsafe { libc::getpgrp() }
}
pub fn getpid() -> i32 { pub fn getpid() -> i32 {
unsafe { libc::getpid() } unsafe { libc::getpid() }
} }

View File

@ -363,7 +363,7 @@ impl TtyTransfer {
let pgid = jg.get_pgid().unwrap(); let pgid = jg.get_pgid().unwrap();
// It should never be fish's pgroup. // It should never be fish's pgroup.
let fish_pgrp = unsafe { libc::getpgrp() }; let fish_pgrp = crate::nix::getpgrp();
assert!( assert!(
pgid.as_pid_t() != fish_pgrp, pgid.as_pid_t() != fish_pgrp,
"Job should not have fish's pgroup" "Job should not have fish's pgroup"
@ -1363,7 +1363,7 @@ pub fn proc_wait_any(parser: &Parser) {
/// Send SIGHUP to the list `jobs`, excepting those which are in fish's pgroup. /// Send SIGHUP to the list `jobs`, excepting those which are in fish's pgroup.
pub fn hup_jobs(jobs: &JobList) { pub fn hup_jobs(jobs: &JobList) {
let fish_pgrp = unsafe { libc::getpgrp() }; let fish_pgrp = crate::nix::getpgrp();
let mut kill_list = Vec::new(); let mut kill_list = Vec::new();
for j in jobs { for j in jobs {
let Some(pgid) = j.get_pgid() else { continue }; let Some(pgid) = j.get_pgid() else { continue };

View File

@ -96,7 +96,7 @@ use crate::io::IoChain;
use crate::key::ViewportPosition; use crate::key::ViewportPosition;
use crate::kill::{kill_add, kill_replace, kill_yank, kill_yank_rotate}; use crate::kill::{kill_add, kill_replace, kill_yank, kill_yank_rotate};
use crate::libc::MB_CUR_MAX; use crate::libc::MB_CUR_MAX;
use crate::nix::isatty; use crate::nix::{getpgrp, getpid, isatty};
use crate::operation_context::{get_bg_context, OperationContext}; use crate::operation_context::{get_bg_context, OperationContext};
use crate::output::parse_color; use crate::output::parse_color;
use crate::output::Outputter; use crate::output::Outputter;
@ -845,7 +845,7 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
// Set up our fixed terminal modes once, // Set up our fixed terminal modes once,
// so we don't get flow control just because we inherited it. // so we don't get flow control just because we inherited it.
if is_interactive_session() { if is_interactive_session() {
if unsafe { libc::getpgrp() == libc::tcgetpgrp(STDIN_FILENO) } { if getpgrp() == unsafe { libc::tcgetpgrp(STDIN_FILENO) } {
term_donate(/*quiet=*/ true); term_donate(/*quiet=*/ true);
} }
} }
@ -863,7 +863,7 @@ pub fn reader_deinit(in_signal_handler: bool, restore_foreground_pgroup: bool) {
/// It's important we do this before restore_foreground_process_group, /// It's important we do this before restore_foreground_process_group,
/// otherwise we won't think we own the terminal. /// otherwise we won't think we own the terminal.
pub fn restore_term_mode(in_signal_handler: bool) { pub fn restore_term_mode(in_signal_handler: bool) {
if !is_interactive_session() || unsafe { libc::getpgrp() != libc::tcgetpgrp(STDIN_FILENO) } { if !is_interactive_session() || getpgrp() != unsafe { libc::tcgetpgrp(STDIN_FILENO) } {
return; return;
} }
@ -4239,7 +4239,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 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. // In that case we simply attempt to claim our own pgroup.
// See #7388. // See #7388.
if owner == unsafe { libc::getpid() } { if owner == getpid() {
unsafe { libc::setpgid(owner, owner) }; unsafe { libc::setpgid(owner, owner) };
return; return;
} }
@ -4295,7 +4295,7 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) {
} else { } else {
if check_for_orphaned_process(loop_count, shell_pgid) { if check_for_orphaned_process(loop_count, shell_pgid) {
// We're orphaned, so we just die. Another sad statistic. // We're orphaned, so we just die. Another sad statistic.
let pid = unsafe { libc::getpid() }; let pid = getpid();
FLOG!(warning, wgettext_fmt!("I appear to be an orphaned process, so I am quitting politely. My pid is %d.", pid)); FLOG!(warning, wgettext_fmt!("I appear to be an orphaned process, so I am quitting politely. My pid is %d.", pid));
exit_without_destructors(1); exit_without_destructors(1);
} }
@ -4314,8 +4314,8 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) {
fn reader_interactive_init(parser: &Parser) { fn reader_interactive_init(parser: &Parser) {
assert_is_main_thread(); assert_is_main_thread();
let mut shell_pgid = unsafe { libc::getpgrp() }; let mut shell_pgid = getpgrp();
let shell_pid = unsafe { libc::getpid() }; let shell_pid = getpid();
// Set up key bindings. // Set up key bindings.
init_input(); init_input();