Replace some direct uses of libc with wrappers (#10090)

This removes some spurious unsafe blocks and makes usage a bit nicer

fish-rust/src/builtins/complete.rs

@@ -6,6 +6,7 @@ use crate::complete::{complete_add_wrapper, complete_remove_wrapper, CompletionR
 use crate::ffi;
 use crate::highlight::colorize;
 use crate::highlight::highlight_shell;
+use crate::nix::isatty;
 use crate::parse_constants::ParseErrorList;
 use crate::parse_util::parse_util_detect_errors_in_argument_list;
 use crate::parse_util::{parse_util_detect_errors, parse_util_token_extent};
@@ -201,7 +202,7 @@ fn builtin_complete_print(cmd: &wstr, streams: &mut IoStreams, parser: &Parser)
     let repr = complete_print(cmd);
 
     // colorize if interactive
-    if !streams.out_is_redirected && unsafe { libc::isatty(STDOUT_FILENO) } != 0 {
+    if !streams.out_is_redirected && isatty(STDOUT_FILENO) {
         let mut colors = vec![];
         highlight_shell(&repr, &mut colors, &parser.context(), false, None);
         streams

fish-rust/src/builtins/function.rs

@@ -7,6 +7,7 @@ use crate::event::{self, EventDescription, EventHandler};
 use crate::function;
 use crate::global_safety::RelaxedAtomicBool;
 use crate::io::IoStreams;
+use crate::nix::getpid;
 use crate::parse_tree::NodeRef;
 use crate::parser::Parser;
 use crate::parser_keywords::parser_keywords_is_reserved;
@@ -148,8 +149,7 @@ fn parse_cmd_opts(
                     }
                     e = EventDescription::CallerExit { caller_id };
                 } else if opt == 'p' && woptarg == "%self" {
-                    // Safety: getpid() is always successful.
-                    let pid = unsafe { libc::getpid() };
+                    let pid: i32 = getpid();
                     e = EventDescription::ProcessExit { pid };
                 } else {
                     let pid = fish_wcstoi(woptarg);

fish-rust/src/builtins/path.rs

@@ -4,6 +4,7 @@ use std::os::unix::prelude::{FileTypeExt, MetadataExt};
 use std::time::SystemTime;
 
 use super::prelude::*;
+use crate::nix::{getegid, geteuid};
 use crate::path::path_apply_working_directory;
 use crate::util::wcsfilecmp_glob;
 use crate::wcstringutil::split_string_tok;
@@ -12,7 +13,7 @@ use crate::wutil::{
     INVALID_FILE_ID,
 };
 use bitflags::bitflags;
-use libc::{getegid, geteuid, mode_t, uid_t, F_OK, PATH_MAX, R_OK, S_ISGID, S_ISUID, W_OK, X_OK};
+use libc::{mode_t, F_OK, PATH_MAX, R_OK, S_ISGID, S_ISUID, W_OK, X_OK};
 
 use super::shared::BuiltinCmd;
 
@@ -808,11 +809,9 @@ fn filter_path(opts: &Options, path: &wstr) -> bool {
                 return false;
             } else if perm.contains(PermFlags::SGID) && (md.mode() as mode_t & S_ISGID) == 0 {
                 return false;
-            } else if perm.contains(PermFlags::USER) && (unsafe { geteuid() } != md.uid() as uid_t)
-            {
+            } else if perm.contains(PermFlags::USER) && geteuid() != md.uid() {
                 return false;
-            } else if perm.contains(PermFlags::GROUP) && (unsafe { getegid() } != md.gid() as uid_t)
-            {
+            } else if perm.contains(PermFlags::GROUP) && getegid() != md.gid() {
                 return false;
             }
         }

fish-rust/src/builtins/read.rs

@@ -15,6 +15,7 @@ use crate::env::Environment;
 use crate::env::READ_BYTE_LIMIT;
 use crate::env::{EnvVar, EnvVarFlags};
 use crate::ffi;
+use crate::nix::isatty;
 use crate::reader::ReaderConfig;
 use crate::reader::{reader_pop, reader_push, reader_readline};
 use crate::tokenizer::Tokenizer;
@@ -592,7 +593,7 @@ pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt
     loop {
         buff.clear();
 
-        let stream_stdin_is_a_tty = unsafe { libc::isatty(streams.stdin_fd) } != 0;
+        let stream_stdin_is_a_tty = isatty(streams.stdin_fd);
         if stream_stdin_is_a_tty && !opts.split_null {
             // Read interactively using reader_readline(). This does not support splitting on null.
             exit_res = read_interactive(

fish-rust/src/builtins/source.rs

@@ -3,6 +3,7 @@ use crate::{
     fds::{wopen_cloexec, AutoCloseFd},
     ffi::reader_read_ffi,
     io::IoChain,
+    nix::isatty,
     parser::Block,
 };
 use libc::{c_int, O_RDONLY, S_IFMT, S_IFREG};
@@ -41,7 +42,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O
             return STATUS_CMD_ERROR;
         }
         // Either a bare `source` which means to implicitly read from stdin or an explicit `-`.
-        if argc == optind && unsafe { libc::isatty(streams.stdin_fd) } != 0 {
+        if argc == optind && isatty(streams.stdin_fd) {
             // Don't implicitly read from the terminal.
             return STATUS_CMD_ERROR;
         }

fish-rust/src/builtins/test.rs

@@ -4,6 +4,7 @@ use crate::common;
 mod test_expressions {
     use super::*;
 
+    use crate::nix::isatty;
     use crate::wutil::{
         file_id_for_path, fish_wcswidth, lwstat, waccess, wcstod::wcstod, wcstoi_opts, wstat,
         Error, Options,
@@ -83,10 +84,6 @@ mod test_expressions {
 
         // Return true if the number is a tty().
         fn isatty(&self, streams: &mut IoStreams) -> bool {
-            fn istty(fd: libc::c_int) -> bool {
-                // Safety: isatty cannot crash.
-                unsafe { libc::isatty(fd) > 0 }
-            }
             if self.delta != 0.0 || self.base > i32::MAX as i64 || self.base < i32::MIN as i64 {
                 return false;
             }
@@ -94,14 +91,14 @@ mod test_expressions {
             if bint == 0 {
                 match streams.stdin_fd {
                     -1 => false,
-                    fd => istty(fd),
+                    fd => isatty(fd),
                 }
             } else if bint == 1 {
-                !streams.out_is_redirected && istty(libc::STDOUT_FILENO)
+                !streams.out_is_redirected && isatty(libc::STDOUT_FILENO)
             } else if bint == 2 {
-                !streams.err_is_redirected && istty(libc::STDERR_FILENO)
+                !streams.err_is_redirected && isatty(libc::STDERR_FILENO)
             } else {
-                istty(bint)
+                isatty(bint)
             }
         }
     }
@@ -928,8 +925,7 @@ mod test_expressions {
             }
             Token::filetype_G => {
                 // "-G", for check effective group id
-                // Safety: getegid cannot fail.
-                stat_and(arg, |buf| unsafe { libc::getegid() } == buf.gid())
+                stat_and(arg, |buf| crate::nix::getegid() == buf.gid())
             }
             Token::filetype_g => {
                 // "-g", for set-group-id
@@ -946,10 +942,9 @@ mod test_expressions {
             }
             Token::filetype_O => {
                 // "-O", for check effective user id
-                stat_and(
-                    arg,
-                    |buf: std::fs::Metadata| unsafe { libc::geteuid() } == buf.uid(),
-                )
+                stat_and(arg, |buf: std::fs::Metadata| {
+                    crate::nix::geteuid() == buf.uid()
+                })
             }
             Token::filetype_p => {
                 // "-p", for FIFO

fish-rust/src/env/environment.rs

@@ -13,6 +13,7 @@ use crate::event::Event;
 use crate::ffi;
 use crate::flog::FLOG;
 use crate::global_safety::RelaxedAtomicBool;
+use crate::nix::{geteuid, getpid, isatty};
 use crate::null_terminated_array::OwningNullTerminatedArray;
 use crate::path::{
     path_emit_config_directory_messages, path_get_config, path_get_data, path_make_canonical,
@@ -26,7 +27,7 @@ use crate::wutil::{fish_wcstol, wgetcwd, wgettext};
 use std::sync::atomic::Ordering;
 
 use lazy_static::lazy_static;
-use libc::{c_int, STDOUT_FILENO, _IONBF};
+use libc::{c_int, uid_t, STDOUT_FILENO, _IONBF};
 use once_cell::sync::OnceCell;
 use std::collections::HashMap;
 use std::ffi::CStr;
@@ -433,7 +434,7 @@ fn get_hostname_identifier() -> Option<WString> {
 
 /// Set up the USER and HOME variable.
 fn setup_user(vars: &EnvStack) {
-    let uid = unsafe { libc::geteuid() };
+    let uid: uid_t = geteuid();
     let user_var = vars.get_unless_empty(L!("USER"));
 
     let mut userinfo: MaybeUninit<libc::passwd> = MaybeUninit::uninit();
@@ -619,11 +620,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
 
     // Set $USER, $HOME and $EUID
     // This involves going to passwd and stuff.
-    vars.set_one(
-        L!("EUID"),
-        EnvMode::GLOBAL,
-        unsafe { libc::geteuid() }.to_wstring(),
-    );
+    vars.set_one(L!("EUID"), EnvMode::GLOBAL, geteuid().to_wstring());
     setup_user(vars);
 
     let user_config_dir = path_get_config();
@@ -657,11 +654,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool
     vars.set_one(L!("FISH_VERSION"), EnvMode::GLOBAL, version);
 
     // Set the $fish_pid variable.
-    vars.set_one(
-        L!("fish_pid"),
-        EnvMode::GLOBAL,
-        unsafe { libc::getpid() }.to_wstring(),
-    );
+    vars.set_one(L!("fish_pid"), EnvMode::GLOBAL, getpid().to_wstring());
 
     // Set the $hostname variable
     let hostname: WString = get_hostname_identifier().unwrap_or("fish".into());
@@ -797,7 +790,7 @@ pub fn misc_init() {
     // If stdout is open on a tty ensure stdio is unbuffered. That's because those functions might
     // be intermixed with `write()` calls and we need to ensure the writes are not reordered. See
     // issue #3748.
-    if unsafe { libc::isatty(STDOUT_FILENO) } != 0 {
+    if isatty(STDOUT_FILENO) {
         let _ = std::io::stdout().flush();
         unsafe { libc::setvbuf(stdout_stream(), std::ptr::null_mut(), _IONBF, 0) };
     }

fish-rust/src/exec.rs

@@ -30,6 +30,7 @@ use crate::io::{
     BufferedOutputStream, FdOutputStream, IoBufferfill, IoChain, IoClose, IoMode, IoPipe,
     IoStreams, OutputStream, SeparatedBuffer, StringOutputStream,
 };
+use crate::nix::isatty;
 use crate::null_terminated_array::{
     null_terminated_array_length, AsNullTerminatedArray, OwningNullTerminatedArray,
 };
@@ -54,7 +55,7 @@ use cxx::{CxxWString, UniquePtr};
 use errno::{errno, set_errno};
 use libc::c_int;
 use libc::{
-    c_char, isatty, EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, O_NOCTTY,
+    c_char, EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, O_NOCTTY,
     O_RDONLY, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO,
 };
 use std::ffi::CStr;
@@ -1383,7 +1384,7 @@ fn allow_exec_with_background_jobs(parser: &Parser) -> bool {
     // Compare run counts, so we only warn once.
     let current_run_count = reader_run_count();
     let last_exec_run_count = &mut parser.libdata_mut().pods.last_exec_run_counter;
-    if unsafe { isatty(STDIN_FILENO) } != 0 && current_run_count - 1 != *last_exec_run_count {
+    if isatty(STDIN_FILENO) && current_run_count - 1 != *last_exec_run_count {
         print_exit_warning_for_jobs(&bgs);
         *last_exec_run_count = current_run_count;
         false

fish-rust/src/fish.rs

@@ -39,6 +39,7 @@ use crate::{
     function, future_feature_flags as features, history,
     history::start_private_mode,
     io::IoChain,
+    nix::{getpid, isatty},
     parse_constants::{ParseErrorList, ParseTreeFlags},
     parse_tree::ParsedSource,
     parse_util::parse_util_detect_errors_in_ast,
@@ -472,10 +473,7 @@ fn fish_parse_opt(args: &mut [WString], opts: &mut FishCmdOpts) -> usize {
     // We are an interactive session if we have not been given an explicit
     // command or file to execute and stdin is a tty. Note that the -i or
     // --interactive options also force interactive mode.
-    if opts.batch_cmds.is_empty()
-        && optind == args.len()
-        && unsafe { libc::isatty(libc::STDIN_FILENO) != 0 }
-    {
+    if opts.batch_cmds.is_empty() && optind == args.len() && isatty(libc::STDIN_FILENO) {
         set_interactive_session(true);
     }
 
@@ -708,7 +706,7 @@ fn main() -> i32 {
         parser.libdata_mut().pods.exit_current_script = false;
     } else if my_optind == args.len() {
         // Implicitly interactive mode.
-        if opts.no_exec && unsafe { libc::isatty(libc::STDIN_FILENO) != 0 } {
+        if opts.no_exec && isatty(libc::STDIN_FILENO) {
             FLOG!(
                 error,
                 "no-execute mode enabled and no script given. Exiting"
@@ -776,10 +774,7 @@ fn main() -> i32 {
         parser.get_last_status()
     };
 
-    event::fire(
-        parser,
-        Event::process_exit(unsafe { libc::getpid() }, exit_status),
-    );
+    event::fire(parser, Event::process_exit(getpid(), exit_status));
 
     // Trigger any exit handlers.
     event::fire_generic(

fish-rust/src/fork_exec/postfork.rs

@@ -3,6 +3,7 @@
 // That means no locking, no allocating, no freeing memory, etc!
 use super::flog_safe::FLOG_SAFE;
 use crate::common::exit_without_destructors;
+use crate::nix::getpid;
 use crate::redirection::Dup2List;
 use crate::signal::signal_reset_handlers;
 use libc::{c_char, c_int, pid_t};
@@ -189,7 +190,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, libc::getpid());
+            let _ = libc::tcsetpgrp(libc::STDIN_FILENO, getpid());
         }
     }
     if let Some(sigmask) = sigmask {

fish-rust/src/io.rs

@@ -8,6 +8,7 @@ use crate::fds::{
 };
 use crate::flog::{should_flog, FLOG, FLOGF};
 use crate::global_safety::RelaxedAtomicBool;
+use crate::nix::isatty;
 use crate::path::path_apply_working_directory;
 use crate::proc::JobGroupRef;
 use crate::redirection::{RedirectionMode, RedirectionSpecList};
@@ -1014,7 +1015,7 @@ impl<'a> IoStreams<'a> {
         }
     }
     pub fn out_is_terminal(&self) -> bool {
-        !self.out_is_redirected && unsafe { libc::isatty(STDOUT_FILENO) == 1 }
+        !self.out_is_redirected && isatty(STDOUT_FILENO)
     }
 }
 

fish-rust/src/nix.rs

@@ -22,3 +22,18 @@ impl TimevalExt for libc::timeval {
         timeval_to_duration(self)
     }
 }
+
+pub fn geteuid() -> u32 {
+    unsafe { libc::geteuid() }
+}
+pub fn getegid() -> u32 {
+    unsafe { libc::getegid() }
+}
+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.
+    return unsafe { libc::isatty(fd) } == 1;
+}

fish-rust/src/signal.rs

@@ -2,6 +2,7 @@ use std::num::NonZeroI32;
 
 use crate::common::{exit_without_destructors, restore_term_foreground_process_group_for_exit};
 use crate::event::{enqueue_signal, is_signal_observed};
+use crate::nix::getpid;
 use crate::reader::{reader_handle_sigint, reader_sighup};
 use crate::termsize::TermsizeContainer;
 use crate::topic_monitor::{generation_t, topic_monitor_principal, topic_t, GenerationsList};
@@ -77,9 +78,7 @@ static MAIN_PID: AtomicI32 = AtomicI32::new(0);
 /// 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.
-    // Safety: getpid() is async-signal-safe.
-    let pid = unsafe { libc::getpid() };
-    if pid == MAIN_PID.load(Ordering::Relaxed) {
+    if getpid() == MAIN_PID.load(Ordering::Relaxed) {
         return false;
     }
 
@@ -250,7 +249,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(unsafe { libc::getpid() }, Ordering::Relaxed);
+    MAIN_PID.store(getpid(), Ordering::Relaxed);
 
     use libc::SIG_IGN;
     let nullptr = std::ptr::null_mut();
@@ -294,7 +293,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(libc::getpid(), libc::SIGCHLD) };
+        unsafe { libc::kill(getpid(), libc::SIGCHLD) };
     }
 }