diff --git a/fish-rust/src/wildcard.rs b/fish-rust/src/wildcard.rs index faceb5d53..c60fb6f67 100644 --- a/fish-rust/src/wildcard.rs +++ b/fish-rust/src/wildcard.rs @@ -1,16 +1,13 @@ // Enumeration of all wildcard types. +use cxx::CxxWString; +use libc::X_OK; use std::cmp::Ordering; use std::collections::HashSet; -use std::io::ErrorKind; -use std::os::unix::prelude::*; -use std::{fs, io}; - -use cxx::CxxWString; -use libc::{mode_t, ELOOP, S_IXGRP, S_IXOTH, S_IXUSR, X_OK}; +use std::fs; use crate::common::{ - char_offset, format_size, is_windows_subsystem_for_linux, unescape_string, UnescapeFlags, + char_offset, is_windows_subsystem_for_linux, unescape_string, UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, }; use crate::complete::{CompleteFlags, Completion, CompletionReceiver, PROG_COMPLETE_SEP}; @@ -23,7 +20,8 @@ use crate::wchar_ffi::WCharFromFFI; use crate::wcstringutil::{ string_fuzzy_match_string, string_suffixes_string_case_insensitive, CaseFold, }; -use crate::wutil::{lwstat, waccess, wstat}; +use crate::wutil::dir_iter::DirEntryType; +use crate::wutil::{dir_iter::DirEntry, lwstat, waccess}; use once_cell::sync::Lazy; static COMPLETE_EXEC_DESC: Lazy<&wstr> = Lazy::new(|| wgettext!("command")); @@ -303,73 +301,37 @@ pub fn wildcard_complete<'f>( /// Obtain a description string for the file specified by the filename. /// +/// It assumes the file exists and won't run stat() to confirm. /// The returned value is a string constant and should not be free'd. /// /// \param filename The file for which to find a description string -/// \param lstat_res The result of calling lstat on the file -/// \param lbuf The struct buf output of calling lstat on the file -/// \param stat_res The result of calling stat on the file -/// \param buf The struct buf output of calling stat on the file -/// \param err The errno value after a failed stat call on the file. +/// \param is_dir Whether the file is a directory or not (might be behind a link) +/// \param is_link Whether it's a link (that might point to a directory) +/// \param definitely_executable Whether we know that it is executable, or don't know fn file_get_desc( filename: &wstr, - lstat: Option, - stat: Option>, + is_dir: bool, + is_link: bool, definitely_executable: bool, ) -> &'static wstr { - let Some(lstat) = lstat else { - return *COMPLETE_FILE_DESC; - }; + let is_executable = + |filename: &wstr| -> bool { definitely_executable || waccess(filename, X_OK) == 0 }; - let is_executable = |buf: &fs::Metadata, filename: &wstr| -> bool { - // Weird group permissions and other such issues make it non-trivial to find out if - // we can actually execute a file using the result from stat. It is much safer to - // use the access function, since it tells us exactly what we want to know. - // - // We skip this check in case the caller tells us the file is definitely executable. - definitely_executable - || (buf.mode() as mode_t & (S_IXUSR | S_IXGRP | S_IXOTH) != 0) - && waccess(filename, X_OK) == 0 - }; - - // stat was only queried if lstat succeeded - let stat = stat.unwrap(); - if lstat.is_symlink() { - return match stat { - Ok(stat) if stat.is_dir() => *COMPLETE_DIRECTORY_SYMLINK_DESC, - Ok(stat) if is_executable(&stat, filename) => *COMPLETE_EXEC_LINK_DESC, - Ok(_) => *COMPLETE_SYMLINK_DESC, - Err(e) if e.kind() == ErrorKind::NotFound => *COMPLETE_BROKEN_SYMLINK_DESC, - Err(e) if e.raw_os_error().unwrap() == ELOOP => *COMPLETE_LOOP_SYMLINK_DESC, - _ => { - // On unknown errors we do nothing. The file will be given the default 'File' - // description or one based on the suffix. - *COMPLETE_FILE_DESC - } - }; - } - - let Ok(stat) = stat else { - // Assuming that the metadata was zero if stat-call failed - return *COMPLETE_FILE_DESC; - }; - - let ft = stat.file_type(); - if ft.is_char_device() { - *COMPLETE_CHAR_DESC - } else if ft.is_block_device() { - *COMPLETE_BLOCK_DESC - } else if ft.is_fifo() { - *COMPLETE_FIFO_DESC - } else if ft.is_socket() { - *COMPLETE_SOCKET_DESC - } else if ft.is_dir() { + return if is_link { + if is_dir { + *COMPLETE_DIRECTORY_SYMLINK_DESC + } else if is_executable(filename) { + *COMPLETE_EXEC_LINK_DESC + } else { + *COMPLETE_SYMLINK_DESC + } + } else if is_dir { *COMPLETE_DIRECTORY_DESC - } else if is_executable(&stat, filename) { + } else if is_executable(filename) { *COMPLETE_EXEC_DESC } else { *COMPLETE_FILE_DESC - } + }; } /// Test if the given file is an executable (if executables_only) or directory (if @@ -381,7 +343,7 @@ fn wildcard_test_flags_then_complete( wc: &wstr, expand_flags: ExpandFlags, out: &mut CompletionReceiver, - known_dir: bool, + entry: &DirEntry, ) -> bool { let executables_only = expand_flags.contains(ExpandFlags::EXECUTABLES_ONLY); let need_directory = expand_flags.contains(ExpandFlags::DIRECTORIES_ONLY); @@ -389,7 +351,8 @@ fn wildcard_test_flags_then_complete( // and we don't need to do anything else, just return it. // This is a common case for cd completions, and removes the `stat` entirely in case the system // supports it. - if known_dir && !executables_only && !expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + if entry.is_dir() && !executables_only && !expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) + { return wildcard_complete( &(filename.to_owned() + L!("/")), wc, @@ -412,31 +375,7 @@ fn wildcard_test_flags_then_complete( return false; } - let lstat: Option = lwstat(filepath).ok(); - let stat: Option>; - if let Some(md) = &lstat { - if md.is_symlink() { - // In order to differentiate between e.g. broken symlinks and symlink loops, we also - // need to know the error status of wstat. - stat = Some(wstat(filepath)); - } else { - stat = Some(Ok(md.clone())); - } - } else { - stat = None; - } - - let (file_size, is_directory, is_executable) = if let Some(Ok(md)) = &stat { - (md.len(), md.is_dir(), md.is_file()) - } else { - (0, false, false) - }; - - if need_directory && !is_directory { - return false; - } - - if executables_only && (!is_executable || waccess(filepath, X_OK) != 0) { + if need_directory && !entry.is_dir() { return false; } @@ -447,20 +386,36 @@ fn wildcard_test_flags_then_complete( return false; } + // regular file *excludes* broken links - we have no use for them as commands. + let is_regular_file = entry + .check_type() + .map(|x| x == DirEntryType::reg) + .unwrap_or(false); + if executables_only && (!is_regular_file || waccess(filepath, X_OK) != 0) { + return false; + } + // Compute the description. + // This is effectively only for command completions, + // because we disable descriptions for regular file completions. let desc = if expand_flags.contains(ExpandFlags::GEN_DESCRIPTIONS) { + let is_link: bool = match entry.is_possible_link() { + Some(n) => n, + None => { + // We do not know it's a link from the d_type, + // so we will have to do an lstat(). + let lstat: Option = lwstat(filepath).ok(); + if let Some(md) = &lstat { + md.is_symlink() + } else { + // This file is no longer be usable, skip it. + return false; + } + } + }; // If we have executables_only, we already checked waccess above, // so we tell file_get_desc that this file is definitely executable so it can skip the check. - let mut desc = file_get_desc(filename, lstat, stat, executables_only).to_owned(); - - if !is_directory && !is_executable { - if !desc.is_empty() { - desc.push_utfstr(L!(", ")); - } - desc.push_utfstr(&format_size(file_size.try_into().unwrap())); - } - - Some(desc) + Some(file_get_desc(filename, entry.is_dir(), is_link, executables_only).to_owned()) } else { None }; @@ -472,7 +427,7 @@ fn wildcard_test_flags_then_complete( None => WString::new(), }; let desc_func: Option<&dyn Fn(&wstr) -> WString> = Some(&desc_func); - if is_directory { + if entry.is_dir() { return wildcard_complete( &(filename.to_owned() + L!("/")), wc, @@ -741,7 +696,7 @@ mod expander { &entry.name, L!(""), prefix, - known_dir, + entry, ); } } @@ -779,7 +734,7 @@ mod expander { // // We only do this when we are the last `*/` component, // because we're a bit inconsistent on when we will enter loops. - if is_final && !entry.is_possible_link() { + if is_final && !entry.is_possible_link().unwrap_or(true) { let full_path: WString = base_dir.to_owned() + entry.name.as_utfstr() + L!("/"); let prefix: WString = prefix.to_owned() + wc_segment + L!("/"); @@ -884,7 +839,6 @@ mod expander { wc: &wstr, prefix: &wstr, ) { - let is_dir = false; let need_dir = self.flags.contains(ExpandFlags::DIRECTORIES_ONLY); while !self.interrupted_or_overflowed() { @@ -902,7 +856,7 @@ mod expander { &entry.name, wc, prefix, - is_dir, + entry, ); } else { // Normal wildcard expansion, not for completions. @@ -997,7 +951,7 @@ mod expander { filename: &wstr, wildcard: &wstr, prefix: &wstr, - known_dir: bool, + entry: &DirEntry, ) { // This function is only for the completions case. assert!(self.flags.contains(ExpandFlags::FOR_COMPLETIONS)); @@ -1017,7 +971,7 @@ mod expander { wildcard, self.flags, self.resolved_completions, - known_dir, + entry, ) { // Hack. We added this completion result based on the last component of the wildcard. // Prepend our prefix to each wildcard that replaces its token. diff --git a/fish-rust/src/wutil/dir_iter.rs b/fish-rust/src/wutil/dir_iter.rs index 10921c5df..b6c36e5d2 100644 --- a/fish-rust/src/wutil/dir_iter.rs +++ b/fish-rust/src/wutil/dir_iter.rs @@ -46,7 +46,7 @@ pub struct DirEntry { typ: Cell>, // whether this could be a link, false if we know definitively it isn't. - possible_link: bool, + possible_link: Option, // fd of the DIR*, used for fstatat(). dirfd: Rc, @@ -75,7 +75,7 @@ impl DirEntry { } /// \return false if we know this can't be a link via d_type, true if it could be. - pub fn is_possible_link(&self) -> bool { + pub fn is_possible_link(&self) -> Option { self.possible_link } /// \return the stat buff for this entry, invoking stat() if necessary. @@ -224,7 +224,7 @@ impl DirIter { stat: Cell::new(None), typ: Cell::new(None), dirfd: dir.clone(), - possible_link: false, + possible_link: None, }; Ok(DirIter { withdot, @@ -293,7 +293,7 @@ impl DirIter { self.entry.typ.set(typ); } // This entry could be a link if it is a link or unknown. - self.entry.possible_link = typ.unwrap_or(DirEntryType::lnk) == DirEntryType::lnk; + self.entry.possible_link = typ.map(|t| t == DirEntryType::lnk); Some(Ok(&self.entry)) } diff --git a/src/wildcard.cpp b/src/wildcard.cpp index 1f9b66f00..a58d9d2ff 100644 --- a/src/wildcard.cpp +++ b/src/wildcard.cpp @@ -235,58 +235,27 @@ wildcard_result_t wildcard_complete(const wcstring &str, const wchar_t *wc, /// Obtain a description string for the file specified by the filename. /// +/// It assumes the file exists and won't run stat() to confirm. /// The returned value is a string constant and should not be free'd. /// /// \param filename The file for which to find a description string -/// \param lstat_res The result of calling lstat on the file -/// \param lbuf The struct buf output of calling lstat on the file -/// \param stat_res The result of calling stat on the file -/// \param buf The struct buf output of calling stat on the file -/// \param err The errno value after a failed stat call on the file. -static const wchar_t *file_get_desc(const wcstring &filename, int lstat_res, - const struct stat &lbuf, int stat_res, const struct stat &buf, - int err, bool definitely_executable) { - if (lstat_res) { - return COMPLETE_FILE_DESC; - } - - if (S_ISLNK(lbuf.st_mode)) { - if (!stat_res) { - if (S_ISDIR(buf.st_mode)) { - return COMPLETE_DIRECTORY_SYMLINK_DESC; - } - if (definitely_executable || (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH) && waccess(filename, X_OK) == 0)) { - // Weird group permissions and other such issues make it non-trivial to find out if - // we can actually execute a file using the result from stat. It is much safer to - // use the access function, since it tells us exactly what we want to know. - // - // We skip this check in case the caller tells us the file is definitely executable. - return COMPLETE_EXEC_LINK_DESC; - } - - return COMPLETE_SYMLINK_DESC; +/// \param is_dir Whether the file is a directory or not (might be behind a link) +/// \param is_link Whether it's a link (that might point to a directory) +/// \param definitely_executable Whether we know that it is executable, or don't know +static const wchar_t *file_get_desc(const wcstring &filename, bool is_dir, + bool is_link, bool definitely_executable) { + if (is_link) { + if (is_dir) { + return COMPLETE_DIRECTORY_SYMLINK_DESC; + } + if (definitely_executable || waccess(filename, X_OK) == 0) { + return COMPLETE_EXEC_LINK_DESC; } - if (err == ENOENT) return COMPLETE_BROKEN_SYMLINK_DESC; - if (err == ELOOP) return COMPLETE_LOOP_SYMLINK_DESC; - // On unknown errors we do nothing. The file will be given the default 'File' - // description or one based on the suffix. - } else if (S_ISCHR(buf.st_mode)) { - return COMPLETE_CHAR_DESC; - } else if (S_ISBLK(buf.st_mode)) { - return COMPLETE_BLOCK_DESC; - } else if (S_ISFIFO(buf.st_mode)) { - return COMPLETE_FIFO_DESC; - } else if (S_ISSOCK(buf.st_mode)) { - return COMPLETE_SOCKET_DESC; - } else if (S_ISDIR(buf.st_mode)) { + return COMPLETE_SYMLINK_DESC; + } else if (is_dir) { return COMPLETE_DIRECTORY_DESC; - } else if (definitely_executable || (buf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH) && waccess(filename, X_OK) == 0)) { - // Weird group permissions and other such issues make it non-trivial to find out if we can - // actually execute a file using the result from stat. It is much safer to use the access - // function, since it tells us exactly what we want to know. - // - // We skip this check in case the caller tells us the file is definitely executable. + } else if (definitely_executable || waccess(filename, X_OK) == 0) { return COMPLETE_EXEC_DESC; } @@ -298,14 +267,14 @@ static const wchar_t *file_get_desc(const wcstring &filename, int lstat_res, /// up. Note that the filename came from a readdir() call, so we know it exists. static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wcstring &filename, const wchar_t *wc, expand_flags_t expand_flags, - completion_receiver_t *out, bool known_dir) { + completion_receiver_t *out, const dir_iter_t::entry_t &entry) { const bool executables_only = expand_flags & expand_flag::executables_only; const bool need_directory = expand_flags & expand_flag::directories_only; // Fast path: If we need directories, and we already know it is one, // and we don't need to do anything else, just return it. // This is a common case for cd completions, and removes the `stat` entirely in case the system // supports it. - if (known_dir && !executables_only && !(expand_flags & expand_flag::gen_descriptions)) { + if (entry.is_dir() && !executables_only && !(expand_flags & expand_flag::gen_descriptions)) { return wildcard_complete(filename + L'/', wc, const_desc(L""), out, expand_flags, COMPLETE_NO_SPACE) == wildcard_result_t::match; } @@ -314,34 +283,7 @@ static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wc return false; } - struct stat lstat_buf = {}, stat_buf = {}; - int stat_res = -1; - int stat_errno = 0; - int lstat_res = lwstat(filepath, &lstat_buf); - if (lstat_res >= 0) { - if (S_ISLNK(lstat_buf.st_mode)) { - stat_res = wstat(filepath, &stat_buf); - - if (stat_res < 0) { - // In order to differentiate between e.g. broken symlinks and symlink loops, we also - // need to know the error status of wstat. - stat_errno = errno; - } - } else { - stat_buf = lstat_buf; - stat_res = lstat_res; - } - } - - const long long file_size = stat_res == 0 ? stat_buf.st_size : 0; - const bool is_directory = stat_res == 0 && S_ISDIR(stat_buf.st_mode); - const bool is_executable = stat_res == 0 && S_ISREG(stat_buf.st_mode); - - if (need_directory && !is_directory) { - return false; - } - - if (executables_only && (!is_executable || waccess(filepath, X_OK) != 0)) { + if (need_directory && !entry.is_dir()) { return false; } @@ -350,23 +292,44 @@ static bool wildcard_test_flags_then_complete(const wcstring &filepath, const wc return false; } + // regular file *excludes* broken links - we have no use for them as commands. + const bool is_regular_file = entry.check_type() == dir_entry_type_t::reg; + if (executables_only && (!is_regular_file || waccess(filepath, X_OK) != 0)) { + return false; + } + // Compute the description. + // This is effectively only for command completions, + // because we disable descriptions for regular file completions. wcstring desc; if (expand_flags & expand_flag::gen_descriptions) { + bool is_link = false; + + if (!entry.is_possible_link().has_value()) { + // We do not know it's a link from the d_type, + // so we will have to do an lstat(). + struct stat lstat_buf = {}; + int lstat_res = lwstat(filepath, &lstat_buf); + if (lstat_res < 0) { + // This file is no longer be usable, skip it. + return false; + } + if (S_ISLNK(lstat_buf.st_mode)) { + is_link = true; + } + } else { + is_link = entry.is_possible_link().value(); + } + // If we have executables_only, we already checked waccess above, // so we tell file_get_desc that this file is definitely executable so it can skip the check. - desc = file_get_desc(filepath, lstat_res, lstat_buf, stat_res, stat_buf, stat_errno, executables_only); - - if (!is_directory && !is_executable && file_size >= 0) { - if (!desc.empty()) desc.append(L", "); - desc.append(format_size(file_size)); - } + desc = file_get_desc(filepath, entry.is_dir(), is_link, executables_only); } // Append a / if this is a directory. Note this requirement may be the only reason we have to // call stat() in some cases. auto desc_func = const_desc(desc); - if (is_directory) { + if (entry.is_dir()) { return wildcard_complete(filename + L'/', wc, desc_func, out, expand_flags, COMPLETE_NO_SPACE) == wildcard_result_t::match; } @@ -500,7 +463,7 @@ class wildcard_expander_t { void try_add_completion_result(const wcstring &filepath, const wcstring &filename, const wcstring &wildcard, const wcstring &prefix, - bool known_dir) { + const dir_iter_t::entry_t &entry) { // This function is only for the completions case. assert(this->flags & expand_flag::for_completions); @@ -512,7 +475,7 @@ class wildcard_expander_t { size_t before = this->resolved_completions->size(); if (wildcard_test_flags_then_complete(abs_path, filename, wildcard.c_str(), this->flags, - this->resolved_completions, known_dir)) { + this->resolved_completions, entry)) { // Hack. We added this completion result based on the last component of the wildcard. // Prepend our prefix to each wildcard that replaces its token. // Note that prepend_token_prefix is a no-op unless COMPLETE_REPLACES_TOKEN is set @@ -616,7 +579,7 @@ void wildcard_expander_t::expand_trailing_slash(const wcstring &base_dir, const if (need_dir && !known_dir) continue; if (!entry->name.empty() && entry->name.at(0) != L'.') { this->try_add_completion_result(base_dir + entry->name, entry->name, L"", - prefix, known_dir); + prefix, *entry); } } } @@ -648,7 +611,7 @@ void wildcard_expander_t::expand_intermediate_segment(const wcstring &base_dir, // // We only do this when we are the last `*/` component, // because we're a bit inconsistent on when we will enter loops. - if (is_final && !entry->is_possible_link()) { + if (is_final && !entry->is_possible_link().value_or(true)) { // We made it through. // Perform normal wildcard expansion on this new directory, // starting at our tail_wc @@ -734,7 +697,6 @@ void wildcard_expander_t::expand_literal_intermediate_segment_with_fuzz(const wc void wildcard_expander_t::expand_last_segment(const wcstring &base_dir, dir_iter_t &base_dir_iter, const wcstring &wc, const wcstring &prefix) { - bool is_dir = false; bool need_dir = flags & expand_flag::directories_only; const dir_iter_t::entry_t *entry{}; @@ -742,7 +704,7 @@ void wildcard_expander_t::expand_last_segment(const wcstring &base_dir, dir_iter if (need_dir && !entry->is_dir()) continue; if (flags & expand_flag::for_completions) { this->try_add_completion_result(base_dir + entry->name, entry->name, wc, prefix, - is_dir); + *entry); } else { // Normal wildcard expansion, not for completions. if (wildcard_match(entry->name, wc, true /* skip files with leading dots */)) { diff --git a/src/wutil.cpp b/src/wutil.cpp index 47c19c0f9..391f3c65c 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -231,9 +231,15 @@ const dir_iter_t::entry_t *dir_iter_t::next() { // Do not store symlinks as type as we will need to resolve them. if (type != dir_entry_type_t::lnk) { entry_.type_ = type; + } else { + entry_.type_ = none(); } // This entry could be a link if it is a link or unknown. - entry_.possible_link_ = !type.has_value() || type == dir_entry_type_t::lnk; + if (type.has_value()) { + entry_.possible_link_ = type == dir_entry_type_t::lnk; + } else { + entry_.possible_link_ = none(); + } #endif return &entry_; } diff --git a/src/wutil.h b/src/wutil.h index 26c471080..121d8f1f1 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -246,7 +246,7 @@ class dir_iter_t : noncopyable_t { bool is_dir() const { return check_type() == dir_entry_type_t::dir; } /// \return false if we know this can't be a link via d_type, true if it could be. - bool is_possible_link() const { return possible_link_; } + maybe_t is_possible_link() const { return possible_link_; } /// \return the stat buff for this entry, invoking stat() if necessary. const maybe_t &stat() const; @@ -267,7 +267,7 @@ class dir_iter_t : noncopyable_t { mutable maybe_t type_{}; /// whether this entry could be a link, false if we know definitively it isn't. - bool possible_link_ = true; + mutable maybe_t possible_link_{}; // fd of the DIR*, used for fstatat(). int dirfd_{-1};