Do not use .display()/.to_string_lossy() unnecessarily

This comes up a lot for quoted strings in messages: OS strings can be
quoted directly and this prevents information loss.

This commit removes ~60% of the calls to these methods (modulo
tests). Some of the remaining calls are benign, for example because
they convert solely to check for the presence of ASCII
characters. Others are nontrivial to improve.
This commit is contained in:
Jan Verbeek
2025-11-19 20:48:24 +01:00
parent c804e517c6
commit e4024045a0
74 changed files with 256 additions and 272 deletions
+1 -1
View File
@@ -52,7 +52,7 @@ impl Config {
if let Some(extra_op) = values.next() {
return Err(UUsageError::new(
BASE_CMD_PARSE_ERROR,
translate!("base-common-extra-operand", "operand" => extra_op.to_string_lossy().quote()),
translate!("base-common-extra-operand", "operand" => extra_op.quote()),
));
}
+2 -2
View File
@@ -760,7 +760,7 @@ fn root_dev_ino_warn(dir_name: &Path) {
} else {
show_warning!(
"{}",
translate!("chcon-warning-dangerous-recursive-dir", "dir" => dir_name.to_string_lossy(), "option" => options::preserve_root::NO_PRESERVE_ROOT)
translate!("chcon-warning-dangerous-recursive-dir", "dir" => dir_name.quote(), "option" => options::preserve_root::NO_PRESERVE_ROOT)
);
}
}
@@ -782,7 +782,7 @@ fn cycle_warning_required(fts_options: c_int, entry: &fts::EntryRef) -> bool {
fn emit_cycle_warning(file_name: &Path) {
show_warning!(
"{}",
translate!("chcon-warning-circular-directory", "file" => file_name.to_string_lossy())
translate!("chcon-warning-circular-directory", "file" => file_name.quote())
);
}
+20 -29
View File
@@ -9,7 +9,7 @@ use clap::{Arg, ArgAction, Command};
use std::ffi::OsString;
use std::fs;
use std::os::unix::fs::{MetadataExt, PermissionsExt};
use std::path::Path;
use std::path::{Path, PathBuf};
use thiserror::Error;
use uucore::display::Quotable;
use uucore::error::{ExitCode, UError, UResult, USimpleError, UUsageError, set_exit_code};
@@ -27,17 +27,17 @@ use uucore::translate;
#[derive(Debug, Error)]
enum ChmodError {
#[error("{}", translate!("chmod-error-cannot-stat", "file" => _0.quote()))]
CannotStat(String),
CannotStat(PathBuf),
#[error("{}", translate!("chmod-error-dangling-symlink", "file" => _0.quote()))]
DanglingSymlink(String),
DanglingSymlink(PathBuf),
#[error("{}", translate!("chmod-error-no-such-file", "file" => _0.quote()))]
NoSuchFile(String),
NoSuchFile(PathBuf),
#[error("{}", translate!("chmod-error-preserve-root", "file" => _0.quote()))]
PreserveRoot(String),
#[error("{}", translate!("chmod-error-permission-denied", "file" => _0.quote()))]
PermissionDenied(String),
#[error("{}", translate!("chmod-error-new-permissions", "file" => _0.clone(), "actual" => _1.clone(), "expected" => _2.clone()))]
NewPermissions(String, String, String),
PreserveRoot(PathBuf),
#[error("{}", translate!("chmod-error-permission-denied", "file" => _0.maybe_quote()))]
PermissionDenied(PathBuf),
#[error("{}", translate!("chmod-error-new-permissions", "file" => _0.maybe_quote(), "actual" => _1.clone(), "expected" => _2.clone()))]
NewPermissions(PathBuf, String, String),
}
impl UError for ChmodError {}
@@ -123,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
Some(fref) => match fs::metadata(fref) {
Ok(meta) => Some(meta.mode() & 0o7777),
Err(_) => {
return Err(ChmodError::CannotStat(fref.to_string_lossy().to_string()).into());
return Err(ChmodError::CannotStat(fref.into()).into());
}
},
None => None,
@@ -384,22 +384,18 @@ impl Chmoder {
}
if !self.quiet {
show!(ChmodError::DanglingSymlink(
filename.to_string_lossy().to_string()
));
show!(ChmodError::DanglingSymlink(filename.into()));
set_exit_code(1);
}
if self.verbose {
println!(
"{}",
translate!("chmod-verbose-failed-dangling", "file" => filename.to_string_lossy().quote())
translate!("chmod-verbose-failed-dangling", "file" => filename.quote())
);
}
} else if !self.quiet {
show!(ChmodError::NoSuchFile(
filename.to_string_lossy().to_string()
));
show!(ChmodError::NoSuchFile(filename.into()));
}
// GNU exits with exit code 1 even if -q or --quiet are passed
// So we set the exit code, because it hasn't been set yet if `self.quiet` is true.
@@ -412,7 +408,7 @@ impl Chmoder {
continue;
}
if self.recursive && self.preserve_root && file == Path::new("/") {
return Err(ChmodError::PreserveRoot("/".to_string()).into());
return Err(ChmodError::PreserveRoot("/".into()).into());
}
if self.recursive {
r = self.walk_dir_with_context(file, true);
@@ -474,10 +470,7 @@ impl Chmoder {
Err(err) => {
// Handle permission denied errors with proper file path context
if err.kind() == std::io::ErrorKind::PermissionDenied {
r = r.and(Err(ChmodError::PermissionDenied(
file_path.to_string_lossy().to_string(),
)
.into()));
r = r.and(Err(ChmodError::PermissionDenied(file_path.into()).into()));
} else {
r = r.and(Err(err.into()));
}
@@ -504,7 +497,7 @@ impl Chmoder {
// Handle permission denied with proper file path context
let e = dir_meta.unwrap_err();
let error = if e.kind() == std::io::ErrorKind::PermissionDenied {
ChmodError::PermissionDenied(entry_path.to_string_lossy().to_string()).into()
ChmodError::PermissionDenied(entry_path).into()
} else {
e.into()
};
@@ -584,9 +577,7 @@ impl Chmoder {
new_mode
);
}
return Err(
ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(),
);
return Err(ChmodError::PermissionDenied(file_path.into()).into());
}
// Report the change using the helper method
@@ -625,9 +616,9 @@ impl Chmoder {
} else if err.kind() == std::io::ErrorKind::PermissionDenied {
// These two filenames would normally be conditionally
// quoted, but GNU's tests expect them to always be quoted
Err(ChmodError::PermissionDenied(file.to_string_lossy().to_string()).into())
Err(ChmodError::PermissionDenied(file.into()).into())
} else {
Err(ChmodError::CannotStat(file.to_string_lossy().to_string()).into())
Err(ChmodError::CannotStat(file.into()).into())
};
}
};
@@ -657,7 +648,7 @@ impl Chmoder {
// if a permission would have been removed if umask was 0, but it wasn't because umask was not 0, print an error and fail
if (new_mode & !naively_expected_new_mode) != 0 {
return Err(ChmodError::NewPermissions(
file.to_string_lossy().to_string(),
file.into(),
display_permissions_unix(new_mode as mode_t, false),
display_permissions_unix(naively_expected_new_mode as mode_t, false),
)
+3 -3
View File
@@ -182,7 +182,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
}
if !options.newroot.is_dir() {
return Err(ChrootError::NoSuchDirectory(format!("{}", options.newroot.display())).into());
return Err(ChrootError::NoSuchDirectory(options.newroot).into());
}
let commands = match matches.get_many::<String>(options::COMMAND) {
@@ -436,7 +436,7 @@ fn enter_chroot(root: &Path, skip_chdir: bool) -> UResult<()> {
let err = unsafe {
chroot(
CString::new(root.as_os_str().as_bytes().to_vec())
.map_err(|e| ChrootError::CannotEnter("root".to_string(), e.into()))?
.map_err(|e| ChrootError::CannotEnter("root".into(), e.into()))?
.as_bytes_with_nul()
.as_ptr()
.cast::<libc::c_char>(),
@@ -449,6 +449,6 @@ fn enter_chroot(root: &Path, skip_chdir: bool) -> UResult<()> {
}
Ok(())
} else {
Err(ChrootError::CannotEnter(format!("{}", root.display()), Error::last_os_error()).into())
Err(ChrootError::CannotEnter(root.into(), Error::last_os_error()).into())
}
}
+3 -2
View File
@@ -5,6 +5,7 @@
// spell-checker:ignore NEWROOT Userspec userspec
//! Errors returned by chroot.
use std::io::Error;
use std::path::PathBuf;
use thiserror::Error;
use uucore::display::Quotable;
use uucore::error::UError;
@@ -16,7 +17,7 @@ use uucore::translate;
pub enum ChrootError {
/// Failed to enter the specified directory.
#[error("{}", translate!("chroot-error-cannot-enter", "dir" => _0.quote(), "err" => _1))]
CannotEnter(String, #[source] Error),
CannotEnter(PathBuf, #[source] Error),
/// Failed to execute the specified command.
#[error("{}", translate!("chroot-error-command-failed", "cmd" => _0.quote(), "err" => _1))]
@@ -52,7 +53,7 @@ pub enum ChrootError {
/// The given directory does not exist.
#[error("{}", translate!("chroot-error-no-such-directory", "dir" => _0.quote()))]
NoSuchDirectory(String),
NoSuchDirectory(PathBuf),
/// The call to `setgid()` failed.
#[error("{}", translate!("chroot-error-set-gid-failed", "gid" => _0, "err" => _1))]
+3 -2
View File
@@ -19,6 +19,7 @@ use uucore::checksum::{
LEGACY_ALGORITHMS, SUPPORTED_ALGORITHMS, calculate_blake2b_length_str, detect_algo,
digest_reader, perform_checksum_validation, sanitize_sha2_sha3_length_str,
};
use uucore::display::Quotable;
use uucore::translate;
use uucore::{
@@ -200,7 +201,7 @@ where
if filepath.is_dir() {
show!(USimpleError::new(
1,
translate!("cksum-error-is-directory", "file" => filepath.display())
translate!("cksum-error-is-directory", "file" => filepath.maybe_quote())
));
continue;
}
@@ -213,7 +214,7 @@ where
file_buf = match File::open(filepath) {
Ok(file) => file,
Err(err) => {
show!(err.map_err_context(|| filepath.to_string_lossy().to_string()));
show!(err.map_err_context(|| filepath.maybe_quote().to_string()));
continue;
}
};
+3 -2
View File
@@ -10,6 +10,7 @@ use std::ffi::OsString;
use std::fs::{File, metadata};
use std::io::{self, BufRead, BufReader, Read, StdinLock, stdin};
use std::path::Path;
use uucore::display::Quotable;
use uucore::error::{FromIo, UResult, USimpleError};
use uucore::format_usage;
use uucore::fs::paths_refer_to_same_file;
@@ -310,9 +311,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let filename1 = matches.get_one::<OsString>(options::FILE_1).unwrap();
let filename2 = matches.get_one::<OsString>(options::FILE_2).unwrap();
let mut f1 = open_file(filename1, line_ending)
.map_err_context(|| filename1.to_string_lossy().to_string())?;
.map_err_context(|| filename1.maybe_quote().to_string())?;
let mut f2 = open_file(filename2, line_ending)
.map_err_context(|| filename2.to_string_lossy().to_string())?;
.map_err_context(|| filename2.maybe_quote().to_string())?;
// Due to default_value(), there must be at least one value here, thus unwrap() must not panic.
let all_delimiters = matches
+2 -2
View File
@@ -1741,13 +1741,13 @@ pub(crate) fn copy_attributes(
if let Some(context) = context {
if let Err(e) = context.set_for_path(dest, false, false) {
return Err(CpError::Error(
translate!("cp-error-selinux-set-context", "path" => dest.display(), "error" => e),
translate!("cp-error-selinux-set-context", "path" => dest.quote(), "error" => e),
));
}
}
} else {
return Err(CpError::Error(
translate!("cp-error-selinux-get-context", "path" => source.display()),
translate!("cp-error-selinux-get-context", "path" => source.quote()),
));
}
Ok(())
+2 -1
View File
@@ -10,6 +10,7 @@ use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use uucore::buf_copy;
use uucore::display::Quotable;
use uucore::translate;
use uucore::mode::get_umask;
@@ -86,7 +87,7 @@ pub(crate) fn copy_on_write(
// support COW).
match reflink_mode {
ReflinkMode::Always => {
return Err(translate!("cp-error-failed-to-clone", "source" => source.display(), "dest" => dest.display(), "error" => error)
return Err(translate!("cp-error-failed-to-clone", "source" => source.quote(), "dest" => dest.quote(), "error" => error)
.into());
}
_ => {
+2 -2
View File
@@ -374,7 +374,7 @@ fn cut_files(mut filenames: Vec<OsString>, mode: &Mode) {
if path.is_dir() {
show_error!(
"{}: {}",
filename.to_string_lossy().maybe_quote(),
filename.maybe_quote(),
translate!("cut-error-is-directory")
);
set_exit_code(1);
@@ -383,7 +383,7 @@ fn cut_files(mut filenames: Vec<OsString>, mode: &Mode) {
show_if_err!(
File::open(path)
.map_err_context(|| filename.to_string_lossy().to_string())
.map_err_context(|| filename.maybe_quote().to_string())
.and_then(|file| {
match &mode {
Mode::Bytes(ranges, opts) | Mode::Characters(ranges, opts) => {
+1 -1
View File
@@ -99,7 +99,7 @@ date-help-universal = print or set Coordinated Universal Time (UTC)
date-error-invalid-date = invalid date '{$date}'
date-error-invalid-format = invalid format '{$format}' ({$error})
date-error-expected-file-got-directory = expected file, got directory '{$path}'
date-error-expected-file-got-directory = expected file, got directory {$path}
date-error-date-overflow = date overflow '{$date}'
date-error-setting-date-not-supported-macos = setting the date is not supported by macOS
date-error-setting-date-not-supported-redox = setting the date is not supported by Redox
+1 -1
View File
@@ -94,7 +94,7 @@ date-help-universal = afficher ou définir le Temps Universel Coordonné (UTC)
date-error-invalid-date = date invalide '{$date}'
date-error-invalid-format = format invalide '{$format}' ({$error})
date-error-expected-file-got-directory = fichier attendu, répertoire obtenu '{$path}'
date-error-expected-file-got-directory = fichier attendu, répertoire obtenu {$path}
date-error-date-overflow = débordement de date '{$date}'
date-error-setting-date-not-supported-macos = la définition de la date n'est pas prise en charge par macOS
date-error-setting-date-not-supported-redox = la définition de la date n'est pas prise en charge par Redox
+6 -5
View File
@@ -18,6 +18,7 @@ use std::fs::File;
use std::io::{BufRead, BufReader};
use std::path::PathBuf;
use std::sync::OnceLock;
use uucore::display::Quotable;
use uucore::error::FromIo;
use uucore::error::{UResult, USimpleError};
use uucore::translate;
@@ -349,23 +350,23 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
if path.is_dir() {
return Err(USimpleError::new(
2,
translate!("date-error-expected-file-got-directory", "path" => path.to_string_lossy()),
translate!("date-error-expected-file-got-directory", "path" => path.quote()),
));
}
let file = File::open(path)
.map_err_context(|| path.as_os_str().to_string_lossy().to_string())?;
let file =
File::open(path).map_err_context(|| path.as_os_str().maybe_quote().to_string())?;
let lines = BufReader::new(file).lines();
let iter = lines.map_while(Result::ok).map(parse_date);
Box::new(iter)
}
DateSource::FileMtime(ref path) => {
let metadata = std::fs::metadata(path)
.map_err_context(|| path.as_os_str().to_string_lossy().to_string())?;
.map_err_context(|| path.as_os_str().maybe_quote().to_string())?;
let mtime = metadata.modified()?;
let ts = Timestamp::try_from(mtime).map_err(|e| {
USimpleError::new(
1,
translate!("date-error-cannot-set-date", "path" => path.to_string_lossy(), "error" => e),
translate!("date-error-cannot-set-date", "path" => path.quote(), "error" => e),
)
})?;
let date = ts.to_zoned(TimeZone::try_system().unwrap_or(TimeZone::UTC));
+1 -1
View File
@@ -370,7 +370,7 @@ where
Err(FsError::InvalidPath) => {
show!(USimpleError::new(
1,
translate!("df-error-no-such-file-or-directory", "path" => path.as_ref().display())
translate!("df-error-no-such-file-or-directory", "path" => path.as_ref().maybe_quote())
));
}
Err(FsError::MountMissing) => {
+2 -2
View File
@@ -149,7 +149,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
if !files.is_empty() {
return Err(UUsageError::new(
1,
translate!("dircolors-error-extra-operand-print-database", "operand" => files[0].to_string_lossy().quote()),
translate!("dircolors-error-extra-operand-print-database", "operand" => files[0].quote()),
));
}
@@ -198,7 +198,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
} else if files.len() > 1 {
return Err(UUsageError::new(
1,
translate!("dircolors-error-extra-operand", "operand" => files[1].to_string_lossy().quote()),
translate!("dircolors-error-extra-operand", "operand" => files[1].quote()),
));
} else if files[0] == "-" {
let fin = BufReader::new(std::io::stdin());
+1 -1
View File
@@ -59,7 +59,7 @@ du-error-invalid-glob = Invalid exclude syntax: { $error }
du-error-cannot-read-directory = cannot read directory { $path }
du-error-cannot-access = cannot access { $path }
du-error-read-error-is-directory = { $file }: read error: Is a directory
du-error-cannot-open-for-reading = cannot open '{ $file }' for reading: No such file or directory
du-error-cannot-open-for-reading = cannot open { $file } for reading: No such file or directory
du-error-invalid-zero-length-file-name = { $file }:{ $line }: invalid zero-length file name
du-error-extra-operand-with-files0-from = extra operand { $file }
file operands cannot be combined with --files0-from
+1 -1
View File
@@ -59,7 +59,7 @@ du-error-invalid-glob = Syntaxe d'exclusion invalide : { $error }
du-error-cannot-read-directory = impossible de lire le répertoire { $path }
du-error-cannot-access = impossible d'accéder à { $path }
du-error-read-error-is-directory = { $file } : erreur de lecture : C'est un répertoire
du-error-cannot-open-for-reading = impossible d'ouvrir '{ $file }' en lecture : Aucun fichier ou répertoire de ce type
du-error-cannot-open-for-reading = impossible d'ouvrir { $file } en lecture : Aucun fichier ou répertoire de ce type
du-error-invalid-zero-length-file-name = { $file }:{ $line } : nom de fichier de longueur zéro invalide
du-error-extra-operand-with-files0-from = opérande supplémentaire { $file }
les opérandes de fichier ne peuvent pas être combinées avec --files0-from
+5 -5
View File
@@ -914,7 +914,7 @@ fn read_files_from(file_name: &OsStr) -> Result<Vec<PathBuf>, std::io::Error> {
let path = PathBuf::from(file_name);
if path.is_dir() {
return Err(std::io::Error::other(
translate!("du-error-read-error-is-directory", "file" => file_name.to_string_lossy()),
translate!("du-error-read-error-is-directory", "file" => file_name.maybe_quote()),
));
}
@@ -923,7 +923,7 @@ fn read_files_from(file_name: &OsStr) -> Result<Vec<PathBuf>, std::io::Error> {
Ok(file) => Box::new(BufReader::new(file)),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
return Err(std::io::Error::other(
translate!("du-error-cannot-open-for-reading", "file" => file_name.to_string_lossy()),
translate!("du-error-cannot-open-for-reading", "file" => file_name.quote()),
));
}
Err(e) => return Err(e),
@@ -939,7 +939,7 @@ fn read_files_from(file_name: &OsStr) -> Result<Vec<PathBuf>, std::io::Error> {
let line_number = i + 1;
show_error!(
"{}",
translate!("du-error-invalid-zero-length-file-name", "file" => file_name.to_string_lossy(), "line" => line_number)
translate!("du-error-invalid-zero-length-file-name", "file" => file_name.maybe_quote(), "line" => line_number)
);
set_exit_code(1);
} else {
@@ -976,7 +976,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
"file" => matches
.get_one::<OsString>(options::FILE)
.unwrap()
.to_string_lossy()
.quote()
),
)
@@ -1169,7 +1168,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
#[cfg(target_os = "linux")]
let error_msg = translate!("du-error-cannot-access", "path" => path.quote());
#[cfg(not(target_os = "linux"))]
let error_msg = translate!("du-error-cannot-access-no-such-file", "path" => path.to_string_lossy().quote());
let error_msg =
translate!("du-error-cannot-access-no-such-file", "path" => path.quote());
print_tx
.send(Err(USimpleError::new(1, error_msg)))
+2 -2
View File
@@ -296,7 +296,7 @@ fn open(path: &OsString) -> UResult<BufReader<Box<dyn Read + 'static>>> {
Ok(BufReader::new(Box::new(stdin()) as Box<dyn Read>))
} else {
let path_ref = Path::new(path);
file_buf = File::open(path_ref).map_err_context(|| path.to_string_lossy().to_string())?;
file_buf = File::open(path_ref).map_err_context(|| path.maybe_quote().to_string())?;
Ok(BufReader::new(Box::new(file_buf) as Box<dyn Read>))
}
}
@@ -458,7 +458,7 @@ fn expand(options: &Options) -> UResult<()> {
if Path::new(file).is_dir() {
show_error!(
"{}",
translate!("expand-error-is-directory", "file" => file.to_string_lossy())
translate!("expand-error-is-directory", "file" => file.maybe_quote())
);
set_exit_code(1);
continue;
+3 -3
View File
@@ -25,7 +25,7 @@ use uucore::checksum::detect_algo;
use uucore::checksum::digest_reader;
use uucore::checksum::escape_filename;
use uucore::checksum::perform_checksum_validation;
use uucore::display::print_verbatim;
use uucore::display::{Quotable, print_verbatim};
use uucore::error::{UResult, strip_errno};
use uucore::format_usage;
use uucore::sum::{Digest, Sha3_224, Sha3_256, Sha3_384, Sha3_512, Shake128, Shake256};
@@ -546,7 +546,7 @@ where
eprintln!(
"{}: {}: {}",
options.binary_name,
filename.to_string_lossy(),
filename.maybe_quote(),
strip_errno(&e)
);
err_found = Some(ChecksumError::Io(e));
@@ -568,7 +568,7 @@ where
eprintln!(
"{}: {}: {}",
options.binary_name,
filename.to_string_lossy(),
filename.maybe_quote(),
strip_errno(&e)
);
err_found = Some(ChecksumError::Io(e));

Some files were not shown because too many files have changed in this diff Show More