From a93c24b0840af13d2fbac25ceab0ee43835105e4 Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Sun, 25 Jan 2026 17:19:38 +0100 Subject: [PATCH] refactor: improve widestring conversion utils - Don't use `WString::from_str` for `str`s which are available at compile-time. Use `L!(input).to_owned()` instead. The reason for this is that `WString::from_str` can cause problems if the input contains PUA bytes which we use for our custom encoding scheme. In such cases, `bytes2wcstring` should be used, to avoid problems when decoding the `WString`. Removing harmless usages of `WString::from_str` allows us to focus on the potentially dangerous ones which don't convert `str`'s that are compiled into the binary. - Make `cstr2wcstring` actually take `CStr` as its input. The former version was only used in one place, and the conversion to `CStr` should happen there, where it can be checked that the conversion makes sense and is safe. The new version is used in `src/env/environmant.rs`, to avoid `to_bytes()` calls cluttering the code there. - Add `osstr2wcstring` function. This function also works for `Path`. Now, these types can be converted to widestrings with much less syntactic clutter. --- crates/widestring/src/lib.rs | 4 +-- src/ast.rs | 10 +++---- src/bin/fish.rs | 15 ++++------- src/builtins/abbr.rs | 2 +- src/builtins/fish_indent.rs | 6 ++--- src/builtins/fish_key_reader.rs | 8 +++--- src/builtins/math.rs | 2 +- src/builtins/status.rs | 28 ++++++++++---------- src/common.rs | 21 ++++++++++----- src/complete.rs | 24 ++++++++--------- src/env/environment.rs | 47 ++++++++++++--------------------- src/env_universal_common.rs | 5 ++-- src/expand.rs | 2 +- src/fs.rs | 4 +-- src/history/history.rs | 15 ++++++----- src/input.rs | 4 +-- src/io.rs | 4 +-- src/kill.rs | 8 +++--- src/path.rs | 4 +-- src/wutil/mod.rs | 4 +-- 20 files changed, 102 insertions(+), 115 deletions(-) diff --git a/crates/widestring/src/lib.rs b/crates/widestring/src/lib.rs index 8f010c13a..d76cf6464 100644 --- a/crates/widestring/src/lib.rs +++ b/crates/widestring/src/lib.rs @@ -453,7 +453,7 @@ fn test_prefix() { assert!(L!("abc").starts_with('a')); assert!(L!("abc").starts_with("ab")); assert!(L!("abc").starts_with(L!("ab"))); - assert!(L!("abc").starts_with(&WString::from_str("abc"))); + assert!(L!("abc").starts_with(L!("abc"))); } #[test] @@ -463,7 +463,7 @@ fn test_suffix() { assert!(L!("abc").ends_with('c')); assert!(L!("abc").ends_with("bc")); assert!(L!("abc").ends_with(L!("bc"))); - assert!(L!("abc").ends_with(&WString::from_str("abc"))); + assert!(L!("abc").ends_with(L!("abc"))); } #[test] diff --git a/src/ast.rs b/src/ast.rs index 31da44a7c..ddac44fc4 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1453,24 +1453,24 @@ pub fn dump(&self, orig: &wstr) -> WString { } else if let Some(n) = node.as_token() { let desc = match n.token_type() { ParseTokenType::String => { - let mut desc = WString::from_str("string"); + let mut desc = L!("string").to_owned(); if let Some(strsource) = n.try_source(orig) { sprintf!(=> &mut desc, ": '%s'", strsource); } desc } ParseTokenType::Redirection => { - let mut desc = WString::from_str("redirection"); + let mut desc = L!("redirection").to_owned(); if let Some(strsource) = n.try_source(orig) { sprintf!(=> &mut desc, ": '%s'", strsource); } desc } - ParseTokenType::End => WString::from_str("<;>"), + ParseTokenType::End => L!("<;>").to_owned(), ParseTokenType::Invalid => { // This may occur with errors, e.g. we expected to see a string but saw a // redirection. - WString::from_str("") + L!("").to_owned() } _ => { token_type_user_presentable_description(n.token_type(), ParseKeyword::None) @@ -2869,7 +2869,7 @@ fn test_ast_parse() { #[test] fn test_is_same_node() { // is_same_node is pretty subtle! Let's check it. - let src = WString::from_str(FISH_FUNC); + let src = L!(FISH_FUNC).to_owned(); let ast = ast::parse(&src, Default::default(), None); assert!(!ast.errored()); let all_nodes: Vec<&dyn Node> = ast.walk().collect(); diff --git a/src/bin/fish.rs b/src/bin/fish.rs index eaa186301..a3fafb778 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -27,7 +27,7 @@ }, }, common::{ - PACKAGE_NAME, PROFILING_ACTIVE, PROGRAM_NAME, bytes2wcstring, escape, + PACKAGE_NAME, PROFILING_ACTIVE, PROGRAM_NAME, bytes2wcstring, escape, osstr2wcstring, save_term_foreground_process_group, wcs2bytes, }, env::{ @@ -174,10 +174,7 @@ fn read_init(parser: &Parser, paths: &ConfigPaths) { eprintf!("%s", msg); } - source_config_in_directory( - parser, - &bytes2wcstring(paths.sysconf.as_os_str().as_bytes()), - ); + source_config_in_directory(parser, &osstr2wcstring(&paths.sysconf)); // We need to get the configuration directory before we can source the user configuration file. // If path_get_config returns false then we have no configuration directory and no custom config @@ -190,7 +187,7 @@ fn read_init(parser: &Parser, paths: &ConfigPaths) { fn run_command_list(parser: &Parser, cmds: &[OsString]) -> Result<(), libc::c_int> { let mut retval = Ok(()); for cmd in cmds { - let cmd_wcs = bytes2wcstring(cmd.as_bytes()); + let cmd_wcs = osstr2wcstring(cmd); let mut errors = ParseErrorList::new(); let ast = ast::parse(&cmd_wcs, ParseTreeFlags::default(), Some(&mut errors)); @@ -391,13 +388,11 @@ fn throwing_main() -> i32 { // Enable debug categories set in FISH_DEBUG. // This is in *addition* to the ones given via --debug. if let Some(debug_categories) = env::var_os("FISH_DEBUG") { - let s = bytes2wcstring(debug_categories.as_bytes()); + let s = osstr2wcstring(debug_categories); activate_flog_categories_by_pattern(&s); } - let mut args: Vec = env::args_os() - .map(|osstr| bytes2wcstring(osstr.as_bytes())) - .collect(); + let mut args: Vec = env::args_os().map(osstr2wcstring).collect(); let mut opts = FishCmdOpts::default(); let mut my_optind = match fish_parse_opt(&mut args, &mut opts) { ControlFlow::Continue(optind) => optind, diff --git a/src/builtins/abbr.rs b/src/builtins/abbr.rs index 0e117237d..e1528352c 100644 --- a/src/builtins/abbr.rs +++ b/src/builtins/abbr.rs @@ -471,7 +471,7 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> BuiltinResult { if opts.commands.is_empty() { let esc_src = escape(arg); if !esc_src.is_empty() { - let var_name = WString::from_str("_fish_abbr_") + esc_src.as_utfstr(); + let var_name = L!("_fish_abbr_").to_owned() + esc_src.as_utfstr(); let ret = parser.remove_var(&var_name, ParserEnvSetMode::new(EnvMode::UNIVERSAL)); diff --git a/src/builtins/fish_indent.rs b/src/builtins/fish_indent.rs index 05d670b5b..91c6def93 100644 --- a/src/builtins/fish_indent.rs +++ b/src/builtins/fish_indent.rs @@ -12,7 +12,7 @@ use crate::ast::{self, AsNode, Ast, Kind, Leaf, Node, NodeVisitor, SourceRangeList, Traversal}; use crate::common::{ PROGRAM_NAME, ReadExt, UnescapeFlags, UnescapeStringStyle, bytes2wcstring, get_program_name, - unescape_string, wcs2bytes, + osstr2wcstring, unescape_string, wcs2bytes, }; use crate::env::EnvStack; use crate::env::env_init; @@ -946,9 +946,7 @@ fn throwing_main() -> i32 { } } - let args: Vec = std::env::args_os() - .map(|osstr| bytes2wcstring(osstr.as_bytes())) - .collect(); + let args: Vec = std::env::args_os().map(osstr2wcstring).collect(); do_indent(None, &mut streams, args).builtin_status_code() } diff --git a/src/builtins/fish_key_reader.rs b/src/builtins/fish_key_reader.rs index 71638b0cd..51830e317 100644 --- a/src/builtins/fish_key_reader.rs +++ b/src/builtins/fish_key_reader.rs @@ -7,13 +7,13 @@ //! //! Type "exit" or "quit" to terminate the program. -use std::{ops::ControlFlow, os::unix::prelude::OsStrExt}; +use std::ops::ControlFlow; use libc::{STDIN_FILENO, VEOF, VINTR}; use crate::{ builtins::shared::BUILTIN_ERR_UNKNOWN, - common::{PROGRAM_NAME, bytes2wcstring, get_program_name, shell_modes}, + common::{PROGRAM_NAME, get_program_name, osstr2wcstring, shell_modes}, env::{EnvStack, Environment, env_init}, future_feature_flags, input_common::{ @@ -307,9 +307,7 @@ fn throwing_main() -> i32 { let mut continuous_mode = false; let mut verbose = false; - let args: Vec = std::env::args_os() - .map(|osstr| bytes2wcstring(osstr.as_bytes())) - .collect(); + let args: Vec = std::env::args_os().map(osstr2wcstring).collect(); if let ControlFlow::Break(s) = parse_flags(None, &mut streams, args, &mut continuous_mode, &mut verbose) { diff --git a/src/builtins/math.rs b/src/builtins/math.rs index c46f93919..508ba129d 100644 --- a/src/builtins/math.rs +++ b/src/builtins/math.rs @@ -164,7 +164,7 @@ fn format_double(mut v: f64, opts: &Options) -> WString { v = v.trunc(); if v == 0.0 { // not 00 - return WString::from_str("0"); + return L!("0").to_owned(); } let mneg = if v.is_sign_negative() { "-" } else { "" }; return sprintf!("%s0%o", mneg, v.abs() as u64); diff --git a/src/builtins/status.rs b/src/builtins/status.rs index d829305d1..61ef1c6d1 100644 --- a/src/builtins/status.rs +++ b/src/builtins/status.rs @@ -1,7 +1,5 @@ -use std::os::unix::prelude::*; - use super::prelude::*; -use crate::common::{bytes2wcstring, get_program_name}; +use crate::common::{bytes2wcstring, get_program_name, osstr2wcstring, str2wcstring}; use crate::env::config_paths::get_fish_path; use crate::future_feature_flags::{self as features, feature_test}; use crate::proc::{ @@ -529,7 +527,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B .chain(CMakeBinaryDir::iter()) { if arg.is_empty() || path.starts_with(arg) { - paths.push(bytes2wcstring(path.as_bytes())); + paths.push(str2wcstring(&path)); } } }; @@ -592,13 +590,17 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B } match s { STATUS_BUILD_INFO => { - let version = bytes2wcstring(crate::BUILD_VERSION.as_bytes()); - let target = bytes2wcstring(env!("BUILD_TARGET_TRIPLE").as_bytes()); - let host = bytes2wcstring(env!("BUILD_HOST_TRIPLE").as_bytes()); - let profile = bytes2wcstring(env!("BUILD_PROFILE").as_bytes()); + let version = str2wcstring(crate::BUILD_VERSION); + let target = str2wcstring(env!("BUILD_TARGET_TRIPLE")); + let host = str2wcstring(env!("BUILD_HOST_TRIPLE")); + let profile = str2wcstring(env!("BUILD_PROFILE")); streams.out.append(L!("Build system: ")); - let buildsystem = if cfg!(using_cmake) { "CMake" } else { "Cargo" }; - streams.out.appendln(bytes2wcstring(buildsystem.as_bytes())); + let buildsystem = if cfg!(using_cmake) { + L!("CMake") + } else { + L!("Cargo") + }; + streams.out.appendln(buildsystem); streams.out.append(L!("Version: ")); streams.out.appendln(version); if target == host { @@ -621,9 +623,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B #[cfg(target_feature = "crt-static")] "crt-static", ]; - streams - .out - .appendln(bytes2wcstring(features.join(" ").as_bytes())); + streams.out.appendln(str2wcstring(features.join(" "))); streams.out.appendln(""); return Ok(SUCCESS); } @@ -735,7 +735,7 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B use crate::env::config_paths::FishPath::*; let result = match get_fish_path() { Absolute(path) => { - let path = bytes2wcstring(path.as_os_str().as_bytes()); + let path = osstr2wcstring(path); Cow::Owned(match wrealpath(&path) { Some(p) if waccess(&p, F_OK) == 0 => p, // realpath did not work, just append the path diff --git a/src/common.rs b/src/common.rs index 8d95e1566..602714a83 100644 --- a/src/common.rs +++ b/src/common.rs @@ -20,7 +20,7 @@ ENCODE_DIRECT_END, decode_byte_from_char, encode_byte_to_char, subslice_position, }; use std::env; -use std::ffi::{CStr, CString, OsString}; +use std::ffi::{CStr, CString, OsStr, OsString}; use std::os::unix::prelude::*; use std::sync::atomic::Ordering; use std::sync::{Arc, MutexGuard, OnceLock}; @@ -978,9 +978,19 @@ fn append_escaped_str(output: &mut WString, input: &str) { result } -pub fn cstr2wcstring(input: &[u8]) -> WString { - let input = CStr::from_bytes_until_nul(input).unwrap().to_bytes(); - bytes2wcstring(input) +/// Use this rather than [`WString::from_str`] when the input could contain PUA bytes we use to +/// encode non-UTF-8 bytes. Otherwise, when decoding the resulting [`WString`], the PUA bytes in +/// the input would be converted to non-UTF-8 bytes. +pub fn str2wcstring>(input: S) -> WString { + bytes2wcstring(input.as_ref().as_bytes()) +} + +pub fn cstr2wcstring>(input: C) -> WString { + bytes2wcstring(input.as_ref().to_bytes()) +} + +pub fn osstr2wcstring>(input: O) -> WString { + bytes2wcstring(input.as_ref().as_bytes()) } pub(crate) fn charptr2wcstring(input: *const libc::c_char) -> WString { @@ -1360,12 +1370,11 @@ fn to_cstring(self) -> CString { #[macro_export] macro_rules! env_stack_set_from_env { ($vars:ident, $var_name:literal) => {{ - use std::os::unix::ffi::OsStrExt; if let Some(var) = std::env::var_os($var_name) { $vars.set_one( L!($var_name), $crate::env::EnvSetMode::new_at_early_startup($crate::env::EnvMode::GLOBAL), - $crate::common::bytes2wcstring(var.as_bytes()), + $crate::common::osstr2wcstring(var), ); } }}; diff --git a/src/complete.rs b/src/complete.rs index 99cd27a5c..3d443dea3 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -2658,18 +2658,18 @@ fn test_complete() { let vars = PwdEnvironment { parent: TestEnvironment { vars: HashMap::from([ - (WString::from_str("Foo1"), WString::new()), - (WString::from_str("Foo2"), WString::new()), - (WString::from_str("Foo3"), WString::new()), - (WString::from_str("Bar1"), WString::new()), - (WString::from_str("Bar2"), WString::new()), - (WString::from_str("Bar3"), WString::new()), - (WString::from_str("alpha"), WString::new()), - (WString::from_str("ALPHA!"), WString::new()), - (WString::from_str("gamma1"), WString::new()), - (WString::from_str("GAMMA2"), WString::new()), - (WString::from_str("SOMEDIR"), L!("/").to_owned()), - (WString::from_str("SOMEVAR"), WString::new()), + (L!("Foo1").to_owned(), WString::new()), + (L!("Foo2").to_owned(), WString::new()), + (L!("Foo3").to_owned(), WString::new()), + (L!("Bar1").to_owned(), WString::new()), + (L!("Bar2").to_owned(), WString::new()), + (L!("Bar3").to_owned(), WString::new()), + (L!("alpha").to_owned(), WString::new()), + (L!("ALPHA!").to_owned(), WString::new()), + (L!("gamma1").to_owned(), WString::new()), + (L!("GAMMA2").to_owned(), WString::new()), + (L!("SOMEDIR").to_owned(), L!("/").to_owned()), + (L!("SOMEVAR").to_owned(), WString::new()), ]), }, }; diff --git a/src/env/environment.rs b/src/env/environment.rs index 71b74e7d6..920447aad 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -5,7 +5,9 @@ }; use crate::abbrs::{Abbreviation, Position, abbrs_get_set}; use crate::builtins::shared::{BuiltinResult, SUCCESS}; -use crate::common::{UnescapeStringStyle, bytes2wcstring, unescape_string, wcs2zstring}; +use crate::common::{ + UnescapeStringStyle, cstr2wcstring, osstr2wcstring, str2wcstring, unescape_string, wcs2zstring, +}; use crate::env::config_paths::ConfigPaths; use crate::env::{EnvMode, EnvSetMode, EnvVar, Statuses}; use crate::env_dispatch::{VarChangeMilieu, env_dispatch_init, env_dispatch_var_change}; @@ -31,7 +33,6 @@ use std::collections::HashMap; use std::ffi::CStr; use std::mem::MaybeUninit; -use std::os::unix::prelude::*; use std::path::PathBuf; use std::sync::{Arc, LazyLock, OnceLock}; @@ -468,7 +469,7 @@ fn get_hostname_identifier() -> Option { let mut b = [0 as libc::c_char; HOSTNAME_LEN + 1]; if unsafe { libc::gethostname(b.as_mut_ptr(), b.len()) } == 0 { let cstr = unsafe { CStr::from_ptr(b.as_ptr()) }; - let res = bytes2wcstring(cstr.to_bytes()); + let res = cstr2wcstring(cstr); if res.is_empty() { None } else { Some(res) } } else { @@ -538,11 +539,7 @@ fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { if vars.get_unless_empty(L!("HOME")).is_none() { if !userinfo.pw_dir.is_null() { let s = unsafe { CStr::from_ptr(userinfo.pw_dir) }; - vars.set_one( - L!("HOME"), - global_exported_mode, - bytes2wcstring(s.to_bytes()), - ); + vars.set_one(L!("HOME"), global_exported_mode, cstr2wcstring(s)); } else { vars.set_empty(L!("HOME"), global_exported_mode); } @@ -566,18 +563,14 @@ fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { if retval == 0 && !result.is_null() { let userinfo = unsafe { userinfo.assume_init() }; let s = unsafe { CStr::from_ptr(userinfo.pw_name) }; - let uname = bytes2wcstring(s.to_bytes()); + let uname = cstr2wcstring(s); vars.set_one(L!("USER"), global_exported_mode, uname); // Only change $HOME if it's empty, so we allow e.g. `HOME=(mktemp -d)`. // This is okay with common `su` and `sudo` because they set $HOME. if vars.get_unless_empty(L!("HOME")).is_none() { if !userinfo.pw_dir.is_null() { let s = unsafe { CStr::from_ptr(userinfo.pw_dir) }; - vars.set_one( - L!("HOME"), - global_exported_mode, - bytes2wcstring(s.to_bytes()), - ); + vars.set_one(L!("HOME"), global_exported_mode, cstr2wcstring(s)); } else { // We cannot get $HOME. This triggers warnings for history and config.fish already, // so it isn't necessary to warn here as well. @@ -601,12 +594,12 @@ fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { let buf = buf; // safety: buf should contain a null-byte, and is not mutable unless we move ownership let cstr = unsafe { CStr::from_ptr(buf.as_ptr()) }; - colon_split(&[bytes2wcstring(cstr.to_bytes())]) + colon_split(&[cstr2wcstring(cstr)]) } else { vec![ - WString::from_str(env!("PREFIX")) + L!("/bin"), - WString::from_str("/usr/bin"), - WString::from_str("/bin"), + str2wcstring(env!("PREFIX")) + L!("/bin"), + L!("/usr/bin").to_owned(), + L!("/bin").to_owned(), ] }; Box::leak(paths.into_boxed_slice()) @@ -632,7 +625,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool let global_exported_mode = EnvSetMode::new_at_early_startup(EnvMode::GLOBAL | EnvMode::EXPORT); let env_iter: Vec<_> = std::env::vars_os() - .map(|(k, v)| (bytes2wcstring(k.as_bytes()), bytes2wcstring(v.as_bytes()))) + .map(|(k, v)| (osstr2wcstring(k), osstr2wcstring(v))) .collect(); let mut inherited_vars = HashMap::new(); @@ -671,7 +664,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool key, global_mode, maybe_path - .map(|path| vec![bytes2wcstring(path.as_os_str().as_bytes())]) + .map(|path| vec![osstr2wcstring(path)]) .unwrap_or_default(), ); }; @@ -720,7 +713,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool ); // Set up the version variable. - let version = bytes2wcstring(crate::BUILD_VERSION.as_bytes()); + let version = str2wcstring(crate::BUILD_VERSION); vars.set_one(L!("version"), global_mode, version.clone()); vars.set_one(L!("FISH_VERSION"), global_mode, version); @@ -737,7 +730,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool let nshlvl_str = if let Some(shlvl_var) = std::env::var_os("SHLVL") { // TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a // diagnostic? - match fish_wcstol(&bytes2wcstring(shlvl_var.as_os_str().as_bytes())) { + match fish_wcstol(&osstr2wcstring(shlvl_var)) { Ok(shlvl_i) if shlvl_i >= 0 => (shlvl_i + 1).to_wstring(), _ => L!("1").to_owned(), } @@ -748,11 +741,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool } else { // If we're not interactive, simply pass the value along. if let Some(shlvl_var) = std::env::var_os("SHLVL") { - vars.set_one( - L!("SHLVL"), - global_exported_mode, - bytes2wcstring(shlvl_var.as_os_str().as_bytes()), - ); + vars.set_one(L!("SHLVL"), global_exported_mode, osstr2wcstring(shlvl_var)); } } @@ -764,9 +753,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // Also reject all paths that don't start with "/", this includes windows paths like "F:\foo". // (see #7636) let incoming_pwd_cstr = std::env::var_os("PWD"); - let incoming_pwd = incoming_pwd_cstr - .map(|s| bytes2wcstring(s.as_os_str().as_bytes())) - .unwrap_or_default(); + let incoming_pwd = incoming_pwd_cstr.map(osstr2wcstring).unwrap_or_default(); if !incoming_pwd.is_empty() && incoming_pwd.char_at(0) == '/' && paths_are_same_file(&incoming_pwd, L!(".")) diff --git a/src/env_universal_common.rs b/src/env_universal_common.rs index dd96f622c..3f4eb1ea5 100644 --- a/src/env_universal_common.rs +++ b/src/env_universal_common.rs @@ -810,8 +810,7 @@ mod tests { use fish_tempfile::TempDir; use fish_widestring::{ENCODE_DIRECT_BASE, char_offset}; - use crate::common::bytes2wcstring; - use crate::common::wcs2osstring; + use crate::common::{osstr2wcstring, wcs2osstring}; use crate::env::{EnvVar, EnvVarFlags, VarTable}; use crate::env_universal_common::{EnvUniversal, UvarFormat}; use crate::prelude::*; @@ -826,7 +825,7 @@ mod tests { fn make_test_uvar_path() -> std::io::Result<(TempDir, WString)> { let temp_dir = fish_tempfile::new_dir()?; let file_path = temp_dir.path().join("varsfile.txt"); - let file_path = bytes2wcstring(file_path.as_os_str().as_encoded_bytes()); + let file_path = osstr2wcstring(file_path); Ok((temp_dir, file_path)) } diff --git a/src/expand.rs b/src/expand.rs index 410fb77d0..4dbca809e 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -1542,7 +1542,7 @@ fn unexpand_tildes(&self, input: &wstr, completions: &mut CompletionList) { // Get the username_with_tilde (like ~bert) and expand it into a home directory. let mut tail_idx = usize::MAX; let username_with_tilde = - WString::from_str("~") + get_home_directory_name(input, &mut tail_idx); + L!("~").to_owned() + get_home_directory_name(input, &mut tail_idx); let mut home = username_with_tilde.clone(); expand_tilde(&mut home, self.ctx.vars()); diff --git a/src/fs.rs b/src/fs.rs index aeee2e819..02184fdbf 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1,5 +1,5 @@ use crate::{ - common::{bytes2wcstring, wcs2bytes, wcs2osstring}, + common::{osstr2wcstring, wcs2bytes, wcs2osstring}, fds::wopen_cloexec, flog, flogf, path::{DirRemoteness, path_remoteness}, @@ -32,7 +32,7 @@ fn create_temporary_file(original_path: &wstr) -> std::io::Result<(File, WString let (path, result) = fish_tempfile::create_file_with_retry(|| dir.join(random_filename(prefix.clone()))); match result { - Ok(file) => Ok((file, bytes2wcstring(path.as_os_str().as_encoded_bytes()))), + Ok(file) => Ok((file, osstr2wcstring(path))), Err(e) => { flog!( error, diff --git a/src/history/history.rs b/src/history/history.rs index 4ebabbf40..aaf8949cd 100644 --- a/src/history/history.rs +++ b/src/history/history.rs @@ -28,7 +28,7 @@ use std::{ borrow::Cow, collections::{BTreeMap, HashMap, HashSet}, - ffi::CString, + ffi::{CStr, CString}, fs::File, io::{BufRead, BufWriter, Read, Write}, mem::MaybeUninit, @@ -1129,7 +1129,10 @@ fn format_history_record( ) } != 0 { - result.push_utfstr(&cstr2wcstring(×tamp_str[..])); + // SAFETY: strftime terminates the string with a null byte. If there is insufficient + // space, strftime returns 0. + let timestamp_cstr = CStr::from_bytes_until_nul(×tamp_str).unwrap(); + result.push_utfstr(&cstr2wcstring(timestamp_cstr)); } } } @@ -1783,8 +1786,7 @@ mod tests { History, HistoryItem, HistorySearch, PathList, PersistenceMode, SearchDirection, SearchFlags, SearchType, VACUUM_FREQUENCY, }; - use crate::common::ESCAPE_TEST_CHAR; - use crate::common::{ScopeGuard, bytes2wcstring, wcs2bytes, wcs2osstring}; + use crate::common::{ESCAPE_TEST_CHAR, ScopeGuard, osstr2wcstring, wcs2bytes, wcs2osstring}; use crate::env::{EnvMode, EnvSetMode, EnvStack}; use crate::fs::{LockedFile, WriteMethod}; use crate::path::path_get_data; @@ -1796,7 +1798,6 @@ mod tests { use rand::rngs::ThreadRng; use std::collections::VecDeque; use std::io::BufReader; - use std::os::unix::ffi::OsStrExt; use std::sync::Arc; use std::time::UNIX_EPOCH; use std::time::{Duration, SystemTime}; @@ -1940,7 +1941,7 @@ macro_rules! test_history_matches { let mut rng = rand::rng(); for i in 1..=max { // Generate a value. - let mut value = WString::from_str("test item ") + &i.to_wstring()[..]; + let mut value = L!("test item ").to_owned() + &i.to_wstring()[..]; // Maybe add some backslashes. if i % 3 == 0 { @@ -2034,7 +2035,7 @@ fn test_history_races() { }); if LockedFile::new( crate::fs::LockingMode::Exclusive(WriteMethod::RenameIntoPlace), - &bytes2wcstring(tmp_path.as_os_str().as_bytes()), + &osstr2wcstring(&tmp_path), ) .is_err() { diff --git a/src/input.rs b/src/input.rs index 84f1221e2..b634b13d1 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1023,7 +1023,7 @@ fn test_input() { input_mappings.add1( prefix_binding, KeyNameStyle::Plain, - WString::from_str("up-line"), + L!("up-line").to_owned(), default_mode(), None, true, @@ -1031,7 +1031,7 @@ fn test_input() { input_mappings.add1( desired_binding.clone(), KeyNameStyle::Plain, - WString::from_str("down-line"), + L!("down-line").to_owned(), default_mode(), None, true, diff --git a/src/io.rs b/src/io.rs index c2282c815..1d0b665ab 100644 --- a/src/io.rs +++ b/src/io.rs @@ -1,5 +1,5 @@ use crate::builtins::shared::{STATUS_CMD_ERROR, STATUS_CMD_OK, STATUS_READ_TOO_MUCH}; -use crate::common::{bytes2wcstring, wcs2bytes}; +use crate::common::{bytes2wcstring, str2wcstring, wcs2bytes}; use crate::fd_monitor::{Callback, FdMonitor, FdMonitorItemId}; use crate::fds::{ BorrowedFdFile, PIPE_ERROR, make_autoclose_pipes, make_fd_nonblocking, wopen_cloexec, @@ -725,7 +725,7 @@ pub fn append_char(&mut self, c: char) -> bool { } pub fn append_narrow(&mut self, s: &str) -> bool { - self.append(&bytes2wcstring(s.as_bytes())) + self.append(&str2wcstring(s)) } // Append data from a narrow buffer, widening it. pub fn append_narrow_buffer(&mut self, buffer: &SeparatedBuffer) -> bool { diff --git a/src/kill.rs b/src/kill.rs index b15b1f1ce..09e26c772 100644 --- a/src/kill.rs +++ b/src/kill.rs @@ -93,9 +93,9 @@ fn test_killring() { assert!(kr.is_empty()); - kr.add(WString::from_str("a")); - kr.add(WString::from_str("b")); - kr.add(WString::from_str("c")); + kr.add(L!("a").to_owned()); + kr.add(L!("b").to_owned()); + kr.add(L!("c").to_owned()); assert_eq!(kr.entries(), [L!("c"), L!("b"), L!("a")]); @@ -105,7 +105,7 @@ fn test_killring() { assert_eq!(kr.yank_rotate(), "a"); assert_eq!(kr.entries(), [L!("a"), L!("c"), L!("b")]); - kr.add(WString::from_str("d")); + kr.add(L!("d").to_owned()); assert_eq!(kr.entries(), [L!("d"), L!("a"), L!("c"), L!("b")]); diff --git a/src/path.rs b/src/path.rs index b0e2a24cd..016bff97f 100644 --- a/src/path.rs +++ b/src/path.rs @@ -559,7 +559,7 @@ fn make_base_directory(xdg_var: &wstr, non_xdg_homepath: &wstr) -> BaseDirectory // the actual $HOME or $XDG_XXX directories. This prevents the tests from failing and/or stops // the tests polluting the user's actual $HOME if a sandbox environment has not been set up. { - use crate::common::{BUILD_DIR, bytes2wcstring}; + use crate::common::{BUILD_DIR, osstr2wcstring}; use std::path::PathBuf; let mut build_dir = PathBuf::from(BUILD_DIR); @@ -568,7 +568,7 @@ fn make_base_directory(xdg_var: &wstr, non_xdg_homepath: &wstr) -> BaseDirectory let err = std::fs::create_dir_all(&build_dir).err(); return BaseDirectory { - path: bytes2wcstring(build_dir.as_os_str().as_bytes()), + path: osstr2wcstring(build_dir), remoteness: DirRemoteness::Unknown, used_xdg: false, err, diff --git a/src/wutil/mod.rs b/src/wutil/mod.rs index ab5192675..a36d18cdf 100644 --- a/src/wutil/mod.rs +++ b/src/wutil/mod.rs @@ -8,7 +8,7 @@ pub mod wcstoi; use crate::common::{ - bytes2wcstring, fish_reserved_codepoint, wcs2bytes, wcs2osstring, wcs2zstring, + bytes2wcstring, fish_reserved_codepoint, osstr2wcstring, wcs2bytes, wcs2osstring, wcs2zstring, }; use crate::fds::BorrowedFdFile; use crate::flog; @@ -93,7 +93,7 @@ pub fn perror_io(s: &str, e: &io::Error) { /// Wide character version of getcwd(). pub fn wgetcwd() -> WString { match std::env::current_dir() { - Ok(cwd) => bytes2wcstring(cwd.into_os_string().as_bytes()), + Ok(cwd) => osstr2wcstring(cwd), Err(e) => { flog!(error, "std::env::current_dir() failed with error:", e); WString::new()