From 04b799c0c5b9eda311e3605891679fa36788e16d Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Sun, 25 Jan 2026 16:48:33 +0100 Subject: [PATCH] nix: use `access` Part of #12380 --- src/bin/fish.rs | 3 ++- src/builtins/path.rs | 18 ++++++++---------- src/builtins/status.rs | 4 ++-- src/builtins/test.rs | 10 +++++----- src/highlight/file_tester.rs | 9 +++++---- src/path.rs | 11 ++++++----- src/wildcard.rs | 15 ++++++++------- src/wutil/mod.rs | 7 ++++--- 8 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/bin/fish.rs b/src/bin/fish.rs index a3fafb778..83df24e70 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -62,6 +62,7 @@ wutil::waccess, }; use libc::STDIN_FILENO; +use nix::unistd::AccessFlags; use std::ffi::{OsStr, OsString}; use std::fs::File; use std::os::unix::prelude::*; @@ -143,7 +144,7 @@ fn source_config_in_directory(parser: &Parser, dir: &wstr) -> bool { // this context so we ignore it. let config_pathname = dir.to_owned() + L!("/config.fish"); let escaped_pathname = escape(dir) + L!("/config.fish"); - if waccess(&config_pathname, libc::R_OK) != 0 { + if waccess(&config_pathname, AccessFlags::R_OK).is_err() { flogf!( config, "not sourcing %s (not readable or does not exist)", diff --git a/src/builtins/path.rs b/src/builtins/path.rs index 754285bea..54e141f40 100644 --- a/src/builtins/path.rs +++ b/src/builtins/path.rs @@ -12,8 +12,8 @@ use bitflags::bitflags; use fish_util::wcsfilecmp_glob; use fish_wcstringutil::split_string_tok; -use libc::{F_OK, PATH_MAX, R_OK, S_ISGID, S_ISUID, W_OK, X_OK, mode_t}; -use nix::unistd::{Gid, Uid}; +use libc::{PATH_MAX, S_ISGID, S_ISUID, mode_t}; +use nix::unistd::{AccessFlags, Gid, Uid}; macro_rules! path_error { ( @@ -784,7 +784,7 @@ fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) } if let Some(perm) = opts.perms { - let mut amode = 0; + let mut amode = AccessFlags::empty(); // TODO: Update bitflags so this works /* for f in perm { @@ -797,21 +797,19 @@ fn filter_path(opts: &Options, path: &wstr, uid: Option, gid: Option) } */ if perm.contains(PermFlags::READ) { - amode |= R_OK; + amode.insert(AccessFlags::R_OK); } if perm.contains(PermFlags::WRITE) { - amode |= W_OK; + amode.insert(AccessFlags::W_OK); } if perm.contains(PermFlags::EXEC) { - amode |= X_OK; + amode.insert(AccessFlags::X_OK); } - // access returns 0 on success, - // -1 on failure. Yes, C can't even keep its bools straight. // Skip this if we don't have a mode to check - the stat can do existence too. // It's tempting to check metadata here if we have it, // e.g. see if any read-bit is set for READ. // That won't work for root. - if amode != 0 && waccess(path, amode) != 0 { + if !amode.is_empty() && waccess(path, amode).is_err() { return false; } @@ -893,7 +891,7 @@ fn path_filter_maybe_is( }) { // If we don't have filters, check if it exists. if opts.perms.is_none() && opts.types.is_none() { - let ok = waccess(arg, F_OK) == 0; + let ok = waccess(arg, AccessFlags::F_OK).is_ok(); if ok == opts.invert { // For --all, fail early if any path does not match the filter. if opts.all { diff --git a/src/builtins/status.rs b/src/builtins/status.rs index 61ef1c6d1..8a2b34c3d 100644 --- a/src/builtins/status.rs +++ b/src/builtins/status.rs @@ -9,7 +9,7 @@ use crate::tty_handoff::{TERMINAL_OS_NAME, get_scroll_content_up_capability, xtversion}; use crate::wutil::{Error, waccess, wbasename, wdirname, wrealpath}; use cfg_if::cfg_if; -use libc::F_OK; +use nix::unistd::AccessFlags; use rust_embed::RustEmbed; /// Create an enum with name `$name`. @@ -737,7 +737,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B Absolute(path) => { let path = osstr2wcstring(path); Cow::Owned(match wrealpath(&path) { - Some(p) if waccess(&p, F_OK) == 0 => p, + Some(p) if waccess(&p, AccessFlags::F_OK).is_ok() => p, // realpath did not work, just append the path // - maybe this was obtained via $PATH? _ => path, diff --git a/src/builtins/test.rs b/src/builtins/test.rs index 0e04b3264..df8379984 100644 --- a/src/builtins/test.rs +++ b/src/builtins/test.rs @@ -3,7 +3,7 @@ use crate::should_flog; mod test_expressions { - use nix::unistd::{Gid, Uid}; + use nix::unistd::{AccessFlags, Gid, Uid}; use super::*; @@ -967,13 +967,13 @@ fn unary_primary_evaluate( UnaryToken::FilePerm(permission) => { let mode = match permission { // "-r", read permission - FilePermission::r => libc::R_OK, + FilePermission::r => AccessFlags::R_OK, // "-w", whether file write permission is allowed - FilePermission::w => libc::W_OK, + FilePermission::w => AccessFlags::W_OK, // "-x", whether file execute/search is allowed - FilePermission::x => libc::X_OK, + FilePermission::x => AccessFlags::X_OK, }; - waccess(arg, mode) == 0 + waccess(arg, mode).is_ok() } UnaryToken::String(predicate) => match predicate { // "-n", non-empty string diff --git a/src/highlight/file_tester.rs b/src/highlight/file_tester.rs index cce1b87d2..8e904f5d9 100644 --- a/src/highlight/file_tester.rs +++ b/src/highlight/file_tester.rs @@ -21,6 +21,7 @@ }; use fish_widestring::{L, WExt, WString, wstr}; use libc::PATH_MAX; +use nix::unistd::AccessFlags; use std::collections::{HashMap, HashSet}; use std::os::fd::RawFd; @@ -159,7 +160,7 @@ pub fn test_redirection_target(&self, target: &wstr, mode: RedirectionMode) -> F // Input redirections must have a readable non-directory. // Note we color "try_input" files as errors if they are invalid, // even though it's possible to execute these (replaced via /dev/null). - if waccess(&target_path, libc::R_OK) == 0 + if waccess(&target_path, AccessFlags::R_OK).is_ok() && wstat(&target_path).is_ok_and(|md| !md.file_type().is_dir()) { Ok(IsFile(true)) @@ -182,8 +183,8 @@ pub fn test_redirection_target(&self, target: &wstr, mode: RedirectionMode) -> F // No err. We can write to it if it's not a directory and we have // permission. file_exists = true; - file_is_writable = - !md.file_type().is_dir() && waccess(&target_path, libc::W_OK) == 0; + file_is_writable = !md.file_type().is_dir() + && waccess(&target_path, AccessFlags::W_OK).is_ok(); } Err(err) => { if err.raw_os_error() == Some(libc::ENOENT) { @@ -200,7 +201,7 @@ pub fn test_redirection_target(&self, target: &wstr, mode: RedirectionMode) -> F // Now the file is considered writable if the parent directory is // writable. file_exists = false; - file_is_writable = waccess(&parent, libc::W_OK) == 0; + file_is_writable = waccess(&parent, AccessFlags::W_OK).is_ok(); } else { // Other errors we treat as not writable. This includes things like // ENOTDIR. diff --git a/src/path.rs b/src/path.rs index 016bff97f..f74d20d75 100644 --- a/src/path.rs +++ b/src/path.rs @@ -9,7 +9,8 @@ use crate::prelude::*; use crate::wutil::{normalize_path, path_normalize_for_cd, waccess, wdirname, wstat}; use errno::{Errno, errno, set_errno}; -use libc::{EACCES, ENOENT, ENOTDIR, F_OK, X_OK}; +use libc::{EACCES, ENOENT, ENOTDIR, X_OK}; +use nix::unistd::AccessFlags; use std::ffi::OsStr; use std::io::ErrorKind; use std::mem::MaybeUninit; @@ -195,7 +196,7 @@ pub fn path_try_get_path(cmd: &wstr, vars: &dyn Environment) -> GetPathResult { } fn path_check_executable(path: &wstr) -> Result<(), std::io::Error> { - if waccess(path, X_OK) != 0 { + if waccess(path, AccessFlags::X_OK).is_err() { return Err(std::io::Error::last_os_error()); } @@ -290,7 +291,7 @@ fn path_get_path_core>(cmd: &wstr, pathsv: &[S]) -> GetPathResult // Keep the first *interesting* error and path around. // ENOENT isn't interesting because not having a file is the normal case. // Ignore if the parent directory is already inaccessible. - if waccess(wdirname(&proposed_path), X_OK) == 0 { + if waccess(wdirname(&proposed_path), AccessFlags::X_OK).is_ok() { best = GetPathResult::new(Some(err), proposed_path); } } @@ -478,10 +479,10 @@ pub fn path_is_valid(path: &wstr, working_directory: &wstr) -> bool { // Prepend the working directory. Note that we know path is not empty here. let mut tmp = working_directory.to_owned(); tmp.push_utfstr(path); - waccess(&tmp, F_OK) == 0 + waccess(&tmp, AccessFlags::F_OK).is_ok() } else { // Simple check. - waccess(path, F_OK) == 0 + waccess(path, AccessFlags::F_OK).is_ok() } } diff --git a/src/wildcard.rs b/src/wildcard.rs index c156dcc63..865c70d38 100644 --- a/src/wildcard.rs +++ b/src/wildcard.rs @@ -2,7 +2,7 @@ use fish_common::WILDCARD_RESERVED_BASE; use fish_widestring::char_offset; -use libc::X_OK; +use nix::unistd::AccessFlags; use std::cell::LazyCell; use std::cmp::Ordering; use std::collections::HashSet; @@ -305,8 +305,9 @@ fn file_get_desc( is_link: bool, definitely_executable: bool, ) -> &'static wstr { - let is_executable = - |filename: &wstr| -> bool { definitely_executable || waccess(filename, X_OK) == 0 }; + let is_executable = |filename: &wstr| -> bool { + definitely_executable || waccess(filename, AccessFlags::X_OK).is_ok() + }; if is_link { if is_dir { @@ -375,7 +376,8 @@ fn wildcard_test_flags_then_complete( // regular file *excludes* broken links - we have no use for them as commands. let is_regular_file = entry.check_type().is_some_and(|x| x == DirEntryType::Reg); - let is_executable = LazyCell::new(|| is_regular_file && waccess(filepath, X_OK) == 0); + let is_executable = + LazyCell::new(|| is_regular_file && waccess(filepath, AccessFlags::X_OK).is_ok()); if executables_only && !*is_executable { return false; } @@ -448,7 +450,6 @@ fn wildcard_test_flags_then_complete( } mod expander { - use libc::F_OK; use crate::{ path::append_path_component, @@ -577,7 +578,7 @@ pub fn expand( if allow_fuzzy && self.resolved_completions.len() == before - && waccess(&intermediate_dirpath, F_OK) != 0 + && waccess(&intermediate_dirpath, AccessFlags::F_OK).is_err() { assert!(self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); if let Ok(mut base_dir_iter) = self.open_dir(base_dir, false) { @@ -685,7 +686,7 @@ fn expand_trailing_slash(&mut self, base_dir: &wstr, prefix: &wstr, info: Parent if !self.flags.contains(ExpandFlags::FOR_COMPLETIONS) { // Trailing slash and not accepting incomplete, e.g. `echo /xyz/`. Insert this file after checking it exists. - if waccess(base_dir, F_OK) == 0 { + if waccess(base_dir, AccessFlags::F_OK).is_ok() { self.add_expansion_result(base_dir.to_owned()); } return; diff --git a/src/wutil/mod.rs b/src/wutil/mod.rs index 955772d06..ac399b8e2 100644 --- a/src/wutil/mod.rs +++ b/src/wutil/mod.rs @@ -15,6 +15,7 @@ use errno::errno; use fish_wcstringutil::{join_strings, str2bytes_callback}; use fish_widestring::{IntoCharIter, L, WExt, WString, wstr}; +use nix::unistd::AccessFlags; use std::ffi::{CStr, OsStr}; use std::fs::{self, canonicalize}; use std::io::{self, Write}; @@ -53,9 +54,9 @@ pub fn fstat(fd: impl AsRawFd) -> io::Result { } /// Wide character version of access(). -pub fn waccess(file_name: &wstr, mode: libc::c_int) -> libc::c_int { - let tmp = wcs2zstring(file_name); - unsafe { libc::access(tmp.as_ptr(), mode) } +pub fn waccess(file_name: &wstr, amode: AccessFlags) -> nix::Result<()> { + let tmp = wcs2osstring(file_name); + nix::unistd::access(tmp.as_os_str(), amode) } /// Wide character version of unlink().