From 60df3c6b7c2601ef2d1015f8ab03c0388a402816 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Wed, 1 Sep 2021 16:34:20 +0200 Subject: [PATCH 01/21] uucore: Cache args_os(), util_name(), execution_phrase() And remove args() because there's no valid use for it, invalid unicode handling is specified in a different way. --- Cargo.lock | 1 + src/uu/base32/src/base32.rs | 6 ++--- src/uu/base64/src/base64.rs | 4 +-- src/uu/basenc/src/basenc.rs | 4 +-- src/uu/du/src/du.rs | 2 +- src/uu/logname/src/logname.rs | 5 ++-- src/uucore/Cargo.toml | 1 + src/uucore/src/lib/lib.rs | 49 ++++++++++++++++++----------------- 8 files changed, 37 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 704d1eea1..808f62e15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3276,6 +3276,7 @@ dependencies = [ "getopts", "lazy_static", "libc", + "once_cell", "termion", "thiserror", "time", diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index ac9ed1075..667fd927e 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -38,7 +38,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let name = uucore::util_name(); let config_result: Result = - base_common::parse_base_cmd_args(args, &name, VERSION, ABOUT, &usage); + base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from @@ -52,12 +52,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - &name, + name, ); 0 } pub fn uu_app() -> App<'static, 'static> { - base_common::base_app(&uucore::util_name(), VERSION, ABOUT) + base_common::base_app(uucore::util_name(), VERSION, ABOUT) } diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index e303f9d29..ded157362 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -38,7 +38,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let usage = usage(); let name = uucore::util_name(); let config_result: Result = - base_common::parse_base_cmd_args(args, &name, VERSION, ABOUT, &usage); + base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); // Create a reference to stdin so we can return a locked stdin from @@ -52,7 +52,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - &name, + name, ); 0 diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index 2b3193d49..86c251ad1 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -47,7 +47,7 @@ fn usage() -> String { } pub fn uu_app() -> App<'static, 'static> { - let mut app = base_common::base_app(&uucore::util_name(), crate_version!(), ABOUT); + let mut app = base_common::base_app(uucore::util_name(), crate_version!(), ABOUT); for encoding in ENCODINGS { app = app.arg(Arg::with_name(encoding.0).long(encoding.0)); } @@ -88,7 +88,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - &name, + name, ); 0 diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 258d58bae..685064dfc 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -466,7 +466,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let options = Options { all: matches.is_present(options::ALL), - util_name: uucore::util_name(), + util_name: uucore::util_name().to_owned(), max_depth, total: matches.is_present(options::TOTAL), separate_dirs: matches.is_present(options::SEPARATE_DIRS), diff --git a/src/uu/logname/src/logname.rs b/src/uu/logname/src/logname.rs index f8dd3fc5d..56866ff62 100644 --- a/src/uu/logname/src/logname.rs +++ b/src/uu/logname/src/logname.rs @@ -35,7 +35,7 @@ fn get_userlogin() -> Option { static SUMMARY: &str = "Print user's login name"; -fn usage() -> String { +fn usage() -> &'static str { uucore::execution_phrase() } @@ -44,8 +44,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .collect_str(InvalidEncodingHandling::Ignore) .accept_any(); - let usage = usage(); - let _ = uu_app().usage(&usage[..]).get_matches_from(args); + let _ = uu_app().usage(usage()).get_matches_from(args); match get_userlogin() { Some(userlogin) => println!("{}", userlogin), diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index fd78e6551..a04f565aa 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -30,6 +30,7 @@ data-encoding = { version="2.1", optional=true } data-encoding-macro = { version="0.1.12", optional=true } z85 = { version="3.0.3", optional=true } libc = { version="0.2.15", optional=true } +once_cell = "1.8.0" [dev-dependencies] clap = "2.33.3" diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 5352a6356..129ff9106 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -70,6 +70,8 @@ pub use crate::features::wide; use std::ffi::OsString; use std::sync::atomic::Ordering; +use once_cell::sync::Lazy; + pub fn get_utility_is_second_arg() -> bool { crate::macros::UTILITY_IS_SECOND_ARG.load(Ordering::SeqCst) } @@ -78,36 +80,40 @@ pub fn set_utility_is_second_arg() { crate::macros::UTILITY_IS_SECOND_ARG.store(true, Ordering::SeqCst) } -/// Get the executable path (as `OsString`). -fn executable_os() -> OsString { - args_os().next().unwrap() -} +// args_os() can be expensive to call, it copies all of argv before iterating. +// So if we want only the first arg or so it's overkill. We cache it. +static ARGV: Lazy> = Lazy::new(|| wild::args_os().collect()); -/// Get the executable path (as `String`). -fn executable() -> String { - executable_os().to_string_lossy().into_owned() -} +static UTIL_NAME: Lazy = Lazy::new(|| { + if get_utility_is_second_arg() { + &ARGV[1] + } else { + &ARGV[0] + } + .to_string_lossy() + .into_owned() +}); /// Derive the utility name. -pub fn util_name() -> String { - if get_utility_is_second_arg() { - args_os().nth(1).unwrap().to_string_lossy().into_owned() - } else { - executable() - } +pub fn util_name() -> &'static str { + &UTIL_NAME } -/// Derive the complete execution phrase for "usage". -pub fn execution_phrase() -> String { +static EXECUTION_PHRASE: Lazy = Lazy::new(|| { if get_utility_is_second_arg() { - args_os() + ARGV.iter() .take(2) .map(|os_str| os_str.to_string_lossy().into_owned()) .collect::>() .join(" ") } else { - executable() + ARGV[0].to_string_lossy().into_owned() } +}); + +/// Derive the complete execution phrase for "usage". +pub fn execution_phrase() -> &'static str { + &EXECUTION_PHRASE } pub enum InvalidEncodingHandling { @@ -204,13 +210,8 @@ pub trait Args: Iterator + Sized { impl + Sized> Args for T {} -// args() ... -pub fn args() -> impl Iterator { - wild::args() -} - pub fn args_os() -> impl Iterator { - wild::args_os() + ARGV.iter().cloned() } #[cfg(test)] From 259f18fcabf2405067f9444fc7bff520cda2dc0c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 3 Sep 2021 16:10:39 +0200 Subject: [PATCH 02/21] Update message quoting and filename printing --- .../cspell.dictionaries/jargon.wordlist.txt | 2 + src/bin/coreutils.rs | 29 ++++++---- src/uu/base32/src/base_common.rs | 10 ++-- src/uu/cat/src/cat.rs | 3 +- src/uu/chcon/src/chcon.rs | 16 +++--- src/uu/chcon/src/errors.rs | 4 +- src/uu/chgrp/src/chgrp.rs | 10 +++- src/uu/chmod/src/chmod.rs | 42 ++++++++------ src/uu/chown/src/chown.rs | 7 ++- src/uu/chroot/src/chroot.rs | 17 ++++-- src/uu/cksum/src/cksum.rs | 5 +- src/uu/cp/src/cp.rs | 35 ++++++------ src/uu/csplit/src/csplit.rs | 3 +- src/uu/csplit/src/csplit_error.rs | 14 +++-- src/uu/cut/src/cut.rs | 7 ++- src/uu/date/src/date.rs | 7 ++- src/uu/dircolors/src/dircolors.rs | 19 +++++-- src/uu/dirname/src/dirname.rs | 3 +- src/uu/du/src/du.rs | 54 +++++++++--------- src/uu/env/src/env.rs | 10 +++- src/uu/expand/src/expand.rs | 3 +- src/uu/factor/src/cli.rs | 5 +- src/uu/fmt/src/fmt.rs | 3 +- src/uu/groups/src/groups.rs | 7 ++- src/uu/hashsum/src/hashsum.rs | 7 ++- src/uu/head/src/head.rs | 9 +-- src/uu/id/src/id.rs | 3 +- src/uu/install/src/install.rs | 43 ++++++--------- src/uu/install/src/mode.rs | 3 +- src/uu/join/src/join.rs | 19 ++++--- src/uu/kill/src/kill.rs | 7 ++- src/uu/ln/src/ln.rs | 46 ++++++++-------- src/uu/ls/src/ls.rs | 29 +++++----- src/uu/mkdir/src/mkdir.rs | 11 ++-- src/uu/mkfifo/src/mkfifo.rs | 4 +- src/uu/mknod/src/mknod.rs | 3 +- src/uu/mktemp/src/mktemp.rs | 23 ++++---- src/uu/more/src/more.rs | 5 +- src/uu/mv/src/mv.rs | 44 +++++++-------- src/uu/nohup/src/nohup.rs | 17 ++++-- src/uu/numfmt/src/format.rs | 6 +- src/uu/numfmt/src/numfmt.rs | 3 +- src/uu/od/src/multifilereader.rs | 4 +- src/uu/od/src/od.rs | 5 +- src/uu/od/src/parse_formats.rs | 17 ++++-- src/uu/pathchk/src/pathchk.rs | 21 +++---- src/uu/pr/src/pr.rs | 19 +++++-- src/uu/printf/src/cli.rs | 11 +--- src/uu/printf/src/memo.rs | 11 ++-- .../src/tokenize/num_format/formatter.rs | 5 +- .../src/tokenize/num_format/num_format.rs | 11 ++-- src/uu/printf/src/tokenize/sub.rs | 3 +- src/uu/ptx/src/ptx.rs | 7 ++- src/uu/readlink/src/readlink.rs | 20 ++----- src/uu/realpath/src/realpath.rs | 17 ++++-- src/uu/relpath/src/relpath.rs | 5 +- src/uu/rm/src/rm.rs | 50 ++++++++--------- src/uu/runcon/src/errors.rs | 4 +- src/uu/seq/src/seq.rs | 9 +-- src/uu/shred/src/shred.rs | 48 ++++++++-------- src/uu/shuf/src/shuf.rs | 17 +++--- src/uu/sort/src/sort.rs | 55 +++++++++++++------ src/uu/split/src/split.rs | 13 +++-- src/uu/stat/src/stat.rs | 17 +++--- src/uu/sum/src/sum.rs | 3 +- src/uu/sync/src/sync.rs | 7 ++- src/uu/tac/src/tac.rs | 11 ++-- src/uu/tee/src/tee.rs | 9 +-- src/uu/test/src/parser.rs | 33 ++++++----- src/uu/test/src/test.rs | 55 +++++++++---------- src/uu/timeout/src/timeout.rs | 9 +-- src/uu/touch/src/touch.rs | 21 +++++-- src/uu/tr/src/tr.rs | 6 +- src/uu/truncate/src/truncate.rs | 5 +- src/uu/tsort/src/tsort.rs | 9 ++- src/uu/unexpand/src/unexpand.rs | 5 +- src/uu/uniq/src/uniq.rs | 10 +++- src/uu/unlink/src/unlink.rs | 8 ++- src/uucore/src/lib/features/perms.rs | 33 +++++------ src/uucore/src/lib/lib.rs | 11 ++-- src/uucore/src/lib/mods/backup_control.rs | 19 +++++-- src/uucore/src/lib/mods/display.rs | 23 ++++++++ src/uucore/src/lib/mods/error.rs | 36 ++++++++---- src/uucore/src/lib/mods/ranges.rs | 4 +- src/uucore/src/lib/parser/parse_size.rs | 14 ++++- src/uucore/src/lib/parser/parse_time.rs | 6 +- tests/by-util/test_chroot.rs | 4 +- tests/by-util/test_cksum.rs | 4 +- tests/by-util/test_sort.rs | 8 +-- tests/by-util/test_sum.rs | 4 +- tests/by-util/test_test.rs | 4 +- 91 files changed, 777 insertions(+), 550 deletions(-) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index 6561dc63b..cd1cc18b3 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -9,6 +9,8 @@ bytewise canonicalization canonicalize canonicalizing +codepoint +codepoints colorizable colorize coprime diff --git a/src/bin/coreutils.rs b/src/bin/coreutils.rs index 477931dbd..1de1b6354 100644 --- a/src/bin/coreutils.rs +++ b/src/bin/coreutils.rs @@ -10,10 +10,12 @@ use clap::Arg; use clap::Shell; use std::cmp; use std::collections::hash_map::HashMap; +use std::ffi::OsStr; use std::ffi::OsString; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process; +use uucore::display::Quotable; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -76,13 +78,21 @@ fn main() { // 0th argument equals util name? if let Some(util_os) = util_name { - let util = util_os.as_os_str().to_string_lossy(); + fn not_found(util: &OsStr) -> ! { + println!("{}: function/utility not found", util.maybe_quote()); + process::exit(1); + } + + let util = match util_os.to_str() { + Some(util) => util, + None => not_found(&util_os), + }; if util == "completion" { gen_completions(args, utils); } - match utils.get(&util[..]) { + match utils.get(util) { Some(&(uumain, _)) => { process::exit(uumain((vec![util_os].into_iter()).chain(args))); } @@ -90,9 +100,12 @@ fn main() { if util == "--help" || util == "-h" { // see if they want help on a specific util if let Some(util_os) = args.next() { - let util = util_os.as_os_str().to_string_lossy(); + let util = match util_os.to_str() { + Some(util) => util, + None => not_found(&util_os), + }; - match utils.get(&util[..]) { + match utils.get(util) { Some(&(uumain, _)) => { let code = uumain( (vec![util_os, OsString::from("--help")].into_iter()) @@ -101,17 +114,13 @@ fn main() { io::stdout().flush().expect("could not flush stdout"); process::exit(code); } - None => { - println!("{}: function/utility not found", util); - process::exit(1); - } + None => not_found(&util_os), } } usage(&utils, binary_as_util); process::exit(0); } else { - println!("{}: function/utility not found", util); - process::exit(1); + not_found(&util_os); } } } diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 78962d9db..0fd0fa5c4 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -9,6 +9,7 @@ use std::io::{stdout, Read, Write}; +use uucore::display::Quotable; use uucore::encoding::{wrap_print, Data, Format}; use uucore::InvalidEncodingHandling; @@ -40,8 +41,9 @@ impl Config { let name = values.next().unwrap(); if let Some(extra_op) = values.next() { return Err(format!( - "extra operand '{}'\nTry '{} --help' for more information.", - extra_op, app_name + "extra operand {}\nTry '{} --help' for more information.", + extra_op.quote(), + app_name )); } @@ -49,7 +51,7 @@ impl Config { None } else { if !Path::exists(Path::new(name)) { - return Err(format!("{}: No such file or directory", name)); + return Err(format!("{}: No such file or directory", name.maybe_quote())); } Some(name.to_owned()) } @@ -61,7 +63,7 @@ impl Config { .value_of(options::WRAP) .map(|num| { num.parse::() - .map_err(|_| format!("invalid wrap size: '{}'", num)) + .map_err(|_| format!("invalid wrap size: {}", num.quote())) }) .transpose()?; diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index a176b8c5d..9459ee86a 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -20,6 +20,7 @@ use clap::{crate_version, App, Arg}; use std::fs::{metadata, File}; use std::io::{self, Read, Write}; use thiserror::Error; +use uucore::display::Quotable; use uucore::error::UResult; #[cfg(unix)] @@ -386,7 +387,7 @@ fn cat_files(files: Vec, options: &OutputOptions) -> UResult<()> { for path in &files { if let Err(err) = cat_path(path, options, &mut state, &out_info) { - error_messages.push(format!("{}: {}", path, err)); + error_messages.push(format!("{}: {}", path.maybe_quote(), err)); } } if state.skipped_carriage_return { diff --git a/src/uu/chcon/src/chcon.rs b/src/uu/chcon/src/chcon.rs index 5f6a80bde..98ebebf34 100644 --- a/src/uu/chcon/src/chcon.rs +++ b/src/uu/chcon/src/chcon.rs @@ -2,7 +2,7 @@ #![allow(clippy::upper_case_acronyms)] -use uucore::{show_error, show_usage_error, show_warning}; +use uucore::{display::Quotable, show_error, show_usage_error, show_warning}; use clap::{App, Arg}; use selinux::{OpaqueSecurityContext, SecurityContext}; @@ -111,13 +111,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(context) => context, Err(_r) => { - show_error!("Invalid security context '{}'.", context.to_string_lossy()); + show_error!("Invalid security context {}.", context.quote()); return libc::EXIT_FAILURE; } }; if SecurityContext::from_c_str(&c_context, false).check() == Some(false) { - show_error!("Invalid security context '{}'.", context.to_string_lossy()); + show_error!("Invalid security context {}.", context.quote()); return libc::EXIT_FAILURE; } @@ -564,7 +564,7 @@ fn process_file( println!( "{}: Changing security context of: {}", uucore::util_name(), - file_full_name.to_string_lossy() + file_full_name.quote() ); } @@ -699,9 +699,9 @@ fn root_dev_ino_warn(dir_name: &Path) { ); } else { show_warning!( - "It is dangerous to operate recursively on '{}' (same as '/'). \ + "It is dangerous to operate recursively on {} (same as '/'). \ Use --{} to override this failsafe.", - dir_name.to_string_lossy(), + dir_name.quote(), options::preserve_root::NO_PRESERVE_ROOT, ); } @@ -726,8 +726,8 @@ fn emit_cycle_warning(file_name: &Path) { "Circular directory structure.\n\ This almost certainly means that you have a corrupted file system.\n\ NOTIFY YOUR SYSTEM MANAGER.\n\ -The following directory is part of the cycle '{}'.", - file_name.display() +The following directory is part of the cycle {}.", + file_name.quote() ) } diff --git a/src/uu/chcon/src/errors.rs b/src/uu/chcon/src/errors.rs index ecbd7d409..2d8f72e67 100644 --- a/src/uu/chcon/src/errors.rs +++ b/src/uu/chcon/src/errors.rs @@ -2,6 +2,8 @@ use std::ffi::OsString; use std::fmt::Write; use std::io; +use uucore::display::Quotable; + pub(crate) type Result = std::result::Result; #[derive(thiserror::Error, Debug)] @@ -30,7 +32,7 @@ pub(crate) enum Error { source: io::Error, }, - #[error("{operation} failed on '{}'", .operand1.to_string_lossy())] + #[error("{operation} failed on {}", .operand1.quote())] Io1 { operation: &'static str, operand1: OsString, diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index a077ab7a4..1795ad0d5 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -9,6 +9,7 @@ #[macro_use] extern crate uucore; +use uucore::display::Quotable; pub use uucore::entries; use uucore::error::{FromIo, UResult, USimpleError}; use uucore::perms::{chown_base, options, IfFrom}; @@ -32,7 +33,7 @@ fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option, Option, let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) { fs::metadata(&file) .map(|meta| Some(meta.gid())) - .map_err_context(|| format!("failed to get attributes of '{}'", file))? + .map_err_context(|| format!("failed to get attributes of {}", file.quote()))? } else { let group = matches.value_of(options::ARG_GROUP).unwrap_or_default(); if group.is_empty() { @@ -40,7 +41,12 @@ fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option, Option, } else { match entries::grp2gid(group) { Ok(g) => Some(g), - _ => return Err(USimpleError::new(1, format!("invalid group: '{}'", group))), + _ => { + return Err(USimpleError::new( + 1, + format!("invalid group: {}", group.quote()), + )) + } } } }; diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 26b3fd85b..e25202fbe 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use std::fs; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::Path; +use uucore::display::Quotable; use uucore::fs::display_permissions_unix; use uucore::libc::mode_t; #[cfg(not(windows))] @@ -75,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .value_of(options::REFERENCE) .and_then(|fref| match fs::metadata(fref) { Ok(meta) => Some(meta.mode()), - Err(err) => crash!(1, "cannot stat attributes of '{}': {}", fref, err), + Err(err) => crash!(1, "cannot stat attributes of {}: {}", fref.quote(), err), }); let modes = matches.value_of(options::MODE).unwrap(); // should always be Some because required let cmode = if mode_had_minus_prefix { @@ -223,21 +224,24 @@ impl Chmoder { if !file.exists() { if is_symlink(file) { println!( - "failed to change mode of '{}' from 0000 (---------) to 0000 (---------)", - filename + "failed to change mode of {} from 0000 (---------) to 0000 (---------)", + filename.quote() ); if !self.quiet { - show_error!("cannot operate on dangling symlink '{}'", filename); + show_error!("cannot operate on dangling symlink {}", filename.quote()); } } else if !self.quiet { - show_error!("cannot access '{}': No such file or directory", filename); + show_error!( + "cannot access {}: No such file or directory", + filename.quote() + ); } return Err(1); } if self.recursive && self.preserve_root && filename == "/" { show_error!( - "it is dangerous to operate recursively on '{}'\nuse --no-preserve-root to override this failsafe", - filename + "it is dangerous to operate recursively on {}\nuse --no-preserve-root to override this failsafe", + filename.quote() ); return Err(1); } @@ -270,15 +274,17 @@ impl Chmoder { if is_symlink(file) { if self.verbose { println!( - "neither symbolic link '{}' nor referent has been changed", - file.display() + "neither symbolic link {} nor referent has been changed", + file.quote() ); } return Ok(()); } else if err.kind() == std::io::ErrorKind::PermissionDenied { - show_error!("'{}': Permission denied", file.display()); + // These two filenames would normally be conditionally + // quoted, but GNU's tests expect them to always be quoted + show_error!("{}: Permission denied", file.quote()); } else { - show_error!("'{}': {}", file.display(), err); + show_error!("{}: {}", file.quote(), err); } return Err(1); } @@ -325,7 +331,7 @@ impl Chmoder { if (new_mode & !naively_expected_new_mode) != 0 { show_error!( "{}: new permissions are {}, not {}", - file.display(), + file.maybe_quote(), display_permissions_unix(new_mode as mode_t, false), display_permissions_unix(naively_expected_new_mode as mode_t, false) ); @@ -342,8 +348,8 @@ impl Chmoder { if fperm == mode { if self.verbose && !self.changes { println!( - "mode of '{}' retained as {:04o} ({})", - file.display(), + "mode of {} retained as {:04o} ({})", + file.quote(), fperm, display_permissions_unix(fperm as mode_t, false), ); @@ -355,8 +361,8 @@ impl Chmoder { } if self.verbose { println!( - "failed to change mode of file '{}' from {:04o} ({}) to {:04o} ({})", - file.display(), + "failed to change mode of file {} from {:04o} ({}) to {:04o} ({})", + file.quote(), fperm, display_permissions_unix(fperm as mode_t, false), mode, @@ -367,8 +373,8 @@ impl Chmoder { } else { if self.verbose || self.changes { println!( - "mode of '{}' changed from {:04o} ({}) to {:04o} ({})", - file.display(), + "mode of {} changed from {:04o} ({}) to {:04o} ({})", + file.quote(), fperm, display_permissions_unix(fperm as mode_t, false), mode, diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 4abb9ac61..5525f9325 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -9,6 +9,7 @@ #[macro_use] extern crate uucore; +use uucore::display::Quotable; pub use uucore::entries::{self, Group, Locate, Passwd}; use uucore::perms::{chown_base, options, IfFrom}; @@ -44,7 +45,7 @@ fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Optio let dest_gid: Option; if let Some(file) = matches.value_of(options::REFERENCE) { let meta = fs::metadata(&file) - .map_err_context(|| format!("failed to get attributes of '{}'", file))?; + .map_err_context(|| format!("failed to get attributes of {}", file.quote()))?; dest_gid = Some(meta.gid()); dest_uid = Some(meta.uid()); } else { @@ -173,7 +174,7 @@ fn parse_spec(spec: &str) -> UResult<(Option, Option)> { let uid = if usr_only || usr_grp { Some( Passwd::locate(args[0]) - .map_err(|_| USimpleError::new(1, format!("invalid user: '{}'", spec)))? + .map_err(|_| USimpleError::new(1, format!("invalid user: {}", spec.quote())))? .uid(), ) } else { @@ -182,7 +183,7 @@ fn parse_spec(spec: &str) -> UResult<(Option, Option)> { let gid = if grp_only || usr_grp { Some( Group::locate(args[1]) - .map_err(|_| USimpleError::new(1, format!("invalid group: '{}'", spec)))? + .map_err(|_| USimpleError::new(1, format!("invalid group: {}", spec.quote())))? .gid(), ) } else { diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index c0f392913..240b5eafe 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -15,6 +15,7 @@ use std::ffi::CString; use std::io::Error; use std::path::Path; use std::process::Command; +use uucore::display::Quotable; use uucore::libc::{self, chroot, setgid, setgroups, setuid}; use uucore::{entries, InvalidEncodingHandling}; @@ -53,8 +54,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !newroot.is_dir() { crash!( 1, - "cannot change root directory to `{}`: no such directory", - newroot.display() + "cannot change root directory to {}: no such directory", + newroot.quote() ); } @@ -170,7 +171,6 @@ fn set_context(root: &Path, options: &clap::ArgMatches) { } fn enter_chroot(root: &Path) { - let root_str = root.display(); std::env::set_current_dir(root).unwrap(); let err = unsafe { chroot(CString::new(".").unwrap().as_bytes_with_nul().as_ptr() as *const libc::c_char) @@ -179,7 +179,7 @@ fn enter_chroot(root: &Path) { crash!( 1, "cannot chroot to {}: {}", - root_str, + root.quote(), Error::last_os_error() ) }; @@ -189,7 +189,7 @@ fn set_main_group(group: &str) { if !group.is_empty() { let group_id = match entries::grp2gid(group) { Ok(g) => g, - _ => crash!(1, "no such group: {}", group), + _ => crash!(1, "no such group: {}", group.maybe_quote()), }; let err = unsafe { setgid(group_id) }; if err != 0 { @@ -234,7 +234,12 @@ fn set_user(user: &str) { let user_id = entries::usr2uid(user).unwrap(); let err = unsafe { setuid(user_id as libc::uid_t) }; if err != 0 { - crash!(1, "cannot set user to {}: {}", user, Error::last_os_error()) + crash!( + 1, + "cannot set user to {}: {}", + user.maybe_quote(), + Error::last_os_error() + ) } } } diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index a5beec368..e682aa70c 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use std::fs::File; use std::io::{self, stdin, BufReader, Read}; use std::path::Path; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; // NOTE: CRC_TABLE_LEN *must* be <= 256 as we cast 0..CRC_TABLE_LEN to u8 @@ -191,7 +192,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { match cksum("-") { Ok((crc, size)) => println!("{} {}", crc, size), Err(err) => { - show_error!("{}", err); + show_error!("-: {}", err); return 2; } } @@ -203,7 +204,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { match cksum(fname.as_ref()) { Ok((crc, size)) => println!("{} {} {}", crc, size, fname), Err(err) => { - show_error!("'{}' {}", fname, err); + show_error!("{}: {}", fname.maybe_quote(), err); exit_code = 2; } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e9e76237b..cd33f9fa6 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -18,6 +18,7 @@ extern crate quick_error; #[macro_use] extern crate uucore; +use uucore::display::Quotable; #[cfg(windows)] use winapi::um::fileapi::CreateFileW; #[cfg(windows)] @@ -541,8 +542,8 @@ impl FromStr for Attribute { "xattr" => Attribute::Xattr, _ => { return Err(Error::InvalidArgument(format!( - "invalid attribute '{}'", - value + "invalid attribute {}", + value.quote() ))); } }) @@ -659,8 +660,8 @@ impl Options { "never" => ReflinkMode::Never, value => { return Err(Error::InvalidArgument(format!( - "invalid argument '{}' for \'reflink\'", - value + "invalid argument {} for \'reflink\'", + value.quote() ))); } } @@ -832,7 +833,7 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu let mut seen_sources = HashSet::with_capacity(sources.len()); for source in sources { if seen_sources.contains(source) { - show_warning!("source '{}' specified more than once", source.display()); + show_warning!("source {} specified more than once", source.quote()); } else { let mut found_hard_link = false; if preserve_hard_links { @@ -873,8 +874,8 @@ fn construct_dest_path( ) -> CopyResult { if options.no_target_dir && target.is_dir() { return Err(format!( - "cannot overwrite directory '{}' with non-directory", - target.display() + "cannot overwrite directory {} with non-directory", + target.quote() ) .into()); } @@ -941,7 +942,7 @@ fn adjust_canonicalization(p: &Path) -> Cow { /// will not cause a short-circuit. fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyResult<()> { if !options.recursive { - return Err(format!("omitting directory '{}'", root.display()).into()); + return Err(format!("omitting directory {}", root.quote()).into()); } // if no-dereference is enabled and this is a symlink, copy it as a file @@ -1041,12 +1042,12 @@ impl OverwriteMode { match *self { OverwriteMode::NoClobber => Err(Error::NotAllFilesCopied), OverwriteMode::Interactive(_) => { - if prompt_yes!("{}: overwrite {}? ", uucore::util_name(), path.display()) { + if prompt_yes!("{}: overwrite {}? ", uucore::util_name(), path.quote()) { Ok(()) } else { Err(Error::Skipped(format!( "Not overwriting {} at user request", - path.display() + path.quote() ))) } } @@ -1056,7 +1057,7 @@ impl OverwriteMode { } fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResult<()> { - let context = &*format!("'{}' -> '{}'", source.display().to_string(), dest.display()); + let context = &*format!("{} -> {}", source.quote(), dest.quote()); let source_metadata = fs::symlink_metadata(source).context(context)?; match *attribute { Attribute::Mode => { @@ -1152,7 +1153,7 @@ fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> { } fn context_for(src: &Path, dest: &Path) -> String { - format!("'{}' -> '{}'", src.display(), dest.display()) + format!("{} -> {}", src.quote(), dest.quote()) } /// Implements a simple backup copy for the destination file. @@ -1332,8 +1333,8 @@ fn copy_link(source: &Path, dest: &Path) -> CopyResult<()> { Some(name) => dest.join(name).into(), None => crash!( EXIT_ERR, - "cannot stat '{}': No such file or directory", - source.display() + "cannot stat {}: No such file or directory", + source.quote() ), } } else { @@ -1454,11 +1455,11 @@ fn copy_on_write_macos( pub fn verify_target_type(target: &Path, target_type: &TargetType) -> CopyResult<()> { match (target_type, target.is_dir()) { (&TargetType::Directory, false) => { - Err(format!("target: '{}' is not a directory", target.display()).into()) + Err(format!("target: {} is not a directory", target.quote()).into()) } (&TargetType::File, true) => Err(format!( - "cannot overwrite directory '{}' with non-directory", - target.display() + "cannot overwrite directory {} with non-directory", + target.quote() ) .into()), _ => Ok(()), diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index 977583a2f..dbf65b71d 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -10,6 +10,7 @@ use std::{ fs::{remove_file, File}, io::{BufRead, BufWriter, Write}, }; +use uucore::display::Quotable; mod csplit_error; mod patterns; @@ -734,7 +735,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let file = crash_if_err!(1, File::open(file_name)); let file_metadata = crash_if_err!(1, file.metadata()); if !file_metadata.is_file() { - crash!(1, "'{}' is not a regular file", file_name); + crash!(1, "{} is not a regular file", file_name.quote()); } crash_if_err!(1, csplit(&options, patterns, BufReader::new(file))); }; diff --git a/src/uu/csplit/src/csplit_error.rs b/src/uu/csplit/src/csplit_error.rs index 637cf8890..1d4823ee2 100644 --- a/src/uu/csplit/src/csplit_error.rs +++ b/src/uu/csplit/src/csplit_error.rs @@ -1,26 +1,28 @@ use std::io; use thiserror::Error; +use uucore::display::Quotable; + /// Errors thrown by the csplit command #[derive(Debug, Error)] pub enum CsplitError { #[error("IO error: {}", _0)] IoError(io::Error), - #[error("'{}': line number out of range", _0)] + #[error("{}: line number out of range", ._0.quote())] LineOutOfRange(String), - #[error("'{}': line number out of range on repetition {}", _0, _1)] + #[error("{}: line number out of range on repetition {}", ._0.quote(), _1)] LineOutOfRangeOnRepetition(String, usize), - #[error("'{}': match not found", _0)] + #[error("{}: match not found", ._0.quote())] MatchNotFound(String), - #[error("'{}': match not found on repetition {}", _0, _1)] + #[error("{}: match not found on repetition {}", ._0.quote(), _1)] MatchNotFoundOnRepetition(String, usize), #[error("line number must be greater than zero")] LineNumberIsZero, #[error("line number '{}' is smaller than preceding line number, {}", _0, _1)] LineNumberSmallerThanPrevious(usize, usize), - #[error("invalid pattern: {}", _0)] + #[error("{}: invalid pattern", ._0.quote())] InvalidPattern(String), - #[error("invalid number: '{}'", _0)] + #[error("invalid number: {}", ._0.quote())] InvalidNumber(String), #[error("incorrect conversion specification in suffix")] SuffixFormatIncorrect, diff --git a/src/uu/cut/src/cut.rs b/src/uu/cut/src/cut.rs index 783502e3d..35d92b83f 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -15,6 +15,7 @@ use clap::{crate_version, App, Arg}; use std::fs::File; use std::io::{stdin, stdout, BufReader, BufWriter, Read, Write}; use std::path::Path; +use uucore::display::Quotable; use self::searcher::Searcher; use uucore::ranges::Range; @@ -351,19 +352,19 @@ fn cut_files(mut filenames: Vec, mode: Mode) -> i32 { let path = Path::new(&filename[..]); if path.is_dir() { - show_error!("{}: Is a directory", filename); + show_error!("{}: Is a directory", filename.maybe_quote()); continue; } if path.metadata().is_err() { - show_error!("{}: No such file or directory", filename); + show_error!("{}: No such file or directory", filename.maybe_quote()); continue; } let file = match File::open(&path) { Ok(f) => f, Err(e) => { - show_error!("opening '{}': {}", &filename[..], e); + show_error!("opening {}: {}", filename.quote(), e); continue; } }; diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 7bf6298c0..b2ccf2e9f 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -17,6 +17,7 @@ use libc::{clock_settime, timespec, CLOCK_REALTIME}; use std::fs::File; use std::io::{BufRead, BufReader}; use std::path::PathBuf; +use uucore::display::Quotable; #[cfg(windows)] use winapi::{ shared::minwindef::WORD, @@ -145,7 +146,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let format = if let Some(form) = matches.value_of(OPT_FORMAT) { if !form.starts_with('+') { - eprintln!("date: invalid date '{}'", form); + eprintln!("date: invalid date {}", form.quote()); return 1; } let form = form[1..].to_string(); @@ -174,7 +175,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let set_to = match matches.value_of(OPT_SET).map(parse_date) { None => None, Some(Err((input, _err))) => { - eprintln!("date: invalid date '{}'", input); + eprintln!("date: invalid date {}", input.quote()); return 1; } Some(Ok(date)) => Some(date), @@ -240,7 +241,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { println!("{}", formatted); } Err((input, _err)) => { - println!("date: invalid date '{}'", input); + println!("date: invalid date {}", input.quote()); } } } diff --git a/src/uu/dircolors/src/dircolors.rs b/src/uu/dircolors/src/dircolors.rs index 2b9c25ba3..5d0b3ac3e 100644 --- a/src/uu/dircolors/src/dircolors.rs +++ b/src/uu/dircolors/src/dircolors.rs @@ -17,6 +17,7 @@ use std::fs::File; use std::io::{BufRead, BufReader}; use clap::{crate_version, App, Arg}; +use uucore::display::Quotable; mod options { pub const BOURNE_SHELL: &str = "bourne-shell"; @@ -94,9 +95,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if matches.is_present(options::PRINT_DATABASE) { if !files.is_empty() { show_usage_error!( - "extra operand '{}'\nfile operands cannot be combined with \ + "extra operand {}\nfile operands cannot be combined with \ --print-database (-p)", - files[0] + files[0].quote() ); return 1; } @@ -126,7 +127,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { result = parse(INTERNAL_DB.lines(), out_format, "") } else { if files.len() > 1 { - show_usage_error!("extra operand '{}'", files[1]); + show_usage_error!("extra operand {}", files[1].quote()); return 1; } match File::open(files[0]) { @@ -135,7 +136,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { result = parse(fin.lines().filter_map(Result::ok), out_format, files[0]) } Err(e) => { - show_error!("{}: {}", files[0], e); + show_error!("{}: {}", files[0].maybe_quote(), e); return 1; } } @@ -314,7 +315,8 @@ where if val.is_empty() { return Err(format!( "{}:{}: invalid line; missing second token", - fp, num + fp.maybe_quote(), + num )); } let lower = key.to_lowercase(); @@ -341,7 +343,12 @@ where } else if let Some(s) = table.get(lower.as_str()) { result.push_str(format!("{}={}:", s, val).as_str()); } else { - return Err(format!("{}:{}: unrecognized keyword {}", fp, num, key)); + return Err(format!( + "{}:{}: unrecognized keyword {}", + fp.maybe_quote(), + num, + key + )); } } } diff --git a/src/uu/dirname/src/dirname.rs b/src/uu/dirname/src/dirname.rs index 3e91d598a..601d93ac0 100644 --- a/src/uu/dirname/src/dirname.rs +++ b/src/uu/dirname/src/dirname.rs @@ -10,6 +10,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use std::path::Path; +use uucore::display::print_verbatim; use uucore::error::{UResult, UUsageError}; use uucore::InvalidEncodingHandling; @@ -65,7 +66,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if d.components().next() == None { print!(".") } else { - print!("{}", d.to_string_lossy()); + print_verbatim(d).unwrap(); } } None => { diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 685064dfc..9fd44b001 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -32,6 +32,7 @@ use std::path::PathBuf; use std::str::FromStr; use std::time::{Duration, UNIX_EPOCH}; use std::{error::Error, fmt::Display}; +use uucore::display::{print_verbatim, Quotable}; use uucore::error::{UError, UResult}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::InvalidEncodingHandling; @@ -293,9 +294,9 @@ fn du( Err(e) => { safe_writeln!( stderr(), - "{}: cannot read directory '{}': {}", + "{}: cannot read directory {}: {}", options.util_name, - my_stat.path.display(), + my_stat.path.quote(), e ); return Box::new(iter::once(my_stat)); @@ -334,11 +335,11 @@ fn du( } Err(error) => match error.kind() { ErrorKind::PermissionDenied => { - let description = format!("cannot access '{}'", entry.path().display()); + let description = format!("cannot access {}", entry.path().quote()); let error_message = "Permission denied"; show_error_custom_description!(description, "{}", error_message) } - _ => show_error!("cannot access '{}': {}", entry.path().display(), error), + _ => show_error!("cannot access {}: {}", entry.path().quote(), error), }, }, Err(error) => show_error!("{}", error), @@ -411,26 +412,30 @@ enum DuError { impl Display for DuError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - DuError::InvalidMaxDepthArg(s) => write!(f, "invalid maximum depth '{}'", s), + DuError::InvalidMaxDepthArg(s) => write!(f, "invalid maximum depth {}", s.quote()), DuError::SummarizeDepthConflict(s) => { - write!(f, "summarizing conflicts with --max-depth={}", s) + write!( + f, + "summarizing conflicts with --max-depth={}", + s.maybe_quote() + ) } DuError::InvalidTimeStyleArg(s) => write!( f, - "invalid argument '{}' for 'time style' + "invalid argument {} for 'time style' Valid arguments are: - 'full-iso' - 'long-iso' - 'iso' Try '{} --help' for more information.", - s, + s.quote(), uucore::execution_phrase() ), DuError::InvalidTimeArg(s) => write!( f, - "Invalid argument '{}' for --time. + "Invalid argument {} for --time. 'birth' and 'creation' arguments are not supported on this platform.", - s + s.quote() ), } } @@ -566,21 +571,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; if !summarize || index == len - 1 { let time_str = tm.format(time_format_str).to_string(); - print!( - "{}\t{}\t{}{}", - convert_size(size), - time_str, - stat.path.display(), - line_separator - ); + print!("{}\t{}\t", convert_size(size), time_str); + print_verbatim(stat.path).unwrap(); + print!("{}", line_separator); } } else if !summarize || index == len - 1 { - print!( - "{}\t{}{}", - convert_size(size), - stat.path.display(), - line_separator - ); + print!("{}\t", convert_size(size)); + print_verbatim(stat.path).unwrap(); + print!("{}", line_separator); } if options.total && index == (len - 1) { // The last element will be the total size of the the path under @@ -590,7 +588,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } Err(_) => { - show_error!("{}: {}", path_string, "No such file or directory"); + show_error!( + "{}: {}", + path_string.maybe_quote(), + "No such file or directory" + ); } } } @@ -837,8 +839,8 @@ fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String // GNU's du echos affected flag, -B or --block-size (-t or --threshold), depending user's selection // GNU's du does distinguish between "invalid (suffix in) argument" match error { - ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), - ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), + ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } } diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index baa3ca86c..af889d093 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -65,8 +65,14 @@ fn parse_name_value_opt<'a>(opts: &mut Options<'a>, opt: &'a str) -> Result(opts: &mut Options<'a>, opt: &'a str) -> Result<(), i32> { if opts.null { - eprintln!("{}: cannot specify --null (-0) with command", crate_name!()); - eprintln!("Type \"{} --help\" for detailed information", crate_name!()); + eprintln!( + "{}: cannot specify --null (-0) with command", + uucore::util_name() + ); + eprintln!( + "Type \"{} --help\" for detailed information", + uucore::execution_phrase() + ); Err(1) } else { opts.program.push(opt); diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index f5a5686f6..b09b8aaab 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -17,6 +17,7 @@ use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; use std::str::from_utf8; use unicode_width::UnicodeWidthChar; +use uucore::display::Quotable; static ABOUT: &str = "Convert tabs in each FILE to spaces, writing to standard output. With no FILE, or when FILE is -, read standard input."; @@ -216,7 +217,7 @@ fn open(path: String) -> BufReader> { } else { file_buf = match File::open(&path[..]) { Ok(a) => a, - Err(e) => crash!(1, "{}: {}\n", &path[..], e), + Err(e) => crash!(1, "{}: {}\n", path.maybe_quote(), e), }; BufReader::new(Box::new(file_buf) as Box) } diff --git a/src/uu/factor/src/cli.rs b/src/uu/factor/src/cli.rs index 95b8bb866..30541c244 100644 --- a/src/uu/factor/src/cli.rs +++ b/src/uu/factor/src/cli.rs @@ -16,6 +16,7 @@ use std::io::{self, stdin, stdout, BufRead, Write}; mod factor; use clap::{crate_version, App, Arg}; pub use factor::*; +use uucore::display::Quotable; mod miller_rabin; pub mod numeric; @@ -52,7 +53,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if let Some(values) = matches.values_of(options::NUMBER) { for number in values { if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { - show_warning!("{}: {}", number, e); + show_warning!("{}: {}", number.maybe_quote(), e); } } } else { @@ -61,7 +62,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for line in stdin.lock().lines() { for number in line.unwrap().split_whitespace() { if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { - show_warning!("{}: {}", number, e); + show_warning!("{}: {}", number.maybe_quote(), e); } } } diff --git a/src/uu/fmt/src/fmt.rs b/src/uu/fmt/src/fmt.rs index 5203745a1..df5d971e5 100644 --- a/src/uu/fmt/src/fmt.rs +++ b/src/uu/fmt/src/fmt.rs @@ -15,6 +15,7 @@ use std::cmp; use std::fs::File; use std::io::{stdin, stdout, Write}; use std::io::{BufReader, BufWriter, Read}; +use uucore::display::Quotable; use self::linebreak::break_lines; use self::parasplit::ParagraphStream; @@ -187,7 +188,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { _ => match File::open(i) { Ok(f) => BufReader::new(Box::new(f) as Box), Err(e) => { - show_warning!("{}: {}", i, e); + show_warning!("{}: {}", i.maybe_quote(), e); continue; } }, diff --git a/src/uu/groups/src/groups.rs b/src/uu/groups/src/groups.rs index c2af5c4b0..43e2a2239 100644 --- a/src/uu/groups/src/groups.rs +++ b/src/uu/groups/src/groups.rs @@ -17,7 +17,10 @@ #[macro_use] extern crate uucore; -use uucore::entries::{get_groups_gnu, gid2grp, Locate, Passwd}; +use uucore::{ + display::Quotable, + entries::{get_groups_gnu, gid2grp, Locate, Passwd}, +}; use clap::{crate_version, App, Arg}; @@ -77,7 +80,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .join(" ") ); } else { - show_error!("'{}': no such user", user); + show_error!("{}: no such user", user.quote()); exit_code = 1; } } diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index f308da300..2081d7612 100644 --- a/src/uu/hashsum/src/hashsum.rs +++ b/src/uu/hashsum/src/hashsum.rs @@ -34,6 +34,7 @@ use std::io::{self, stdin, BufRead, BufReader, Read}; use std::iter; use std::num::ParseIntError; use std::path::Path; +use uucore::display::Quotable; const NAME: &str = "hashsum"; @@ -525,7 +526,7 @@ where if options.warn { show_warning!( "{}: {}: improperly formatted {} checksum line", - filename.display(), + filename.maybe_quote(), i + 1, options.algoname ); @@ -546,6 +547,10 @@ where ) ) .to_ascii_lowercase(); + // FIXME: (How) should these be quoted? + // They seem like they would be processed programmatically, and + // our ordinary quoting might interfere, but newlines should be + // sanitized probably if sum == real_sum { if !options.quiet { println!("{}: OK", ck_filename); diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 760cb44d5..ead734088 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -9,6 +9,7 @@ use clap::{crate_version, App, Arg}; use std::convert::TryFrom; use std::ffi::OsString; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; +use uucore::display::Quotable; use uucore::{crash, show_error_custom_description}; const EXIT_FAILURE: i32 = 1; @@ -127,10 +128,10 @@ fn arg_iterate<'a>( match parse::parse_obsolete(s) { Some(Ok(iter)) => Ok(Box::new(vec![first].into_iter().chain(iter).chain(args))), Some(Err(e)) => match e { - parse::ParseError::Syntax => Err(format!("bad argument format: '{}'", s)), + parse::ParseError::Syntax => Err(format!("bad argument format: {}", s.quote())), parse::ParseError::Overflow => Err(format!( - "invalid argument: '{}' Value too large for defined datatype", - s + "invalid argument: {} Value too large for defined datatype", + s.quote() )), }, None => Ok(Box::new(vec![first, second].into_iter().chain(args))), @@ -418,7 +419,7 @@ fn uu_head(options: &HeadOptions) -> Result<(), u32> { let mut file = match std::fs::File::open(name) { Ok(f) => f, Err(err) => { - let prefix = format!("cannot open '{}' for reading", name); + let prefix = format!("cannot open {} for reading", name.quote()); match err.kind() { ErrorKind::NotFound => { show_error_custom_description!(prefix, "No such file or directory"); diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 4e8e30e52..1229b577e 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -41,6 +41,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use std::ffi::CStr; +use uucore::display::Quotable; use uucore::entries::{self, Group, Locate, Passwd}; use uucore::error::UResult; use uucore::error::{set_exit_code, USimpleError}; @@ -230,7 +231,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { match Passwd::locate(users[i].as_str()) { Ok(p) => Some(p), Err(_) => { - show_error!("'{}': no such user", users[i]); + show_error!("{}: no such user", users[i].quote()); set_exit_code(1); if i + 1 >= users.len() { break; diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index b72721d20..a9f91f658 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -16,6 +16,7 @@ use clap::{crate_version, App, Arg, ArgMatches}; use file_diff::diff; use filetime::{set_file_times, FileTime}; use uucore::backup_control::{self, BackupMode}; +use uucore::display::Quotable; use uucore::entries::{grp2gid, usr2uid}; use uucore::error::{FromIo, UError, UIoError, UResult}; use uucore::mode::get_umask; @@ -95,40 +96,30 @@ impl Display for InstallError { ) } IE::CreateDirFailed(dir, e) => { - Display::fmt(&uio_error!(e, "failed to create {}", dir.display()), f) + Display::fmt(&uio_error!(e, "failed to create {}", dir.quote()), f) } - IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.display()), + IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.quote()), IE::InvalidTarget(target) => write!( f, "invalid target {}: No such file or directory", - target.display() + target.quote() ), IE::TargetDirIsntDir(target) => { - write!(f, "target '{}' is not a directory", target.display()) + write!(f, "target {} is not a directory", target.quote()) } IE::BackupFailed(from, to, e) => Display::fmt( - &uio_error!( - e, - "cannot backup '{}' to '{}'", - from.display(), - to.display() - ), + &uio_error!(e, "cannot backup {} to {}", from.quote(), to.quote()), f, ), IE::InstallFailed(from, to, e) => Display::fmt( - &uio_error!( - e, - "cannot install '{}' to '{}'", - from.display(), - to.display() - ), + &uio_error!(e, "cannot install {} to {}", from.quote(), to.quote()), f, ), IE::StripProgramFailed(msg) => write!(f, "strip program failed: {}", msg), IE::MetadataFailed(e) => Display::fmt(&uio_error!(e, ""), f), - IE::NoSuchUser(user) => write!(f, "no such user: {}", user), - IE::NoSuchGroup(group) => write!(f, "no such group: {}", group), - IE::OmittingDirectory(dir) => write!(f, "omitting directory '{}'", dir.display()), + IE::NoSuchUser(user) => write!(f, "no such user: {}", user.maybe_quote()), + IE::NoSuchGroup(group) => write!(f, "no such group: {}", group.maybe_quote()), + IE::OmittingDirectory(dir) => write!(f, "omitting directory {}", dir.quote()), } } } @@ -416,14 +407,14 @@ fn directory(paths: Vec, b: Behavior) -> UResult<()> { // the default mode. Hence it is safe to use fs::create_dir_all // and then only modify the target's dir mode. if let Err(e) = - fs::create_dir_all(path).map_err_context(|| format!("{}", path.display())) + fs::create_dir_all(path).map_err_context(|| path.maybe_quote().to_string()) { show!(e); continue; } if b.verbose { - println!("creating directory '{}'", path.display()); + println!("creating directory {}", path.quote()); } } @@ -445,7 +436,7 @@ fn directory(paths: Vec, b: Behavior) -> UResult<()> { fn is_new_file_path(path: &Path) -> bool { !path.exists() && (path.parent().map(Path::is_dir).unwrap_or(true) - || path.parent().unwrap().to_string_lossy().is_empty()) // In case of a simple file + || path.parent().unwrap().as_os_str().is_empty()) // In case of a simple file } /// Perform an install, given a list of paths and behavior. @@ -502,7 +493,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR if !sourcepath.exists() { let err = UIoError::new( std::io::ErrorKind::NotFound, - format!("cannot stat '{}'", sourcepath.display()), + format!("cannot stat {}", sourcepath.quote()), ); show!(err); continue; @@ -566,7 +557,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { } } - if from.to_string_lossy() == "/dev/null" { + if from.as_os_str() == "/dev/null" { /* workaround a limitation of fs::copy * https://github.com/rust-lang/rust/issues/79390 */ @@ -674,9 +665,9 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { } if b.verbose { - print!("'{}' -> '{}'", from.display(), to.display()); + print!("{} -> {}", from.quote(), to.quote()); match backup_path { - Some(path) => println!(" (backup: '{}')", path.display()), + Some(path) => println!(" (backup: {})", path.quote()), None => println!(), } } diff --git a/src/uu/install/src/mode.rs b/src/uu/install/src/mode.rs index fd4cee50e..310e1fb43 100644 --- a/src/uu/install/src/mode.rs +++ b/src/uu/install/src/mode.rs @@ -22,8 +22,9 @@ pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result Result<(), ()> { use std::os::unix::fs::PermissionsExt; + use uucore::display::Quotable; fs::set_permissions(path, fs::Permissions::from_mode(mode)).map_err(|err| { - show_error!("{}: chmod failed with error {}", path.display(), err); + show_error!("{}: chmod failed with error {}", path.maybe_quote(), err); }) } diff --git a/src/uu/join/src/join.rs b/src/uu/join/src/join.rs index ae991489f..51dfeec6f 100644 --- a/src/uu/join/src/join.rs +++ b/src/uu/join/src/join.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use std::cmp::Ordering; use std::fs::File; use std::io::{stdin, BufRead, BufReader, Lines, Stdin}; +use uucore::display::Quotable; static NAME: &str = "join"; @@ -181,18 +182,18 @@ impl Spec { return Spec::Key; } - crash!(1, "invalid field specifier: '{}'", format); + crash!(1, "invalid field specifier: {}", format.quote()); } Some('1') => FileNum::File1, Some('2') => FileNum::File2, - _ => crash!(1, "invalid file number in field spec: '{}'", format), + _ => crash!(1, "invalid file number in field spec: {}", format.quote()), }; if let Some('.') = chars.next() { return Spec::Field(file_num, parse_field_number(chars.as_str())); } - crash!(1, "invalid field specifier: '{}'", format); + crash!(1, "invalid field specifier: {}", format.quote()); } } @@ -245,7 +246,7 @@ impl<'a> State<'a> { } else { match File::open(name) { Ok(file) => Box::new(BufReader::new(file)) as Box, - Err(err) => crash!(1, "{}: {}", name, err), + Err(err) => crash!(1, "{}: {}", name.maybe_quote(), err), } }; @@ -393,7 +394,11 @@ impl<'a> State<'a> { let diff = input.compare(self.get_current_key(), line.get_field(self.key)); if diff == Ordering::Greater { - eprintln!("{}:{}: is not sorted", self.file_name, self.line_num); + eprintln!( + "{}:{}: is not sorted", + self.file_name.maybe_quote(), + self.line_num + ); // This is fatal if the check is enabled. if input.check_order == CheckOrder::Enabled { @@ -727,7 +732,7 @@ fn get_field_number(keys: Option, key: Option) -> usize { fn parse_field_number(value: &str) -> usize { match value.parse::() { Ok(result) if result > 0 => result - 1, - _ => crash!(1, "invalid field number: '{}'", value), + _ => crash!(1, "invalid field number: {}", value.quote()), } } @@ -735,7 +740,7 @@ fn parse_file_number(value: &str) -> FileNum { match value { "1" => FileNum::File1, "2" => FileNum::File2, - value => crash!(1, "invalid file number: '{}'", value), + value => crash!(1, "invalid file number: {}", value.quote()), } } diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 47bd97dbc..494dc0602 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -13,6 +13,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use libc::{c_int, pid_t}; use std::io::Error; +use uucore::display::Quotable; use uucore::error::{UResult, USimpleError}; use uucore::signals::ALL_SIGNALS; use uucore::InvalidEncodingHandling; @@ -154,7 +155,7 @@ fn print_signal(signal_name_or_value: &str) -> UResult<()> { } Err(USimpleError::new( 1, - format!("unknown signal name {}", signal_name_or_value), + format!("unknown signal name {}", signal_name_or_value.quote()), )) } @@ -190,7 +191,7 @@ fn kill(signalname: &str, pids: &[String]) -> UResult<()> { None => { return Err(USimpleError::new( 1, - format!("unknown signal name {}", signalname), + format!("unknown signal name {}", signalname.quote()), )); } }; @@ -204,7 +205,7 @@ fn kill(signalname: &str, pids: &[String]) -> UResult<()> { Err(e) => { return Err(USimpleError::new( 1, - format!("failed to parse argument {}: {}", pid, e), + format!("failed to parse argument {}: {}", pid.quote(), e), )); } }; diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 4eeb637e7..e480d8f13 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -11,11 +11,12 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +use uucore::display::Quotable; use uucore::error::{UError, UResult}; use std::borrow::Cow; use std::error::Error; -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; use std::fmt::Display; use std::fs; @@ -49,26 +50,26 @@ pub enum OverwriteMode { #[derive(Debug)] enum LnError { - TargetIsDirectory(String), + TargetIsDirectory(PathBuf), SomeLinksFailed, FailedToLink(String), - MissingDestination(String), - ExtraOperand(String), + MissingDestination(PathBuf), + ExtraOperand(OsString), } impl Display for LnError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::TargetIsDirectory(s) => write!(f, "target '{}' is not a directory", s), - Self::FailedToLink(s) => write!(f, "failed to link '{}'", s), + Self::TargetIsDirectory(s) => write!(f, "target {} is not a directory", s.quote()), + Self::FailedToLink(e) => write!(f, "failed to link: {}", e), Self::SomeLinksFailed => write!(f, "some links failed to create"), Self::MissingDestination(s) => { - write!(f, "missing destination file operand after '{}'", s) + write!(f, "missing destination file operand after {}", s.quote()) } Self::ExtraOperand(s) => write!( f, - "extra operand '{}'\nTry '{} --help' for more information.", - s, + "extra operand {}\nTry '{} --help' for more information.", + s.quote(), uucore::execution_phrase() ), } @@ -279,10 +280,10 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { // 1st form. Now there should be only two operands, but if -T is // specified we may have a wrong number of operands. if files.len() == 1 { - return Err(LnError::MissingDestination(files[0].to_string_lossy().into()).into()); + return Err(LnError::MissingDestination(files[0].clone()).into()); } if files.len() > 2 { - return Err(LnError::ExtraOperand(files[2].display().to_string()).into()); + return Err(LnError::ExtraOperand(files[2].clone().into()).into()); } assert!(!files.is_empty()); @@ -294,7 +295,7 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> UResult<()> { if !target_dir.is_dir() { - return Err(LnError::TargetIsDirectory(target_dir.display().to_string()).into()); + return Err(LnError::TargetIsDirectory(target_dir.to_owned()).into()); } let mut all_successful = true; @@ -306,7 +307,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) if is_symlink(target_dir) { if target_dir.is_file() { if let Err(e) = fs::remove_file(target_dir) { - show_error!("Could not update {}: {}", target_dir.display(), e) + show_error!("Could not update {}: {}", target_dir.quote(), e) }; } if target_dir.is_dir() { @@ -314,7 +315,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) // considered as a dir // See test_ln::test_symlink_no_deref_dir if let Err(e) = fs::remove_dir(target_dir) { - show_error!("Could not update {}: {}", target_dir.display(), e) + show_error!("Could not update {}: {}", target_dir.quote(), e) }; } } @@ -332,10 +333,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) } } None => { - show_error!( - "cannot stat '{}': No such file or directory", - srcpath.display() - ); + show_error!("cannot stat {}: No such file or directory", srcpath.quote()); all_successful = false; continue; } @@ -344,9 +342,9 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) if let Err(e) = link(srcpath, &targetpath, settings) { show_error!( - "cannot link '{}' to '{}': {}", - targetpath.display(), - srcpath.display(), + "cannot link {} to {}: {}", + targetpath.quote(), + srcpath.quote(), e ); all_successful = false; @@ -399,7 +397,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> { match settings.overwrite { OverwriteMode::NoClobber => {} OverwriteMode::Interactive => { - print!("{}: overwrite '{}'? ", uucore::util_name(), dst.display()); + print!("{}: overwrite {}? ", uucore::util_name(), dst.quote()); if !read_yes() { return Ok(()); } @@ -426,9 +424,9 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> { } if settings.verbose { - print!("'{}' -> '{}'", dst.display(), &source.display()); + print!("{} -> {}", dst.quote(), source.quote()); match backup_path { - Some(path) => println!(" (backup: '{}')", path.display()), + Some(path) => println!(" (backup: {})", path.quote()), None => println!(), } } diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6f63c2a4a..fad30157c 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -40,7 +40,10 @@ use std::{ time::Duration, }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; -use uucore::error::{set_exit_code, FromIo, UError, UResult}; +use uucore::{ + display::Quotable, + error::{set_exit_code, FromIo, UError, UResult}, +}; use unicode_width::UnicodeWidthStr; #[cfg(unix)] @@ -150,8 +153,8 @@ impl Error for LsError {} impl Display for LsError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - LsError::InvalidLineWidth(s) => write!(f, "invalid line width: '{}'", s), - LsError::NoMetadata(p) => write!(f, "could not open file: '{}'", p.display()), + LsError::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()), + LsError::NoMetadata(p) => write!(f, "could not open file: {}", p.quote()), } } } @@ -410,18 +413,18 @@ impl Config { }, None => match termsize::get() { Some(size) => size.cols, - None => match std::env::var("COLUMNS") { - Ok(columns) => match columns.parse() { - Ok(columns) => columns, - Err(_) => { + None => match std::env::var_os("COLUMNS") { + Some(columns) => match columns.to_str().and_then(|s| s.parse().ok()) { + Some(columns) => columns, + None => { show_error!( - "ignoring invalid width in environment variable COLUMNS: '{}'", - columns + "ignoring invalid width in environment variable COLUMNS: {}", + columns.quote() ); DEFAULT_TERM_WIDTH } }, - Err(_) => DEFAULT_TERM_WIDTH, + None => DEFAULT_TERM_WIDTH, }, }, }; @@ -538,7 +541,7 @@ impl Config { Ok(p) => { ignore_patterns.add(p); } - Err(_) => show_warning!("Invalid pattern for ignore: '{}'", pattern), + Err(_) => show_warning!("Invalid pattern for ignore: {}", pattern.quote()), } } @@ -548,7 +551,7 @@ impl Config { Ok(p) => { ignore_patterns.add(p); } - Err(_) => show_warning!("Invalid pattern for hide: '{}'", pattern), + Err(_) => show_warning!("Invalid pattern for hide: {}", pattern.quote()), } } } @@ -1255,7 +1258,7 @@ fn list(locs: Vec<&Path>, config: Config) -> UResult<()> { if path_data.md().is_none() { show!(std::io::ErrorKind::NotFound - .map_err_context(|| format!("cannot access '{}'", path_data.p_buf.display()))); + .map_err_context(|| format!("cannot access {}", path_data.p_buf.quote()))); // We found an error, no need to continue the execution continue; } diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 848f79988..92c068408 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -12,6 +12,7 @@ use clap::OsValues; use clap::{crate_version, App, Arg}; use std::fs; use std::path::Path; +use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError}; static ABOUT: &str = "Create the given DIRECTORY(ies) if they do not exist"; @@ -43,7 +44,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Not tested on Windows let mode: u16 = match matches.value_of(options::MODE) { Some(m) => u16::from_str_radix(m, 8) - .map_err(|_| USimpleError::new(1, format!("invalid mode '{}'", m)))?, + .map_err(|_| USimpleError::new(1, format!("invalid mode {}", m.quote())))?, None => 0o755_u16, }; @@ -100,13 +101,13 @@ fn mkdir(path: &Path, recursive: bool, mode: u16, verbose: bool) -> UResult<()> fs::create_dir }; - create_dir(path).map_err_context(|| format!("cannot create directory '{}'", path.display()))?; + create_dir(path).map_err_context(|| format!("cannot create directory {}", path.quote()))?; if verbose { println!( - "{}: created directory '{}'", + "{}: created directory {}", uucore::util_name(), - path.display() + path.quote() ); } @@ -121,7 +122,7 @@ fn chmod(path: &Path, mode: u16) -> UResult<()> { let mode = Permissions::from_mode(u32::from(mode)); set_permissions(path, mode) - .map_err_context(|| format!("cannot set permissions '{}'", path.display())) + .map_err_context(|| format!("cannot set permissions {}", path.quote())) } #[cfg(windows)] diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs index 009675811..66c3f7bb6 100644 --- a/src/uu/mkfifo/src/mkfifo.rs +++ b/src/uu/mkfifo/src/mkfifo.rs @@ -11,7 +11,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use libc::mkfifo; use std::ffi::CString; -use uucore::InvalidEncodingHandling; +use uucore::{display::Quotable, InvalidEncodingHandling}; static NAME: &str = "mkfifo"; static USAGE: &str = "mkfifo [OPTION]... NAME..."; @@ -61,7 +61,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { mkfifo(name.as_ptr(), mode as libc::mode_t) }; if err == -1 { - show_error!("cannot create fifo '{}': File exists", f); + show_error!("cannot create fifo {}: File exists", f.quote()); exit_code = 1; } } diff --git a/src/uu/mknod/src/mknod.rs b/src/uu/mknod/src/mknod.rs index f8fb9c469..dd529c3ba 100644 --- a/src/uu/mknod/src/mknod.rs +++ b/src/uu/mknod/src/mknod.rs @@ -16,6 +16,7 @@ use clap::{crate_version, App, Arg, ArgMatches}; use libc::{dev_t, mode_t}; use libc::{S_IFBLK, S_IFCHR, S_IFIFO, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Create the special file NAME of the given TYPE."; @@ -219,7 +220,7 @@ fn valid_type(tpe: String) -> Result<(), String> { if vec!['b', 'c', 'u', 'p'].contains(&first_char) { Ok(()) } else { - Err(format!("invalid device type '{}'", tpe)) + Err(format!("invalid device type {}", tpe.quote())) } }) } diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 0b30f0087..59e82569a 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -12,6 +12,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +use uucore::display::{println_verbatim, Quotable}; use uucore::error::{FromIo, UError, UResult}; use std::env; @@ -57,16 +58,20 @@ impl Display for MkTempError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use MkTempError::*; match self { - PersistError(p) => write!(f, "could not persist file '{}'", p.display()), - MustEndInX(s) => write!(f, "with --suffix, template '{}' must end in X", s), - TooFewXs(s) => write!(f, "too few X's in template '{}'", s), + PersistError(p) => write!(f, "could not persist file {}", p.quote()), + MustEndInX(s) => write!(f, "with --suffix, template {} must end in X", s.quote()), + TooFewXs(s) => write!(f, "too few X's in template {}", s.quote()), ContainsDirSeparator(s) => { - write!(f, "invalid suffix '{}', contains directory separator", s) + write!( + f, + "invalid suffix {}, contains directory separator", + s.quote() + ) } InvalidTemplate(s) => write!( f, - "invalid template, '{}'; with --tmpdir, it may not be absolute", - s + "invalid template, {}; with --tmpdir, it may not be absolute", + s.quote() ), } } @@ -244,8 +249,7 @@ pub fn dry_exec(mut tmpdir: PathBuf, prefix: &str, rand: usize, suffix: &str) -> } } tmpdir.push(buf); - println!("{}", tmpdir.display()); - Ok(()) + println_verbatim(tmpdir).map_err_context(|| "failed to print directory name".to_owned()) } fn exec(dir: PathBuf, prefix: &str, rand: usize, suffix: &str, make_dir: bool) -> UResult<()> { @@ -274,6 +278,5 @@ fn exec(dir: PathBuf, prefix: &str, rand: usize, suffix: &str, make_dir: bool) - .map_err(|e| MkTempError::PersistError(e.file.path().to_path_buf()))? .1 }; - println!("{}", path.display()); - Ok(()) + println_verbatim(path).map_err_context(|| "failed to print directory name".to_owned()) } diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 8097d1402..3a601c1e8 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -30,6 +30,7 @@ use crossterm::{ use unicode_segmentation::UnicodeSegmentation; use unicode_width::UnicodeWidthStr; +use uucore::display::Quotable; const BELL: &str = "\x07"; @@ -64,12 +65,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let file = Path::new(file); if file.is_dir() { terminal::disable_raw_mode().unwrap(); - show_usage_error!("'{}' is a directory.", file.display()); + show_usage_error!("{} is a directory.", file.quote()); return 1; } if !file.exists() { terminal::disable_raw_mode().unwrap(); - show_error!("cannot open {}: No such file or directory", file.display()); + show_error!("cannot open {}: No such file or directory", file.quote()); return 1; } if length > 1 { diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index e2e2352a0..9d23f86de 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -21,6 +21,7 @@ use std::os::unix; use std::os::windows; use std::path::{Path, PathBuf}; use uucore::backup_control::{self, BackupMode}; +use uucore::display::Quotable; use fs_extra::dir::{move_dir, CopyOptions as DirCopyOptions}; @@ -223,10 +224,7 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { // `Ok()` results unless the source does not exist, or the user // lacks permission to access metadata. if source.symlink_metadata().is_err() { - show_error!( - "cannot stat '{}': No such file or directory", - source.display() - ); + show_error!("cannot stat {}: No such file or directory", source.quote()); return 1; } @@ -234,8 +232,8 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { if b.no_target_dir { if !source.is_dir() { show_error!( - "cannot overwrite directory '{}' with non-directory", - target.display() + "cannot overwrite directory {} with non-directory", + target.quote() ); return 1; } @@ -243,9 +241,9 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { return match rename(source, target, &b) { Err(e) => { show_error!( - "cannot move '{}' to '{}': {}", - source.display(), - target.display(), + "cannot move {} to {}: {}", + source.quote(), + target.quote(), e.to_string() ); 1 @@ -257,9 +255,9 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { return move_files_into_dir(&[source.clone()], target, &b); } else if target.exists() && source.is_dir() { show_error!( - "cannot overwrite non-directory '{}' with directory '{}'", - target.display(), - source.display() + "cannot overwrite non-directory {} with directory {}", + target.quote(), + source.quote() ); return 1; } @@ -272,9 +270,9 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { _ => { if b.no_target_dir { show_error!( - "mv: extra operand '{}'\n\ + "mv: extra operand {}\n\ Try '{} --help' for more information.", - files[2].display(), + files[2].quote(), uucore::execution_phrase() ); return 1; @@ -288,7 +286,7 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 { if !target_dir.is_dir() { - show_error!("target '{}' is not a directory", target_dir.display()); + show_error!("target {} is not a directory", target_dir.quote()); return 1; } @@ -298,8 +296,8 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3 Some(name) => target_dir.join(name), None => { show_error!( - "cannot stat '{}': No such file or directory", - sourcepath.display() + "cannot stat {}: No such file or directory", + sourcepath.quote() ); all_successful = false; @@ -309,9 +307,9 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3 if let Err(e) = rename(sourcepath, &targetpath, b) { show_error!( - "cannot move '{}' to '{}': {}", - sourcepath.display(), - targetpath.display(), + "cannot move {} to {}: {}", + sourcepath.quote(), + targetpath.quote(), e.to_string() ); all_successful = false; @@ -332,7 +330,7 @@ fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> { match b.overwrite { OverwriteMode::NoClobber => return Ok(()), OverwriteMode::Interactive => { - println!("{}: overwrite '{}'? ", uucore::util_name(), to.display()); + println!("{}: overwrite {}? ", uucore::util_name(), to.quote()); if !read_yes() { return Ok(()); } @@ -365,9 +363,9 @@ fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> { rename_with_fallback(from, to)?; if b.verbose { - print!("'{}' -> '{}'", from.display(), to.display()); + print!("{} -> {}", from.quote(), to.quote()); match backup_path { - Some(path) => println!(" (backup: '{}')", path.display()), + Some(path) => println!(" (backup: {})", path.quote()), None => println!(), } } diff --git a/src/uu/nohup/src/nohup.rs b/src/uu/nohup/src/nohup.rs index 1ecb9914f..d83170ae8 100644 --- a/src/uu/nohup/src/nohup.rs +++ b/src/uu/nohup/src/nohup.rs @@ -19,6 +19,7 @@ use std::fs::{File, OpenOptions}; use std::io::Error; use std::os::unix::prelude::*; use std::path::{Path, PathBuf}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Run COMMAND ignoring hangup signals."; @@ -122,13 +123,16 @@ fn find_stdout() -> File { .open(Path::new(NOHUP_OUT)) { Ok(t) => { - show_error!("ignoring input and appending output to '{}'", NOHUP_OUT); + show_error!( + "ignoring input and appending output to {}", + NOHUP_OUT.quote() + ); t } Err(e1) => { let home = match env::var("HOME") { Err(_) => { - show_error!("failed to open '{}': {}", NOHUP_OUT, e1); + show_error!("failed to open {}: {}", NOHUP_OUT.quote(), e1); exit!(internal_failure_code) } Ok(h) => h, @@ -143,12 +147,15 @@ fn find_stdout() -> File { .open(&homeout) { Ok(t) => { - show_error!("ignoring input and appending output to '{}'", homeout_str); + show_error!( + "ignoring input and appending output to {}", + homeout_str.quote() + ); t } Err(e2) => { - show_error!("failed to open '{}': {}", NOHUP_OUT, e1); - show_error!("failed to open '{}': {}", homeout_str, e2); + show_error!("failed to open {}: {}", NOHUP_OUT.quote(), e1); + show_error!("failed to open {}: {}", homeout_str.quote(), e2); exit!(internal_failure_code) } } diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index e44446818..bdee83e12 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -1,3 +1,5 @@ +use uucore::display::Quotable; + use crate::options::{NumfmtOptions, RoundMethod}; use crate::units::{DisplayableSuffix, RawSuffix, Result, Suffix, Unit, IEC_BASES, SI_BASES}; @@ -78,7 +80,7 @@ fn parse_suffix(s: &str) -> Result<(f64, Option)> { Some('Z') => Some((RawSuffix::Z, with_i)), Some('Y') => Some((RawSuffix::Y, with_i)), Some('0'..='9') => None, - _ => return Err(format!("invalid suffix in input: '{}'", s)), + _ => return Err(format!("invalid suffix in input: {}", s.quote())), }; let suffix_len = match suffix { @@ -89,7 +91,7 @@ fn parse_suffix(s: &str) -> Result<(f64, Option)> { let number = s[..s.len() - suffix_len] .parse::() - .map_err(|_| format!("invalid number: '{}'", s))?; + .map_err(|_| format!("invalid number: {}", s.quote()))?; Ok((number, suffix)) } diff --git a/src/uu/numfmt/src/numfmt.rs b/src/uu/numfmt/src/numfmt.rs index 1798975dc..da2fa8130 100644 --- a/src/uu/numfmt/src/numfmt.rs +++ b/src/uu/numfmt/src/numfmt.rs @@ -15,6 +15,7 @@ use crate::options::*; use crate::units::{Result, Unit}; use clap::{crate_version, App, AppSettings, Arg, ArgMatches}; use std::io::{BufRead, Write}; +use uucore::display::Quotable; use uucore::ranges::Range; pub mod format; @@ -113,7 +114,7 @@ fn parse_options(args: &ArgMatches) -> Result { 0 => Err(value), _ => Ok(n), }) - .map_err(|value| format!("invalid header value '{}'", value)) + .map_err(|value| format!("invalid header value {}", value.quote())) } }?; diff --git a/src/uu/od/src/multifilereader.rs b/src/uu/od/src/multifilereader.rs index 0f8616467..90796b2eb 100644 --- a/src/uu/od/src/multifilereader.rs +++ b/src/uu/od/src/multifilereader.rs @@ -5,6 +5,8 @@ use std::io; use std::io::BufReader; use std::vec::Vec; +use uucore::display::Quotable; + pub enum InputSource<'a> { FileName(&'a str), Stdin, @@ -57,7 +59,7 @@ impl<'b> MultifileReader<'b> { // print an error at the time that the file is needed, // then move on the the next file. // This matches the behavior of the original `od` - eprintln!("{}: '{}': {}", uucore::util_name(), fname, e); + eprintln!("{}: {}: {}", uucore::util_name(), fname.maybe_quote(), e); self.any_err = true } } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 6c1110362..e9983f991 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -43,6 +43,7 @@ use crate::partialreader::*; use crate::peekreader::*; use crate::prn_char::format_ascii_dump; use clap::{self, crate_version, AppSettings, Arg, ArgMatches}; +use uucore::display::Quotable; use uucore::parse_size::ParseSizeError; use uucore::InvalidEncodingHandling; @@ -635,7 +636,7 @@ fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String // GNU's od echos affected flag, -N or --read-bytes (-j or --skip-bytes, etc.), depending user's selection // GNU's od does distinguish between "invalid (suffix in) argument" match error { - ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), - ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), + ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } } diff --git a/src/uu/od/src/parse_formats.rs b/src/uu/od/src/parse_formats.rs index 80d477b27..301bb5154 100644 --- a/src/uu/od/src/parse_formats.rs +++ b/src/uu/od/src/parse_formats.rs @@ -1,5 +1,7 @@ // spell-checker:ignore formatteriteminfo docopt fvox fvoxw vals acdx +use uucore::display::Quotable; + use crate::formatteriteminfo::FormatterItemInfo; use crate::prn_char::*; use crate::prn_float::*; @@ -272,8 +274,9 @@ fn parse_type_string(params: &str) -> Result, Strin while let Some(type_char) = ch { let type_char = format_type(type_char).ok_or_else(|| { format!( - "unexpected char '{}' in format specification '{}'", - type_char, params + "unexpected char '{}' in format specification {}", + type_char, + params.quote() ) })?; @@ -293,8 +296,9 @@ fn parse_type_string(params: &str) -> Result, Strin if !decimal_size.is_empty() { byte_size = decimal_size.parse().map_err(|_| { format!( - "invalid number '{}' in format specification '{}'", - decimal_size, params + "invalid number {} in format specification {}", + decimal_size.quote(), + params.quote() ) })?; } @@ -305,8 +309,9 @@ fn parse_type_string(params: &str) -> Result, Strin let ft = od_format_type(type_char, byte_size).ok_or_else(|| { format!( - "invalid size '{}' in format specification '{}'", - byte_size, params + "invalid size '{}' in format specification {}", + byte_size, + params.quote() ) })?; formats.push(ParsedFormatterItemInfo::new(ft, show_ascii_dump)); diff --git a/src/uu/pathchk/src/pathchk.rs b/src/uu/pathchk/src/pathchk.rs index 863bb6bf2..8afeaff18 100644 --- a/src/uu/pathchk/src/pathchk.rs +++ b/src/uu/pathchk/src/pathchk.rs @@ -15,6 +15,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use std::fs; use std::io::{ErrorKind, Write}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; // operating mode @@ -153,10 +154,10 @@ fn check_basic(path: &[String]) -> bool { if component_len > POSIX_NAME_MAX { writeln!( &mut std::io::stderr(), - "limit {} exceeded by length {} of file name component '{}'", + "limit {} exceeded by length {} of file name component {}", POSIX_NAME_MAX, component_len, - p + p.quote() ); return false; } @@ -175,8 +176,8 @@ fn check_extra(path: &[String]) -> bool { if p.starts_with('-') { writeln!( &mut std::io::stderr(), - "leading hyphen in file name component '{}'", - p + "leading hyphen in file name component {}", + p.quote() ); return false; } @@ -197,10 +198,10 @@ fn check_default(path: &[String]) -> bool { if total_len > libc::PATH_MAX as usize { writeln!( &mut std::io::stderr(), - "limit {} exceeded by length {} of file name '{}'", + "limit {} exceeded by length {} of file name {}", libc::PATH_MAX, total_len, - joined_path + joined_path.quote() ); return false; } @@ -210,10 +211,10 @@ fn check_default(path: &[String]) -> bool { if component_len > libc::FILENAME_MAX as usize { writeln!( &mut std::io::stderr(), - "limit {} exceeded by length {} of file name component '{}'", + "limit {} exceeded by length {} of file name component {}", libc::FILENAME_MAX, component_len, - p + p.quote() ); return false; } @@ -246,9 +247,9 @@ fn check_portable_chars(path_segment: &str) -> bool { let invalid = path_segment[i..].chars().next().unwrap(); writeln!( &mut std::io::stderr(), - "nonportable character '{}' in file name component '{}'", + "nonportable character '{}' in file name component {}", invalid, - path_segment + path_segment.quote() ); return false; } diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 1358cef6c..45d9480a7 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -24,6 +24,8 @@ use std::io::{stdin, stdout, BufRead, BufReader, Lines, Read, Stdout, Write}; #[cfg(unix)] use std::os::unix::fs::FileTypeExt; +use uucore::display::Quotable; + type IOError = std::io::Error; const NAME: &str = "pr"; @@ -517,7 +519,7 @@ fn parse_usize(matches: &Matches, opt: &str) -> Option> { let i = value_to_parse.0; let option = value_to_parse.1; i.parse().map_err(|_e| { - PrError::EncounteredErrors(format!("invalid {} argument '{}'", option, i)) + PrError::EncounteredErrors(format!("invalid {} argument {}", option, i.quote())) }) }; matches @@ -619,7 +621,7 @@ fn build_options( let unparsed_num = i.get(1).unwrap().as_str().trim(); let x: Vec<_> = unparsed_num.split(':').collect(); x[0].to_string().parse::().map_err(|_e| { - PrError::EncounteredErrors(format!("invalid {} argument '{}'", "+", unparsed_num)) + PrError::EncounteredErrors(format!("invalid {} argument {}", "+", unparsed_num.quote())) }) }) { Some(res) => res?, @@ -633,7 +635,11 @@ fn build_options( .map(|unparsed_num| { let x: Vec<_> = unparsed_num.split(':').collect(); x[1].to_string().parse::().map_err(|_e| { - PrError::EncounteredErrors(format!("invalid {} argument '{}'", "+", unparsed_num)) + PrError::EncounteredErrors(format!( + "invalid {} argument {}", + "+", + unparsed_num.quote() + )) }) }) { Some(res) => Some(res?), @@ -643,7 +649,10 @@ fn build_options( let invalid_pages_map = |i: String| { let unparsed_value = matches.opt_str(options::PAGE_RANGE_OPTION).unwrap(); i.parse::().map_err(|_e| { - PrError::EncounteredErrors(format!("invalid --pages argument '{}'", unparsed_value)) + PrError::EncounteredErrors(format!( + "invalid --pages argument {}", + unparsed_value.quote() + )) }) }; @@ -741,7 +750,7 @@ fn build_options( let start_column_option = match re_col.captures(&free_args).map(|i| { let unparsed_num = i.get(1).unwrap().as_str().trim(); unparsed_num.parse::().map_err(|_e| { - PrError::EncounteredErrors(format!("invalid {} argument '{}'", "-", unparsed_num)) + PrError::EncounteredErrors(format!("invalid {} argument {}", "-", unparsed_num.quote())) }) }) { Some(res) => Some(res?), diff --git a/src/uu/printf/src/cli.rs b/src/uu/printf/src/cli.rs index a5e9c9775..fa2fda1d2 100644 --- a/src/uu/printf/src/cli.rs +++ b/src/uu/printf/src/cli.rs @@ -2,20 +2,11 @@ // spell-checker:ignore (ToDO) bslice -use std::env; -use std::io::{stderr, stdout, Write}; +use std::io::{stdout, Write}; pub const EXIT_OK: i32 = 0; pub const EXIT_ERR: i32 = 1; -pub fn err_msg(msg: &str) { - let exe_path = match env::current_exe() { - Ok(p) => p.to_string_lossy().into_owned(), - _ => String::from(""), - }; - writeln!(&mut stderr(), "{}: {}", exe_path, msg).unwrap(); -} - // by default stdout only flushes // to console when a newline is passed. pub fn flush_char(c: char) { diff --git a/src/uu/printf/src/memo.rs b/src/uu/printf/src/memo.rs index 2f12f9192..f5d41aeb6 100644 --- a/src/uu/printf/src/memo.rs +++ b/src/uu/printf/src/memo.rs @@ -8,8 +8,9 @@ use itertools::put_back_n; use std::iter::Peekable; use std::slice::Iter; +use uucore::display::Quotable; +use uucore::show_error; -use crate::cli; use crate::tokenize::sub::Sub; use crate::tokenize::token::{Token, Tokenizer}; use crate::tokenize::unescaped_text::UnescapedText; @@ -19,10 +20,10 @@ pub struct Memo { } fn warn_excess_args(first_arg: &str) { - cli::err_msg(&format!( - "warning: ignoring excess arguments, starting with '{}'", - first_arg - )); + show_error!( + "warning: ignoring excess arguments, starting with {}", + first_arg.quote() + ); } impl Memo { diff --git a/src/uu/printf/src/tokenize/num_format/formatter.rs b/src/uu/printf/src/tokenize/num_format/formatter.rs index f5f5d71b1..0438f78bf 100644 --- a/src/uu/printf/src/tokenize/num_format/formatter.rs +++ b/src/uu/printf/src/tokenize/num_format/formatter.rs @@ -3,11 +3,10 @@ use itertools::{put_back_n, PutBackN}; use std::str::Chars; +use uucore::{display::Quotable, show_error}; use super::format_field::FormatField; -use crate::cli; - // contains the rough ingredients to final // output for a number, organized together // to allow for easy generalization of output manipulation @@ -66,5 +65,5 @@ pub fn get_it_at(offset: usize, str_in: &str) -> PutBackN { // TODO: put this somewhere better pub fn warn_incomplete_conv(pf_arg: &str) { // important: keep println here not print - cli::err_msg(&format!("{}: value not completely converted", pf_arg)) + show_error!("{}: value not completely converted", pf_arg.maybe_quote()); } diff --git a/src/uu/printf/src/tokenize/num_format/num_format.rs b/src/uu/printf/src/tokenize/num_format/num_format.rs index b32731f2d..74666ad8e 100644 --- a/src/uu/printf/src/tokenize/num_format/num_format.rs +++ b/src/uu/printf/src/tokenize/num_format/num_format.rs @@ -7,6 +7,9 @@ use std::env; use std::vec::Vec; +use uucore::display::Quotable; +use uucore::show_error; + use super::format_field::{FieldType, FormatField}; use super::formatter::{Base, FormatPrimitive, Formatter, InitialPrefix}; use super::formatters::cninetyninehexfloatf::CninetyNineHexFloatf; @@ -15,11 +18,9 @@ use super::formatters::floatf::Floatf; use super::formatters::intf::Intf; use super::formatters::scif::Scif; -use crate::cli; - pub fn warn_expected_numeric(pf_arg: &str) { // important: keep println here not print - cli::err_msg(&format!("{}: expected a numeric value", pf_arg)); + show_error!("{}: expected a numeric value", pf_arg.maybe_quote()); } // when character constant arguments have excess characters @@ -29,11 +30,11 @@ fn warn_char_constant_ign(remaining_bytes: Vec) { Ok(_) => {} Err(e) => { if let env::VarError::NotPresent = e { - cli::err_msg(&format!( + show_error!( "warning: {:?}: character(s) following character \ constant have been ignored", &*remaining_bytes - )); + ); } } } diff --git a/src/uu/printf/src/tokenize/sub.rs b/src/uu/printf/src/tokenize/sub.rs index d01e00699..48d854fab 100644 --- a/src/uu/printf/src/tokenize/sub.rs +++ b/src/uu/printf/src/tokenize/sub.rs @@ -10,6 +10,7 @@ use std::iter::Peekable; use std::process::exit; use std::slice::Iter; use std::str::Chars; +use uucore::show_error; // use std::collections::HashSet; use super::num_format::format_field::{FieldType, FormatField}; @@ -19,7 +20,7 @@ use super::unescaped_text::UnescapedText; use crate::cli; fn err_conv(sofar: &str) { - cli::err_msg(&format!("%{}: invalid conversion specification", sofar)); + show_error!("%{}: invalid conversion specification", sofar); exit(cli::EXIT_ERR); } diff --git a/src/uu/ptx/src/ptx.rs b/src/uu/ptx/src/ptx.rs index 264a37d72..3619b8d4d 100644 --- a/src/uu/ptx/src/ptx.rs +++ b/src/uu/ptx/src/ptx.rs @@ -17,6 +17,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::default::Default; use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static NAME: &str = "ptx"; @@ -292,7 +293,11 @@ fn create_word_set(config: &Config, filter: &WordFilter, file_map: &FileMap) -> fn get_reference(config: &Config, word_ref: &WordRef, line: &str, context_reg: &Regex) -> String { if config.auto_ref { - format!("{}:{}", word_ref.filename, word_ref.local_line_nr + 1) + format!( + "{}:{}", + word_ref.filename.maybe_quote(), + word_ref.local_line_nr + 1 + ) } else if config.input_ref { let (beg, end) = match context_reg.find(line) { Some(x) => (x.start(), x.end()), diff --git a/src/uu/readlink/src/readlink.rs b/src/uu/readlink/src/readlink.rs index 1c62d8278..d6dd1634a 100644 --- a/src/uu/readlink/src/readlink.rs +++ b/src/uu/readlink/src/readlink.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use std::fs; use std::io::{stdout, Write}; use std::path::{Path, PathBuf}; +use uucore::display::Quotable; use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; const ABOUT: &str = "Print value of a symbolic link or canonical file name."; @@ -71,10 +72,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } if no_newline && files.len() > 1 && !silent { - eprintln!( - "{}: ignoring --no-newline with multiple arguments", - uucore::util_name() - ); + show_error!("ignoring --no-newline with multiple arguments"); no_newline = false; } @@ -85,12 +83,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(path) => show(&path, no_newline, use_zero), Err(err) => { if verbose { - eprintln!( - "{}: {}: errno {}", - uucore::util_name(), - f, - err.raw_os_error().unwrap() - ); + show_error!("{}: errno {}", f.maybe_quote(), err.raw_os_error().unwrap()); } return 1; } @@ -100,12 +93,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(path) => show(&path, no_newline, use_zero), Err(err) => { if verbose { - eprintln!( - "{}: {}: errno {:?}", - uucore::util_name(), - f, - err.raw_os_error().unwrap() - ); + show_error!("{}: errno {}", f.maybe_quote(), err.raw_os_error().unwrap()); } return 1; } diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index 451253d50..d13aed6c7 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -11,8 +11,14 @@ extern crate uucore; use clap::{crate_version, App, Arg}; -use std::path::{Path, PathBuf}; -use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; +use std::{ + io::{stdout, Write}, + path::{Path, PathBuf}, +}; +use uucore::{ + display::{print_verbatim, Quotable}, + fs::{canonicalize, MissingHandling, ResolveMode}, +}; static ABOUT: &str = "print the resolved path"; @@ -58,7 +64,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for path in &paths { if let Err(e) = resolve_path(path, strip, zero, logical, can_mode) { if !quiet { - show_error!("{}: {}", e, path.display()); + show_error!("{}: {}", path.maybe_quote(), e); } retcode = 1 }; @@ -154,8 +160,9 @@ fn resolve_path( ResolveMode::Physical }; let abs = canonicalize(p, can_mode, resolve)?; - let line_ending = if zero { '\0' } else { '\n' }; + let line_ending = if zero { b'\0' } else { b'\n' }; - print!("{}{}", abs.display(), line_ending); + print_verbatim(&abs)?; + stdout().write_all(&[line_ending])?; Ok(()) } diff --git a/src/uu/relpath/src/relpath.rs b/src/uu/relpath/src/relpath.rs index 95f03e423..16b920861 100644 --- a/src/uu/relpath/src/relpath.rs +++ b/src/uu/relpath/src/relpath.rs @@ -10,6 +10,7 @@ use clap::{crate_version, App, Arg}; use std::env; use std::path::{Path, PathBuf}; +use uucore::display::println_verbatim; use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; use uucore::InvalidEncodingHandling; @@ -48,7 +49,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !absto.as_path().starts_with(absbase.as_path()) || !absfrom.as_path().starts_with(absbase.as_path()) { - println!("{}", absto.display()); + println_verbatim(absto).unwrap(); return 0; } } @@ -74,7 +75,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .map(|x| result.push(x.as_os_str())) .last(); - println!("{}", result.display()); + println_verbatim(result).unwrap(); 0 } diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 0613ff857..54fce52ff 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -17,6 +17,7 @@ use std::fs; use std::io::{stderr, stdin, BufRead, Write}; use std::ops::BitOr; use std::path::{Path, PathBuf}; +use uucore::display::Quotable; use walkdir::{DirEntry, WalkDir}; #[derive(Eq, PartialEq, Clone, Copy)] @@ -236,7 +237,10 @@ fn remove(files: Vec, options: Options) -> bool { // (e.g., permission), even rm -f should fail with // outputting the error, but there's no easy eay. if !options.force { - show_error!("cannot remove '{}': No such file or directory", filename); + show_error!( + "cannot remove {}: No such file or directory", + filename.quote() + ); true } else { false @@ -263,13 +267,9 @@ fn handle_dir(path: &Path, options: &Options) -> bool { // GNU compatibility (rm/fail-eacces.sh) // here, GNU doesn't use some kind of remove_dir_all // It will show directory+file - show_error!( - "cannot remove '{}': {}", - path.display(), - "Permission denied" - ); + show_error!("cannot remove {}: {}", path.quote(), "Permission denied"); } else { - show_error!("cannot remove '{}': {}", path.display(), e); + show_error!("cannot remove {}: {}", path.quote(), e); } } } else { @@ -287,7 +287,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool { } Err(e) => { had_err = true; - show_error!("recursing in '{}': {}", path.display(), e); + show_error!("recursing in {}: {}", path.quote(), e); } } } @@ -299,12 +299,12 @@ fn handle_dir(path: &Path, options: &Options) -> bool { } else if options.dir && (!is_root || !options.preserve_root) { had_err = remove_dir(path, options).bitor(had_err); } else if options.recursive { - show_error!("could not remove directory '{}'", path.display()); + show_error!("could not remove directory {}", path.quote()); had_err = true; } else { show_error!( - "cannot remove '{}': Is a directory", // GNU's rm error message does not include help - path.display() + "cannot remove {}: Is a directory", // GNU's rm error message does not include help + path.quote() ); had_err = true; } @@ -325,36 +325,36 @@ fn remove_dir(path: &Path, options: &Options) -> bool { match fs::remove_dir(path) { Ok(_) => { if options.verbose { - println!("removed directory '{}'", normalize(path).display()); + println!("removed directory {}", normalize(path).quote()); } } Err(e) => { if e.kind() == std::io::ErrorKind::PermissionDenied { // GNU compatibility (rm/fail-eacces.sh) show_error!( - "cannot remove '{}': {}", - path.display(), + "cannot remove {}: {}", + path.quote(), "Permission denied" ); } else { - show_error!("cannot remove '{}': {}", path.display(), e); + show_error!("cannot remove {}: {}", path.quote(), e); } return true; } } } else { // directory can be read but is not empty - show_error!("cannot remove '{}': Directory not empty", path.display()); + show_error!("cannot remove {}: Directory not empty", path.quote()); return true; } } else { // called to remove a symlink_dir (windows) without "-r"/"-R" or "-d" - show_error!("cannot remove '{}': Is a directory", path.display()); + show_error!("cannot remove {}: Is a directory", path.quote()); return true; } } else { // GNU's rm shows this message if directory is empty but not readable - show_error!("cannot remove '{}': Directory not empty", path.display()); + show_error!("cannot remove {}: Directory not empty", path.quote()); return true; } } @@ -372,19 +372,15 @@ fn remove_file(path: &Path, options: &Options) -> bool { match fs::remove_file(path) { Ok(_) => { if options.verbose { - println!("removed '{}'", normalize(path).display()); + println!("removed {}", normalize(path).quote()); } } Err(e) => { if e.kind() == std::io::ErrorKind::PermissionDenied { // GNU compatibility (rm/fail-eacces.sh) - show_error!( - "cannot remove '{}': {}", - path.display(), - "Permission denied" - ); + show_error!("cannot remove {}: {}", path.quote(), "Permission denied"); } else { - show_error!("cannot remove '{}': {}", path.display(), e); + show_error!("cannot remove {}: {}", path.quote(), e); } return true; } @@ -396,9 +392,9 @@ fn remove_file(path: &Path, options: &Options) -> bool { fn prompt_file(path: &Path, is_dir: bool) -> bool { if is_dir { - prompt(&(format!("rm: remove directory '{}'? ", path.display()))) + prompt(&(format!("rm: remove directory {}? ", path.quote()))) } else { - prompt(&(format!("rm: remove file '{}'? ", path.display()))) + prompt(&(format!("rm: remove file {}? ", path.quote()))) } } diff --git a/src/uu/runcon/src/errors.rs b/src/uu/runcon/src/errors.rs index bc10a2f3e..5f8258de0 100644 --- a/src/uu/runcon/src/errors.rs +++ b/src/uu/runcon/src/errors.rs @@ -3,6 +3,8 @@ use std::fmt::Write; use std::io; use std::str::Utf8Error; +use uucore::display::Quotable; + pub(crate) type Result = std::result::Result; #[derive(thiserror::Error, Debug)] @@ -31,7 +33,7 @@ pub(crate) enum Error { source: io::Error, }, - #[error("{operation} failed on '{}'", .operand1.to_string_lossy())] + #[error("{operation} failed on {}", .operand1.quote())] Io1 { operation: &'static str, operand1: OsString, diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 730c5efc7..501b12ac0 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -14,6 +14,7 @@ use num_traits::{Num, ToPrimitive}; use std::cmp; use std::io::{stdout, Write}; use std::str::FromStr; +use uucore::display::Quotable; static ABOUT: &str = "Display numbers from FIRST to LAST, in steps of INCREMENT."; static OPT_SEPARATOR: &str = "separator"; @@ -115,14 +116,14 @@ impl FromStr for Number { } Err(_) => match s.parse::() { Ok(value) if value.is_nan() => Err(format!( - "invalid 'not-a-number' argument: '{}'\nTry '{} --help' for more information.", - s, + "invalid 'not-a-number' argument: {}\nTry '{} --help' for more information.", + s.quote(), uucore::execution_phrase(), )), Ok(value) => Ok(Number::F64(value)), Err(_) => Err(format!( - "invalid floating point argument: '{}'\nTry '{} --help' for more information.", - s, + "invalid floating point argument: {}\nTry '{} --help' for more information.", + s.quote(), uucore::execution_phrase(), )), }, diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 0464c5d50..fa455f027 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -18,12 +18,12 @@ use std::io; use std::io::prelude::*; use std::io::SeekFrom; use std::path::{Path, PathBuf}; -use uucore::InvalidEncodingHandling; +use uucore::display::Quotable; +use uucore::{util_name, InvalidEncodingHandling}; #[macro_use] extern crate uucore; -static NAME: &str = "shred"; const BLOCK_SIZE: usize = 512; const NAME_CHARSET: &[u8] = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_."; @@ -281,7 +281,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !matches.is_present(options::FILE) { show_error!("Missing an argument"); - show_error!("For help, try '{} --help'", NAME); + show_error!("For help, try '{} --help'", uucore::execution_phrase()); return 0; } @@ -289,7 +289,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Some(s) => match s.parse::() { Ok(u) => u, Err(_) => { - errs.push(format!("invalid number of passes: '{}'", s)); + errs.push(format!("invalid number of passes: {}", s.quote())); 0 } }, @@ -414,7 +414,11 @@ fn get_size(size_str_opt: Option) -> Option { let coefficient = match size_str.parse::() { Ok(u) => u, Err(_) => { - println!("{}: {}: Invalid file size", NAME, size_str_opt.unwrap()); + println!( + "{}: {}: Invalid file size", + util_name(), + size_str_opt.unwrap().maybe_quote() + ); exit!(1); } }; @@ -452,11 +456,11 @@ fn wipe_file( // Get these potential errors out of the way first let path: &Path = Path::new(path_str); if !path.exists() { - show_error!("{}: No such file or directory", path.display()); + show_error!("{}: No such file or directory", path.maybe_quote()); return; } if !path.is_file() { - show_error!("{}: Not a file", path.display()); + show_error!("{}: Not a file", path.maybe_quote()); return; } @@ -520,7 +524,7 @@ fn wipe_file( let mut file: File = match OpenOptions::new().write(true).truncate(false).open(path) { Ok(f) => f, Err(e) => { - show_error!("{}: failed to open for writing: {}", path.display(), e); + show_error!("{}: failed to open for writing: {}", path.maybe_quote(), e); return; } }; @@ -535,8 +539,8 @@ fn wipe_file( if total_passes.to_string().len() == 1 { println!( "{}: {}: pass {}/{} ({})... ", - NAME, - path.display(), + util_name(), + path.maybe_quote(), i + 1, total_passes, pass_name @@ -544,8 +548,8 @@ fn wipe_file( } else { println!( "{}: {}: pass {:2.0}/{:2.0} ({})... ", - NAME, - path.display(), + util_name(), + path.maybe_quote(), i + 1, total_passes, pass_name @@ -556,7 +560,7 @@ fn wipe_file( match do_pass(&mut file, path, &mut generator, *pass_type, size) { Ok(_) => {} Err(e) => { - show_error!("{}: File write pass failed: {}", path.display(), e); + show_error!("{}: File write pass failed: {}", path.maybe_quote(), e); } } // Ignore failed writes; just keep trying @@ -567,7 +571,7 @@ fn wipe_file( match do_remove(path, path_str, verbose) { Ok(_) => {} Err(e) => { - show_error!("{}: failed to remove file: {}", path.display(), e); + show_error!("{}: failed to remove file: {}", path.maybe_quote(), e); } } } @@ -622,9 +626,9 @@ fn wipe_name(orig_path: &Path, verbose: bool) -> Option { if verbose { println!( "{}: {}: renamed to {}", - NAME, - last_path.display(), - new_path.display() + util_name(), + last_path.maybe_quote(), + new_path.quote() ); } @@ -641,9 +645,9 @@ fn wipe_name(orig_path: &Path, verbose: bool) -> Option { Err(e) => { println!( "{}: {}: Couldn't rename to {}: {}", - NAME, - last_path.display(), - new_path.display(), + util_name(), + last_path.maybe_quote(), + new_path.quote(), e ); return None; @@ -657,7 +661,7 @@ fn wipe_name(orig_path: &Path, verbose: bool) -> Option { fn do_remove(path: &Path, orig_filename: &str, verbose: bool) -> Result<(), io::Error> { if verbose { - println!("{}: {}: removing", NAME, orig_filename); + println!("{}: {}: removing", util_name(), orig_filename.maybe_quote()); } let renamed_path: Option = wipe_name(path, verbose); @@ -666,7 +670,7 @@ fn do_remove(path: &Path, orig_filename: &str, verbose: bool) -> Result<(), io:: } if verbose { - println!("{}: {}: removed", NAME, orig_filename); + println!("{}: {}: removed", util_name(), orig_filename.maybe_quote()); } Ok(()) diff --git a/src/uu/shuf/src/shuf.rs b/src/uu/shuf/src/shuf.rs index 7125014b1..9a899d746 100644 --- a/src/uu/shuf/src/shuf.rs +++ b/src/uu/shuf/src/shuf.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use rand::Rng; use std::fs::File; use std::io::{stdin, stdout, BufReader, BufWriter, Read, Write}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; enum Mode { @@ -76,7 +77,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Some(count) => match count.parse::() { Ok(val) => val, Err(_) => { - show_error!("invalid line count: '{}'", count); + show_error!("invalid line count: {}", count.quote()); return 1; } }, @@ -185,13 +186,13 @@ fn read_input_file(filename: &str) -> Vec { } else { match File::open(filename) { Ok(f) => Box::new(f) as Box, - Err(e) => crash!(1, "failed to open '{}': {}", filename, e), + Err(e) => crash!(1, "failed to open {}: {}", filename.quote(), e), } }); let mut data = Vec::new(); if let Err(e) = file.read_to_end(&mut data) { - crash!(1, "failed reading '{}': {}", filename, e) + crash!(1, "failed reading {}: {}", filename.quote(), e) }; data @@ -235,7 +236,7 @@ fn shuf_bytes(input: &mut Vec<&[u8]>, opts: Options) { None => Box::new(stdout()) as Box, Some(s) => match File::create(&s[..]) { Ok(f) => Box::new(f) as Box, - Err(e) => crash!(1, "failed to open '{}' for writing: {}", &s[..], e), + Err(e) => crash!(1, "failed to open {} for writing: {}", s.quote(), e), }, }); @@ -243,7 +244,7 @@ fn shuf_bytes(input: &mut Vec<&[u8]>, opts: Options) { Some(r) => WrappedRng::RngFile(rand::rngs::adapter::ReadRng::new( match File::open(&r[..]) { Ok(f) => f, - Err(e) => crash!(1, "failed to open random source '{}': {}", &r[..], e), + Err(e) => crash!(1, "failed to open random source {}: {}", r.quote(), e), }, )), None => WrappedRng::RngDefault(rand::thread_rng()), @@ -288,14 +289,14 @@ fn shuf_bytes(input: &mut Vec<&[u8]>, opts: Options) { fn parse_range(input_range: &str) -> Result<(usize, usize), String> { let split: Vec<&str> = input_range.split('-').collect(); if split.len() != 2 { - Err(format!("invalid input range: '{}'", input_range)) + Err(format!("invalid input range: {}", input_range.quote())) } else { let begin = split[0] .parse::() - .map_err(|_| format!("invalid input range: '{}'", split[0]))?; + .map_err(|_| format!("invalid input range: {}", split[0].quote()))?; let end = split[1] .parse::() - .map_err(|_| format!("invalid input range: '{}'", split[1]))?; + .map_err(|_| format!("invalid input range: {}", split[1].quote()))?; Ok((begin, end + 1)) } } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index abba1e63b..bab6dd770 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -45,6 +45,7 @@ use std::path::Path; use std::path::PathBuf; use std::str::Utf8Error; use unicode_width::UnicodeWidthStr; +use uucore::display::Quotable; use uucore::error::{set_exit_code, strip_errno, UError, UResult, USimpleError, UUsageError}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::version_cmp::version_cmp; @@ -139,7 +140,7 @@ enum SortError { error: std::io::Error, }, ReadFailed { - path: String, + path: PathBuf, error: std::io::Error, }, ParseKeyError { @@ -189,7 +190,7 @@ impl Display for SortError { write!( f, "{}:{}: disorder: {}", - file.to_string_lossy(), + file.maybe_quote(), line_number, line ) @@ -198,13 +199,23 @@ impl Display for SortError { } } SortError::OpenFailed { path, error } => { - write!(f, "open failed: {}: {}", path, strip_errno(error)) + write!( + f, + "open failed: {}: {}", + path.maybe_quote(), + strip_errno(error) + ) } SortError::ParseKeyError { key, msg } => { - write!(f, "failed to parse key `{}`: {}", key, msg) + write!(f, "failed to parse key {}: {}", key.quote(), msg) } SortError::ReadFailed { path, error } => { - write!(f, "cannot read: {}: {}", path, strip_errno(error)) + write!( + f, + "cannot read: {}: {}", + path.maybe_quote(), + strip_errno(error) + ) } SortError::OpenTmpFileFailed { error } => { write!(f, "failed to open temporary file: {}", strip_errno(error)) @@ -213,7 +224,7 @@ impl Display for SortError { write!(f, "couldn't execute compress program: errno {}", code) } SortError::CompressProgTerminatedAbnormally { prog } => { - write!(f, "'{}' terminated abnormally", prog) + write!(f, "{} terminated abnormally", prog.quote()) } SortError::TmpDirCreationFailed => write!(f, "could not create temporary directory"), SortError::Uft8Error { error } => write!(f, "{}", error), @@ -1179,7 +1190,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if let Some(n_merge) = matches.value_of(options::BATCH_SIZE) { settings.merge_batch_size = n_merge.parse().map_err(|_| { - UUsageError::new(2, format!("invalid --batch-size argument '{}'", n_merge)) + UUsageError::new( + 2, + format!("invalid --batch-size argument {}", n_merge.quote()), + ) })?; } @@ -1211,23 +1225,30 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else if settings.check && files.len() != 1 { return Err(UUsageError::new( 2, - format!( - "extra operand `{}' not allowed with -c", - files[1].to_string_lossy() - ), + format!("extra operand {} not allowed with -c", files[1].quote()), )); } if let Some(arg) = matches.args.get(options::SEPARATOR) { - let separator = arg.vals[0].to_string_lossy(); - let mut separator = separator.as_ref(); + let mut separator = arg.vals[0].to_str().ok_or_else(|| { + UUsageError::new( + 2, + format!("separator is not valid unicode: {}", arg.vals[0].quote()), + ) + })?; if separator == "\\0" { separator = "\0"; } + // This rejects non-ASCII codepoints, but perhaps we don't have to. + // On the other hand GNU accepts any single byte, valid unicode or not. + // (Supporting multi-byte chars would require changes in tokenize_with_separator().) if separator.len() != 1 { return Err(UUsageError::new( 2, - "separator must be exactly one character long", + format!( + "separator must be exactly one character long: {}", + separator.quote() + ), )); } settings.separator = Some(separator.chars().next().unwrap()) @@ -1816,7 +1837,7 @@ fn open(path: impl AsRef) -> UResult> { match File::open(path) { Ok(f) => Ok(Box::new(f) as Box), Err(error) => Err(SortError::ReadFailed { - path: path.to_string_lossy().to_string(), + path: path.to_owned(), error, } .into()), @@ -1828,8 +1849,8 @@ fn format_error_message(error: ParseSizeError, s: &str, option: &str) -> String // GNU's sort echos affected flag, -S or --buffer-size, depending user's selection // GNU's sort does distinguish between "invalid (suffix in) argument" match error { - ParseSizeError::ParseFailure(_) => format!("invalid --{} argument '{}'", option, s), - ParseSizeError::SizeTooBig(_) => format!("--{} argument '{}' too large", option, s), + ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), + ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } } diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index e405c757a..581b632d2 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -19,6 +19,7 @@ use std::fs::File; use std::io::{stdin, BufRead, BufReader, BufWriter, Read, Write}; use std::path::Path; use std::{char, fs::remove_file}; +use uucore::display::Quotable; use uucore::parse_size::parse_size; static OPT_BYTES: &str = "bytes"; @@ -238,7 +239,11 @@ impl LineSplitter { fn new(settings: &Settings) -> LineSplitter { LineSplitter { lines_per_split: settings.strategy_param.parse().unwrap_or_else(|_| { - crash!(1, "invalid number of lines: '{}'", settings.strategy_param) + crash!( + 1, + "invalid number of lines: {}", + settings.strategy_param.quote() + ) }), } } @@ -373,8 +378,8 @@ fn split(settings: &Settings) -> i32 { let r = File::open(Path::new(&settings.input)).unwrap_or_else(|_| { crash!( 1, - "cannot open '{}' for reading: No such file or directory", - settings.input + "cannot open {} for reading: No such file or directory", + settings.input.quote() ) }); Box::new(r) as Box @@ -383,7 +388,7 @@ fn split(settings: &Settings) -> i32 { let mut splitter: Box = match settings.strategy.as_str() { s if s == OPT_LINES => Box::new(LineSplitter::new(settings)), s if (s == OPT_BYTES || s == OPT_LINE_BYTES) => Box::new(ByteSplitter::new(settings)), - a => crash!(1, "strategy {} not supported", a), + a => crash!(1, "strategy {} not supported", a.quote()), }; let mut fileno = 0; diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index d017999b4..fd4a6443d 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -7,6 +7,7 @@ #[macro_use] extern crate uucore; +use uucore::display::Quotable; use uucore::entries; use uucore::fs::display_permissions; use uucore::fsext::{ @@ -24,7 +25,7 @@ use std::{cmp, fs, iter}; macro_rules! check_bound { ($str: ident, $bound:expr, $beg: expr, $end: expr) => { if $end >= $bound { - return Err(format!("'{}': invalid directive", &$str[$beg..$end])); + return Err(format!("{}: invalid directive", $str[$beg..$end].quote())); } }; } @@ -652,11 +653,7 @@ impl Stater { return 1; } }; - arg = format!( - "`{}' -> `{}'", - file, - dst.to_string_lossy() - ); + arg = format!("{} -> {}", file.quote(), dst.quote()); } else { arg = file.to_string(); } @@ -750,7 +747,7 @@ impl Stater { } } Err(e) => { - show_error!("cannot stat '{}': {}", file, e); + show_error!("cannot stat {}: {}", file.quote(), e); return 1; } } @@ -843,7 +840,11 @@ impl Stater { } } Err(e) => { - show_error!("cannot read file system information for '{}': {}", file, e); + show_error!( + "cannot read file system information for {}: {}", + file.quote(), + e + ); return 1; } } diff --git a/src/uu/sum/src/sum.rs b/src/uu/sum/src/sum.rs index f1147ca2e..15c20f09d 100644 --- a/src/uu/sum/src/sum.rs +++ b/src/uu/sum/src/sum.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use std::fs::File; use std::io::{stdin, Read, Result}; use std::path::Path; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static NAME: &str = "sum"; @@ -118,7 +119,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let reader = match open(file) { Ok(f) => f, Err(error) => { - show_error!("'{}' {}", file, error); + show_error!("{}: {}", file.maybe_quote(), error); exit_code = 2; continue; } diff --git a/src/uu/sync/src/sync.rs b/src/uu/sync/src/sync.rs index f49f51728..d6a21f280 100644 --- a/src/uu/sync/src/sync.rs +++ b/src/uu/sync/src/sync.rs @@ -14,6 +14,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use std::path::Path; +use uucore::display::Quotable; static EXIT_ERR: i32 = 1; @@ -175,7 +176,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for f in &files { if !Path::new(&f).exists() { - crash!(EXIT_ERR, "cannot stat '{}': No such file or directory", f); + crash!( + EXIT_ERR, + "cannot stat {}: No such file or directory", + f.quote() + ); } } diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 0568f1601..e54697f2b 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -14,6 +14,7 @@ use clap::{crate_version, App, Arg}; use memchr::memmem; use std::io::{stdin, stdout, BufReader, Read, Write}; use std::{fs::File, path::Path}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static NAME: &str = "tac"; @@ -141,11 +142,11 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { if path.is_dir() { - show_error!("{}: read error: Invalid argument", filename); + show_error!("{}: read error: Invalid argument", filename.maybe_quote()); } else { show_error!( - "failed to open '{}' for reading: No such file or directory", - filename + "failed to open {} for reading: No such file or directory", + filename.quote() ); } exit_code = 1; @@ -154,7 +155,7 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { match File::open(path) { Ok(f) => Box::new(f) as Box, Err(e) => { - show_error!("failed to open '{}' for reading: {}", filename, e); + show_error!("failed to open {} for reading: {}", filename.quote(), e); exit_code = 1; continue; } @@ -163,7 +164,7 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { let mut data = Vec::new(); if let Err(e) = file.read_to_end(&mut data) { - show_error!("failed to read '{}': {}", filename, e); + show_error!("failed to read {}: {}", filename.quote(), e); exit_code = 1; continue; }; diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index d98835265..d2fb015bf 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -12,7 +12,8 @@ use clap::{crate_version, App, Arg}; use retain_mut::RetainMut; use std::fs::OpenOptions; use std::io::{copy, sink, stdin, stdout, Error, ErrorKind, Read, Result, Write}; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; +use uucore::display::Quotable; #[cfg(unix)] use uucore::libc; @@ -167,7 +168,7 @@ impl Write for MultiWriter { let result = writer.write_all(buf); match result { Err(f) => { - show_error!("{}: {}", writer.name, f.to_string()); + show_error!("{}: {}", writer.name.maybe_quote(), f); false } _ => true, @@ -181,7 +182,7 @@ impl Write for MultiWriter { let result = writer.flush(); match result { Err(f) => { - show_error!("{}: {}", writer.name, f.to_string()); + show_error!("{}: {}", writer.name.maybe_quote(), f); false } _ => true, @@ -214,7 +215,7 @@ impl Read for NamedReader { fn read(&mut self, buf: &mut [u8]) -> Result { match self.inner.read(buf) { Err(f) => { - show_error!("{}: {}", Path::new("stdin").display(), f.to_string()); + show_error!("stdin: {}", f); Err(f) } okay => okay, diff --git a/src/uu/test/src/parser.rs b/src/uu/test/src/parser.rs index 7b7d77469..0d68cea39 100644 --- a/src/uu/test/src/parser.rs +++ b/src/uu/test/src/parser.rs @@ -10,6 +10,8 @@ use std::ffi::OsString; use std::iter::Peekable; +use uucore::display::Quotable; + /// Represents one of the binary comparison operators for strings, integers, or files #[derive(Debug, PartialEq)] pub enum Operator { @@ -43,19 +45,22 @@ impl Symbol { /// Returns Symbol::None in place of None fn new(token: Option) -> Symbol { match token { - Some(s) => match s.to_string_lossy().as_ref() { - "(" => Symbol::LParen, - "!" => Symbol::Bang, - "-a" | "-o" => Symbol::BoolOp(s), - "=" | "==" | "!=" => Symbol::Op(Operator::String(s)), - "-eq" | "-ge" | "-gt" | "-le" | "-lt" | "-ne" => Symbol::Op(Operator::Int(s)), - "-ef" | "-nt" | "-ot" => Symbol::Op(Operator::File(s)), - "-n" | "-z" => Symbol::UnaryOp(UnaryOperator::StrlenOp(s)), - "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-G" | "-h" | "-k" | "-L" | "-O" - | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => { - Symbol::UnaryOp(UnaryOperator::FiletestOp(s)) - } - _ => Symbol::Literal(s), + Some(s) => match s.to_str() { + Some(t) => match t { + "(" => Symbol::LParen, + "!" => Symbol::Bang, + "-a" | "-o" => Symbol::BoolOp(s), + "=" | "==" | "!=" => Symbol::Op(Operator::String(s)), + "-eq" | "-ge" | "-gt" | "-le" | "-lt" | "-ne" => Symbol::Op(Operator::Int(s)), + "-ef" | "-nt" | "-ot" => Symbol::Op(Operator::File(s)), + "-n" | "-z" => Symbol::UnaryOp(UnaryOperator::StrlenOp(s)), + "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-G" | "-h" | "-k" | "-L" | "-O" + | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => { + Symbol::UnaryOp(UnaryOperator::FiletestOp(s)) + } + _ => Symbol::Literal(s), + }, + None => Symbol::Literal(s), }, None => Symbol::None, } @@ -391,7 +396,7 @@ impl Parser { self.expr(); match self.tokens.next() { - Some(token) => Err(format!("extra argument '{}'", token.to_string_lossy())), + Some(token) => Err(format!("extra argument {}", token.quote())), None => Ok(()), } } diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 50563ba49..5ce798bfa 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -13,7 +13,7 @@ mod parser; use clap::{crate_version, App, AppSettings}; use parser::{parse, Operator, Symbol, UnaryOperator}; use std::ffi::{OsStr, OsString}; -use std::path::Path; +use uucore::{display::Quotable, show_error}; const USAGE: &str = "test EXPRESSION or: test @@ -93,10 +93,7 @@ pub fn uu_app() -> App<'static, 'static> { pub fn uumain(mut args: impl uucore::Args) -> i32 { let program = args.next().unwrap_or_else(|| OsString::from("test")); - let binary_name = Path::new(&program) - .file_name() - .unwrap_or_else(|| OsStr::new("test")) - .to_string_lossy(); + let binary_name = uucore::util_name(); let mut args: Vec<_> = args.collect(); if binary_name.ends_with('[') { @@ -116,8 +113,8 @@ pub fn uumain(mut args: impl uucore::Args) -> i32 { } // If invoked via name '[', matching ']' must be in the last arg let last = args.pop(); - if last != Some(OsString::from("]")) { - eprintln!("[: missing ']'"); + if last.as_deref() != Some(OsStr::new("]")) { + show_error!("missing ']'"); return 2; } } @@ -133,7 +130,7 @@ pub fn uumain(mut args: impl uucore::Args) -> i32 { } } Err(e) => { - eprintln!("test: {}", e); + show_error!("{}", e); 2 } } @@ -190,11 +187,11 @@ fn eval(stack: &mut Vec) -> Result { }) } Some(Symbol::UnaryOp(UnaryOperator::FiletestOp(op))) => { - let op = op.to_string_lossy(); + let op = op.to_str().unwrap(); let f = pop_literal!(); - Ok(match op.as_ref() { + Ok(match op { "-b" => path(&f, PathCondition::BlockSpecial), "-c" => path(&f, PathCondition::CharacterSpecial), "-d" => path(&f, PathCondition::Directory), @@ -231,31 +228,33 @@ fn eval(stack: &mut Vec) -> Result { } fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { - let format_err = |value| format!("invalid integer '{}'", value); + let format_err = |value: &OsStr| format!("invalid integer {}", value.quote()); - let a = a.to_string_lossy(); - let a: i64 = a.parse().map_err(|_| format_err(a))?; + let a: i64 = a + .to_str() + .and_then(|s| s.parse().ok()) + .ok_or_else(|| format_err(a))?; - let b = b.to_string_lossy(); - let b: i64 = b.parse().map_err(|_| format_err(b))?; + let b: i64 = b + .to_str() + .and_then(|s| s.parse().ok()) + .ok_or_else(|| format_err(b))?; - let operator = op.to_string_lossy(); - Ok(match operator.as_ref() { - "-eq" => a == b, - "-ne" => a != b, - "-gt" => a > b, - "-ge" => a >= b, - "-lt" => a < b, - "-le" => a <= b, - _ => return Err(format!("unknown operator '{}'", operator)), + Ok(match op.to_str() { + Some("-eq") => a == b, + Some("-ne") => a != b, + Some("-gt") => a > b, + Some("-ge") => a >= b, + Some("-lt") => a < b, + Some("-le") => a <= b, + _ => return Err(format!("unknown operator {}", op.quote())), }) } fn isatty(fd: &OsStr) -> Result { - let fd = fd.to_string_lossy(); - - fd.parse() - .map_err(|_| format!("invalid integer '{}'", fd)) + fd.to_str() + .and_then(|s| s.parse().ok()) + .ok_or_else(|| format!("invalid integer {}", fd.quote())) .map(|i| { #[cfg(not(target_os = "redox"))] unsafe { diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 21b0a0c37..f686dde3b 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -16,6 +16,7 @@ use clap::{crate_version, App, AppSettings, Arg}; use std::io::ErrorKind; use std::process::{Command, Stdio}; use std::time::Duration; +use uucore::display::Quotable; use uucore::process::ChildExt; use uucore::signals::{signal_by_name_or_value, signal_name_by_value}; use uucore::InvalidEncodingHandling; @@ -61,7 +62,7 @@ impl Config { let signal_result = signal_by_name_or_value(signal_); match signal_result { None => { - unreachable!("invalid signal '{}'", signal_); + unreachable!("invalid signal {}", signal_.quote()); } Some(signal_value) => signal_value, } @@ -216,9 +217,9 @@ fn timeout( Ok(None) => { if verbose { show_error!( - "sending signal {} to command '{}'", + "sending signal {} to command {}", signal_name_by_value(signal).unwrap(), - cmd[0] + cmd[0].quote() ); } crash_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); @@ -233,7 +234,7 @@ fn timeout( } Ok(None) => { if verbose { - show_error!("sending signal KILL to command '{}'", cmd[0]); + show_error!("sending signal KILL to command {}", cmd[0].quote()); } crash_if_err!( ERR_EXIT_STATUS, diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 7c0903665..8dd7bf7d2 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -17,6 +17,7 @@ use clap::{crate_version, App, Arg, ArgGroup}; use filetime::*; use std::fs::{self, File}; use std::path::Path; +use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult, USimpleError}; static ABOUT: &str = "Update the access and modification times of each FILE to the current time."; @@ -82,7 +83,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } if let Err(e) = File::create(path) { - show!(e.map_err_context(|| format!("cannot touch '{}'", path.display()))); + show!(e.map_err_context(|| format!("cannot touch {}", path.quote()))); continue; }; @@ -122,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { filetime::set_file_times(path, atime, mtime) } - .map_err_context(|| format!("setting times of '{}'", path.display()))?; + .map_err_context(|| format!("setting times of {}", path.quote()))?; } Ok(()) @@ -209,7 +210,7 @@ fn stat(path: &Path, follow: bool) -> UResult<(FileTime, FileTime)> { } else { fs::metadata(path) } - .map_err_context(|| format!("failed to get attributes of '{}'", path.display()))?; + .map_err_context(|| format!("failed to get attributes of {}", path.quote()))?; Ok(( FileTime::from_last_access_time(&metadata), @@ -249,11 +250,16 @@ fn parse_timestamp(s: &str) -> UResult { 10 => ("%y%m%d%H%M", s.to_owned()), 11 => ("%Y%m%d%H%M.%S", format!("{}{}", now.tm_year + 1900, s)), 8 => ("%Y%m%d%H%M", format!("{}{}", now.tm_year + 1900, s)), - _ => return Err(USimpleError::new(1, format!("invalid date format '{}'", s))), + _ => { + return Err(USimpleError::new( + 1, + format!("invalid date format {}", s.quote()), + )) + } }; let tm = time::strptime(&ts, format) - .map_err(|_| USimpleError::new(1, format!("invalid date format '{}'", s)))?; + .map_err(|_| USimpleError::new(1, format!("invalid date format {}", s.quote())))?; let mut local = to_local(tm); local.tm_isdst = -1; @@ -269,7 +275,10 @@ fn parse_timestamp(s: &str) -> UResult { }; let tm2 = time::at(ts); if tm.tm_hour != tm2.tm_hour { - return Err(USimpleError::new(1, format!("invalid date format '{}'", s))); + return Err(USimpleError::new( + 1, + format!("invalid date format {}", s.quote()), + )); } Ok(ft) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index d46318e38..fbc4bab9b 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -21,7 +21,7 @@ use fnv::FnvHashMap; use std::io::{stdin, stdout, BufRead, BufWriter, Write}; use crate::expand::ExpandSet; -use uucore::InvalidEncodingHandling; +use uucore::{display::Quotable, InvalidEncodingHandling}; static ABOUT: &str = "translate or delete characters"; @@ -271,8 +271,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if !(delete_flag || squeeze_flag) && sets.len() < 2 { show_error!( - "missing operand after '{}'\nTry '{} --help' for more information.", - sets[0], + "missing operand after {}\nTry '{} --help' for more information.", + sets[0].quote(), uucore::execution_phrase() ); return 1; diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 062ef3811..6fb1f06f6 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -15,6 +15,7 @@ use std::convert::TryFrom; use std::fs::{metadata, OpenOptions}; use std::io::ErrorKind; use std::path::Path; +use uucore::display::Quotable; use uucore::parse_size::{parse_size, ParseSizeError}; #[derive(Debug, Eq, PartialEq)] @@ -120,8 +121,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let reference = matches.value_of(options::REFERENCE).map(String::from); crash!( 1, - "cannot stat '{}': No such file or directory", - reference.unwrap_or_else(|| "".to_string()) + "cannot stat {}: No such file or directory", + reference.as_deref().unwrap_or("").quote() ); // TODO: fix '--no-create' see test_reference and test_truncate_bytes_size } _ => crash!(1, "{}", e.to_string()), diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index c0ef66598..11798db13 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -14,6 +14,7 @@ use std::collections::{HashMap, HashSet}; use std::fs::File; use std::io::{stdin, BufRead, BufReader, Read}; use std::path::Path; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static SUMMARY: &str = "Topological sort the strings in FILE. @@ -45,7 +46,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { file_buf = match File::open(Path::new(&input)) { Ok(a) => a, _ => { - show_error!("{}: No such file or directory", input); + show_error!("{}: No such file or directory", input.maybe_quote()); return 1; } }; @@ -68,7 +69,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for ab in tokens.chunks(2) { match ab.len() { 2 => g.add_edge(&ab[0], &ab[1]), - _ => crash!(1, "{}: input contains an odd number of tokens", input), + _ => crash!( + 1, + "{}: input contains an odd number of tokens", + input.maybe_quote() + ), } } } diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 7fb9b2590..95383b89d 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -16,6 +16,7 @@ use std::fs::File; use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Stdout, Write}; use std::str::from_utf8; use unicode_width::UnicodeWidthChar; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static NAME: &str = "unexpand"; @@ -141,9 +142,9 @@ fn open(path: String) -> BufReader> { if path == "-" { BufReader::new(Box::new(stdin()) as Box) } else { - file_buf = match File::open(&path[..]) { + file_buf = match File::open(&path) { Ok(a) => a, - Err(e) => crash!(1, "{}: {}", &path[..], e), + Err(e) => crash!(1, "{}: {}", path.maybe_quote(), e), }; BufReader::new(Box::new(file_buf) as Box) } diff --git a/src/uu/uniq/src/uniq.rs b/src/uu/uniq/src/uniq.rs index 5c3fa3b1e..f84bfc26d 100644 --- a/src/uu/uniq/src/uniq.rs +++ b/src/uu/uniq/src/uniq.rs @@ -14,6 +14,7 @@ use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Result, Write} use std::path::Path; use std::str::FromStr; use strum_macros::{AsRefStr, EnumString}; +use uucore::display::Quotable; static ABOUT: &str = "Report or omit repeated lines."; pub mod options { @@ -217,7 +218,14 @@ fn get_line_string(io_line: Result>) -> String { fn opt_parsed(opt_name: &str, matches: &ArgMatches) -> Option { matches.value_of(opt_name).map(|arg_str| { let opt_val: Option = arg_str.parse().ok(); - opt_val.unwrap_or_else(|| crash!(1, "Invalid argument for {}: {}", opt_name, arg_str)) + opt_val.unwrap_or_else(|| { + crash!( + 1, + "Invalid argument for {}: {}", + opt_name, + arg_str.maybe_quote() + ) + }) }) } diff --git a/src/uu/unlink/src/unlink.rs b/src/uu/unlink/src/unlink.rs index c7fc00639..7c66708e0 100644 --- a/src/uu/unlink/src/unlink.rs +++ b/src/uu/unlink/src/unlink.rs @@ -17,6 +17,7 @@ use libc::{lstat, stat, unlink}; use libc::{S_IFLNK, S_IFMT, S_IFREG}; use std::ffi::CString; use std::io::{Error, ErrorKind}; +use uucore::display::Quotable; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Unlink the file at [FILE]."; @@ -63,7 +64,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let result = unsafe { lstat(c_string.as_ptr(), &mut buf as *mut stat) }; if result < 0 { - crash!(1, "Cannot stat '{}': {}", paths[0], Error::last_os_error()); + crash!( + 1, + "Cannot stat {}: {}", + paths[0].quote(), + Error::last_os_error() + ); } buf.st_mode & S_IFMT diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 8a2f20417..16ee01b88 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -5,6 +5,7 @@ //! Common functions to manage permissions +use crate::display::Quotable; use crate::error::strip_errno; use crate::error::UResult; use crate::error::USimpleError; @@ -80,29 +81,29 @@ pub fn wrap_chown>( VerbosityLevel::Silent => (), level => { out = format!( - "changing {} of '{}': {}", + "changing {} of {}: {}", if verbosity.groups_only { "group" } else { "ownership" }, - path.display(), + path.quote(), e ); if level == VerbosityLevel::Verbose { out = if verbosity.groups_only { format!( - "{}\nfailed to change group of '{}' from {} to {}", + "{}\nfailed to change group of {} from {} to {}", out, - path.display(), + path.quote(), entries::gid2grp(meta.gid()).unwrap(), entries::gid2grp(dest_gid).unwrap() ) } else { format!( - "{}\nfailed to change ownership of '{}' from {}:{} to {}:{}", + "{}\nfailed to change ownership of {} from {}:{} to {}:{}", out, - path.display(), + path.quote(), entries::uid2usr(meta.uid()).unwrap(), entries::gid2grp(meta.gid()).unwrap(), entries::uid2usr(dest_uid).unwrap(), @@ -120,15 +121,15 @@ pub fn wrap_chown>( VerbosityLevel::Changes | VerbosityLevel::Verbose => { out = if verbosity.groups_only { format!( - "changed group of '{}' from {} to {}", - path.display(), + "changed group of {} from {} to {}", + path.quote(), entries::gid2grp(meta.gid()).unwrap(), entries::gid2grp(dest_gid).unwrap() ) } else { format!( - "changed ownership of '{}' from {}:{} to {}:{}", - path.display(), + "changed ownership of {} from {}:{} to {}:{}", + path.quote(), entries::uid2usr(meta.uid()).unwrap(), entries::gid2grp(meta.gid()).unwrap(), entries::uid2usr(dest_uid).unwrap(), @@ -141,14 +142,14 @@ pub fn wrap_chown>( } else if verbosity.level == VerbosityLevel::Verbose { out = if verbosity.groups_only { format!( - "group of '{}' retained as {}", - path.display(), + "group of {} retained as {}", + path.quote(), entries::gid2grp(dest_gid).unwrap_or_default() ) } else { format!( - "ownership of '{}' retained as {}:{}", - path.display(), + "ownership of {} retained as {}:{}", + path.quote(), entries::uid2usr(dest_uid).unwrap(), entries::gid2grp(dest_gid).unwrap() ) @@ -358,9 +359,9 @@ impl ChownExecutor { match self.verbosity.level { VerbosityLevel::Silent => (), _ => show_error!( - "cannot {} '{}': {}", + "cannot {} {}: {}", if follow { "dereference" } else { "access" }, - path.display(), + path.quote(), strip_errno(&e) ), } diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 129ff9106..925246c24 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -72,6 +72,8 @@ use std::sync::atomic::Ordering; use once_cell::sync::Lazy; +use crate::display::Quotable; + pub fn get_utility_is_second_arg() -> bool { crate::macros::UTILITY_IS_SECOND_ARG.load(Ordering::SeqCst) } @@ -171,14 +173,15 @@ pub trait Args: Iterator + Sized { Ok(string) => Ok(string), Err(s_ret) => { full_conversion = false; - let lossy_conversion = s_ret.to_string_lossy(); eprintln!( - "Input with broken encoding occurred! (s = '{}') ", - &lossy_conversion + "Input with broken encoding occurred! (s = {}) ", + s_ret.quote() ); match handling { InvalidEncodingHandling::Ignore => Err(String::new()), - InvalidEncodingHandling::ConvertLossy => Err(lossy_conversion.to_string()), + InvalidEncodingHandling::ConvertLossy => { + Err(s_ret.to_string_lossy().into_owned()) + } InvalidEncodingHandling::Panic => { panic!("Broken encoding found but caller cannot handle it") } diff --git a/src/uucore/src/lib/mods/backup_control.rs b/src/uucore/src/lib/mods/backup_control.rs index 83617f8ed..acb7342b7 100644 --- a/src/uucore/src/lib/mods/backup_control.rs +++ b/src/uucore/src/lib/mods/backup_control.rs @@ -78,7 +78,10 @@ // spell-checker:ignore backupopt -use crate::error::{UError, UResult}; +use crate::{ + display::Quotable, + error::{UError, UResult}, +}; use clap::ArgMatches; use std::{ env, @@ -167,18 +170,22 @@ impl Display for BackupError { match self { BE::InvalidArgument(arg, origin) => write!( f, - "invalid argument '{}' for '{}'\n{}", - arg, origin, VALID_ARGS_HELP + "invalid argument {} for '{}'\n{}", + arg.quote(), + origin, + VALID_ARGS_HELP ), BE::AmbiguousArgument(arg, origin) => write!( f, - "ambiguous argument '{}' for '{}'\n{}", - arg, origin, VALID_ARGS_HELP + "ambiguous argument {} for '{}'\n{}", + arg.quote(), + origin, + VALID_ARGS_HELP ), BE::BackupImpossible() => write!(f, "cannot create backup"), // Placeholder for later // BE::BackupFailed(from, to, e) => Display::fmt( - // &uio_error!(e, "failed to backup '{}' to '{}'", from.display(), to.display()), + // &uio_error!(e, "failed to backup {} to {}", from.quote(), to.quote()), // f // ), } diff --git a/src/uucore/src/lib/mods/display.rs b/src/uucore/src/lib/mods/display.rs index e647a8c71..dfe64184f 100644 --- a/src/uucore/src/lib/mods/display.rs +++ b/src/uucore/src/lib/mods/display.rs @@ -87,13 +87,16 @@ macro_rules! impl_as_ref { }; } +impl_as_ref!(str); impl_as_ref!(&'_ str); impl_as_ref!(String); +impl_as_ref!(std::path::Path); impl_as_ref!(&'_ std::path::Path); impl_as_ref!(std::path::PathBuf); impl_as_ref!(std::path::Component<'_>); impl_as_ref!(std::path::Components<'_>); impl_as_ref!(std::path::Iter<'_>); +impl_as_ref!(std::ffi::OsStr); impl_as_ref!(&'_ std::ffi::OsStr); impl_as_ref!(std::ffi::OsString); @@ -106,6 +109,13 @@ impl Quotable for Cow<'_, str> { } } +impl Quotable for Cow<'_, std::path::Path> { + fn quote(&self) -> Quoted<'_> { + let text: &std::path::Path = self.as_ref(); + Quoted::new(text.as_ref()) + } +} + /// A wrapper around [`OsStr`] for printing paths with quoting and escaping applied. #[derive(Debug, Copy, Clone)] pub struct Quoted<'a> { @@ -407,6 +417,19 @@ pub fn println_verbatim>(text: S) -> io::Result<()> { Ok(()) } +/// Like `println_verbatim`, without the trailing newline. +pub fn print_verbatim>(text: S) -> io::Result<()> { + let mut stdout = io::stdout(); + #[cfg(any(unix, target_os = "wasi"))] + { + stdout.write_all(text.as_ref().as_bytes()) + } + #[cfg(not(any(unix, target_os = "wasi")))] + { + write!(stdout, "{}", std::path::Path::new(text.as_ref()).display()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index 04019e234..11ec91bdf 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -99,7 +99,10 @@ pub type UResult = Result>; /// An example of a custom error from `ls`: /// /// ``` -/// use uucore::error::{UError, UResult}; +/// use uucore::{ +/// display::Quotable, +/// error::{UError, UResult} +/// }; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -126,8 +129,8 @@ pub type UResult = Result>; /// impl Display for LsError { /// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { /// match self { -/// LsError::InvalidLineWidth(s) => write!(f, "invalid line width: '{}'", s), -/// LsError::NoMetadata(p) => write!(f, "could not open file: '{}'", p.display()), +/// LsError::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()), +/// LsError::NoMetadata(p) => write!(f, "could not open file: {}", p.quote()), /// } /// } /// } @@ -158,7 +161,10 @@ pub trait UError: Error + Send { /// # Example /// /// ``` - /// use uucore::error::{UError}; + /// use uucore::{ + /// display::Quotable, + /// error::UError + /// }; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -189,8 +195,8 @@ pub trait UError: Error + Send { /// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { /// use MyError as ME; /// match self { - /// ME::Foo(s) => write!(f, "Unknown Foo: '{}'", s), - /// ME::Bar(p) => write!(f, "Couldn't find Bar: '{}'", p.display()), + /// ME::Foo(s) => write!(f, "Unknown Foo: {}", s.quote()), + /// ME::Bar(p) => write!(f, "Couldn't find Bar: {}", p.quote()), /// ME::Bing() => write!(f, "Exterminate!"), /// } /// } @@ -209,7 +215,10 @@ pub trait UError: Error + Send { /// # Example /// /// ``` - /// use uucore::error::{UError}; + /// use uucore::{ + /// display::Quotable, + /// error::UError + /// }; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -240,8 +249,8 @@ pub trait UError: Error + Send { /// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { /// use MyError as ME; /// match self { - /// ME::Foo(s) => write!(f, "Unknown Foo: '{}'", s), - /// ME::Bar(p) => write!(f, "Couldn't find Bar: '{}'", p.display()), + /// ME::Foo(s) => write!(f, "Unknown Foo: {}", s.quote()), + /// ME::Bar(p) => write!(f, "Couldn't find Bar: {}", p.quote()), /// ME::Bing() => write!(f, "Exterminate!"), /// } /// } @@ -342,7 +351,10 @@ impl UError for UUsageError { /// There are two ways to construct this type: with [`UIoError::new`] or by calling the /// [`FromIo::map_err_context`] method on a [`std::io::Result`] or [`std::io::Error`]. /// ``` -/// use uucore::error::{FromIo, UResult, UIoError, UError}; +/// use uucore::{ +/// display::Quotable, +/// error::{FromIo, UResult, UIoError, UError} +/// }; /// use std::fs::File; /// use std::path::Path; /// let path = Path::new("test.txt"); @@ -350,12 +362,12 @@ impl UError for UUsageError { /// // Manual construction /// let e: Box = UIoError::new( /// std::io::ErrorKind::NotFound, -/// format!("cannot access '{}'", path.display()) +/// format!("cannot access {}", path.quote()) /// ); /// let res: UResult<()> = Err(e.into()); /// /// // Converting from an `std::io::Error`. -/// let res: UResult = File::open(path).map_err_context(|| format!("cannot access '{}'", path.display())); +/// let res: UResult = File::open(path).map_err_context(|| format!("cannot access {}", path.quote())); /// ``` #[derive(Debug)] pub struct UIoError { diff --git a/src/uucore/src/lib/mods/ranges.rs b/src/uucore/src/lib/mods/ranges.rs index 9e1e67d5a..f142e14fb 100644 --- a/src/uucore/src/lib/mods/ranges.rs +++ b/src/uucore/src/lib/mods/ranges.rs @@ -9,6 +9,8 @@ use std::str::FromStr; +use crate::display::Quotable; + #[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] pub struct Range { pub low: usize, @@ -86,7 +88,7 @@ impl Range { for item in list.split(',') { let range_item = FromStr::from_str(item) - .map_err(|e| format!("range '{}' was invalid: {}", item, e))?; + .map_err(|e| format!("range {} was invalid: {}", item.quote(), e))?; ranges.push(range_item); } diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index ec0b08c9e..c05c0d3f1 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -9,6 +9,8 @@ use std::convert::TryFrom; use std::error::Error; use std::fmt; +use crate::display::Quotable; + /// Parse a size string into a number of bytes. /// /// A size string comprises an integer and an optional unit. The unit @@ -107,6 +109,9 @@ impl fmt::Display for ParseSizeError { } } +// FIXME: It's more idiomatic to move the formatting into the Display impl, +// but there's a lot of downstream code that constructs these errors manually +// that would be affected impl ParseSizeError { fn parse_failure(s: &str) -> ParseSizeError { // stderr on linux (GNU coreutils 8.32) (LC_ALL=C) @@ -140,7 +145,7 @@ impl ParseSizeError { // --width // --strings // etc. - ParseSizeError::ParseFailure(format!("'{}'", s)) + ParseSizeError::ParseFailure(format!("{}", s.quote())) } fn size_too_big(s: &str) -> ParseSizeError { @@ -160,7 +165,10 @@ impl ParseSizeError { // stderr on macos (brew - GNU coreutils 8.32) also differs for the same version, e.g.: // ghead: invalid number of bytes: '1Y': Value too large to be stored in data type // gtail: invalid number of bytes: '1Y': Value too large to be stored in data type - ParseSizeError::SizeTooBig(format!("'{}': Value too large for defined data type", s)) + ParseSizeError::SizeTooBig(format!( + "{}: Value too large for defined data type", + s.quote() + )) } } @@ -262,7 +270,7 @@ mod tests { for &test_string in &test_strings { assert_eq!( parse_size(test_string).unwrap_err(), - ParseSizeError::ParseFailure(format!("'{}'", test_string)) + ParseSizeError::ParseFailure(format!("{}", test_string.quote())) ); } } diff --git a/src/uucore/src/lib/parser/parse_time.rs b/src/uucore/src/lib/parser/parse_time.rs index fdf43b727..68f0ca8d0 100644 --- a/src/uucore/src/lib/parser/parse_time.rs +++ b/src/uucore/src/lib/parser/parse_time.rs @@ -9,6 +9,8 @@ use std::time::Duration; +use crate::display::Quotable; + pub fn from_str(string: &str) -> Result { let len = string.len(); if len == 0 { @@ -25,13 +27,13 @@ pub fn from_str(string: &str) -> Result { if string == "inf" || string == "infinity" { ("inf", 1) } else { - return Err(format!("invalid time interval '{}'", string)); + return Err(format!("invalid time interval {}", string.quote())); } } }; let num = numstr .parse::() - .map_err(|e| format!("invalid time interval '{}': {}", string, e))?; + .map_err(|e| format!("invalid time interval {}: {}", string.quote(), e))?; const NANOS_PER_SEC: u32 = 1_000_000_000; let whole_secs = num.trunc(); diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 3bac07d44..65d821d01 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -23,7 +23,7 @@ fn test_enter_chroot_fails() { assert!(result .stderr_str() - .starts_with("chroot: cannot chroot to jail: Operation not permitted (os error 1)")); + .starts_with("chroot: cannot chroot to 'jail': Operation not permitted (os error 1)")); } #[test] @@ -34,7 +34,7 @@ fn test_no_such_directory() { ucmd.arg("a") .fails() - .stderr_is("chroot: cannot change root directory to `a`: no such directory"); + .stderr_is("chroot: cannot change root directory to 'a': no such directory"); } #[test] diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 9590c1ac5..bf31ceb18 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -68,7 +68,7 @@ fn test_invalid_file() { .arg(folder_name) .fails() .no_stdout() - .stderr_contains("cksum: 'asdf' No such file or directory"); + .stderr_contains("cksum: asdf: No such file or directory"); // Then check when the file is of an invalid type at.mkdir(folder_name); @@ -76,7 +76,7 @@ fn test_invalid_file() { .arg(folder_name) .fails() .no_stdout() - .stderr_contains("cksum: 'asdf' Is a directory"); + .stderr_contains("cksum: asdf: Is a directory"); } // Make sure crc is correct for files larger than 32 bytes diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index cfe96d3c5..b389c9011 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -531,7 +531,7 @@ fn test_keys_invalid_field() { new_ucmd!() .args(&["-k", "1."]) .fails() - .stderr_only("sort: failed to parse key `1.`: failed to parse character index ``: cannot parse integer from empty string"); + .stderr_only("sort: failed to parse key '1.': failed to parse character index ``: cannot parse integer from empty string"); } #[test] @@ -539,7 +539,7 @@ fn test_keys_invalid_field_option() { new_ucmd!() .args(&["-k", "1.1x"]) .fails() - .stderr_only("sort: failed to parse key `1.1x`: invalid option: `x`"); + .stderr_only("sort: failed to parse key '1.1x': invalid option: `x`"); } #[test] @@ -547,7 +547,7 @@ fn test_keys_invalid_field_zero() { new_ucmd!() .args(&["-k", "0.1"]) .fails() - .stderr_only("sort: failed to parse key `0.1`: field index can not be 0"); + .stderr_only("sort: failed to parse key '0.1': field index can not be 0"); } #[test] @@ -555,7 +555,7 @@ fn test_keys_invalid_char_zero() { new_ucmd!() .args(&["-k", "1.0"]) .fails() - .stderr_only("sort: failed to parse key `1.0`: invalid character index 0 for the start position of a field"); + .stderr_only("sort: failed to parse key '1.0': invalid character index 0 for the start position of a field"); } #[test] diff --git a/tests/by-util/test_sum.rs b/tests/by-util/test_sum.rs index f09ba9d00..0248c05cf 100644 --- a/tests/by-util/test_sum.rs +++ b/tests/by-util/test_sum.rs @@ -59,7 +59,7 @@ fn test_invalid_file() { at.mkdir("a"); - ucmd.arg("a").fails().stderr_is("sum: 'a' Is a directory"); + ucmd.arg("a").fails().stderr_is("sum: a: Is a directory"); } #[test] @@ -68,5 +68,5 @@ fn test_invalid_metadata() { ucmd.arg("b") .fails() - .stderr_is("sum: 'b' No such file or directory"); + .stderr_is("sum: b: No such file or directory"); } diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index 23facd610..db74265a4 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -320,7 +320,7 @@ fn test_invalid_utf8_integer_compare() { cmd.run() .status_code(2) - .stderr_is("test: invalid integer 'fo�o'"); + .stderr_is("test: invalid integer $'fo\\x80o'"); let mut cmd = new_ucmd!(); cmd.raw.arg(arg); @@ -328,7 +328,7 @@ fn test_invalid_utf8_integer_compare() { cmd.run() .status_code(2) - .stderr_is("test: invalid integer 'fo�o'"); + .stderr_is("test: invalid integer $'fo\\x80o'"); } #[test] From 741f065a12ffe10014bd489ab41a8394c5faaa0b Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 7 Sep 2021 19:37:03 +0200 Subject: [PATCH 03/21] test_ls: Fix clippy warnings --- tests/by-util/test_ls.rs | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3d6092416..fe737512c 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -432,10 +432,7 @@ fn test_ls_long_symlink_color() { let mut result_lines = result .stdout_str() .lines() - .filter_map(|line| match line.starts_with("lrwx") { - true => Some(line), - false => None, - }) + .filter(|line| line.starts_with("lrwx")) .enumerate(); // For each enumerated line, we assert that the output of ls matches the expected output. @@ -455,14 +452,12 @@ fn test_ls_long_symlink_color() { // We look up the Colors that are expected in `colors` using the ColorReferences // stored in `expected_output`. - let expected_name_color = match expected_output[i].0 { - Some(color_reference) => Some(colors[color_reference[0]][color_reference[1]].as_str()), - None => None, - }; - let expected_target_color = match expected_output[i].2 { - Some(color_reference) => Some(colors[color_reference[0]][color_reference[1]].as_str()), - None => None, - }; + let expected_name_color = expected_output[i] + .0 + .map(|color_reference| colors[color_reference[0]][color_reference[1]].as_str()); + let expected_target_color = expected_output[i] + .2 + .map(|color_reference| colors[color_reference[0]][color_reference[1]].as_str()); // This is the important part. The asserts inside assert_names_and_colors_are_equal // will panic if the colors or names do not match the expected colors or names. @@ -470,11 +465,11 @@ fn test_ls_long_symlink_color() { // don't expect any color here, as in `expected_output[2], or don't know what specific // color to expect yet, as in expected_output[0:1]. assert_names_and_colors_are_equal( - &matched_name_color, + matched_name_color, expected_name_color, &matched_name, expected_output[i].1, - &matched_target_color, + matched_target_color, expected_target_color, &matched_target, expected_output[i].3, @@ -505,6 +500,7 @@ fn test_ls_long_symlink_color() { } } + #[allow(clippy::too_many_arguments)] fn assert_names_and_colors_are_equal( name_color: &str, expected_name_color: Option<&str>, @@ -530,7 +526,7 @@ fn test_ls_long_symlink_color() { fn capture_colored_string(input: &str) -> (Color, Name) { let colored_name = Regex::new(r"\x1b\[([0-9;]+)m(.+)\x1b\[0m").unwrap(); - match colored_name.captures(&input) { + match colored_name.captures(input) { Some(captures) => ( captures.get(1).unwrap().as_str().to_string(), captures.get(2).unwrap().as_str().to_string(), From 6d346b2307c2a1b337bf3b9741a0a93320a7b618 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 7 Sep 2021 19:48:06 +0200 Subject: [PATCH 04/21] Replace manual formatting by show_error!()/show_warning!() --- src/uu/date/src/date.rs | 15 ++++++++------- src/uu/env/src/env.rs | 3 ++- src/uu/od/src/multifilereader.rs | 4 ++-- src/uu/printf/src/memo.rs | 6 +++--- .../printf/src/tokenize/num_format/num_format.rs | 6 +++--- src/uu/test/src/parser.rs | 3 ++- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index b2ccf2e9f..adcf77024 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -18,6 +18,7 @@ use std::fs::File; use std::io::{BufRead, BufReader}; use std::path::PathBuf; use uucore::display::Quotable; +use uucore::show_error; #[cfg(windows)] use winapi::{ shared::minwindef::WORD, @@ -146,7 +147,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let format = if let Some(form) = matches.value_of(OPT_FORMAT) { if !form.starts_with('+') { - eprintln!("date: invalid date {}", form.quote()); + show_error!("invalid date {}", form.quote()); return 1; } let form = form[1..].to_string(); @@ -175,7 +176,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let set_to = match matches.value_of(OPT_SET).map(parse_date) { None => None, Some(Err((input, _err))) => { - eprintln!("date: invalid date {}", input.quote()); + show_error!("invalid date {}", input.quote()); return 1; } Some(Ok(date)) => Some(date), @@ -241,7 +242,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { println!("{}", formatted); } Err((input, _err)) => { - println!("date: invalid date {}", input.quote()); + show_error!("invalid date {}", input.quote()); } } } @@ -353,13 +354,13 @@ fn set_system_datetime(_date: DateTime) -> i32 { #[cfg(target_os = "macos")] fn set_system_datetime(_date: DateTime) -> i32 { - eprintln!("date: setting the date is not supported by macOS"); + show_error!("setting the date is not supported by macOS"); 1 } #[cfg(target_os = "redox")] fn set_system_datetime(_date: DateTime) -> i32 { - eprintln!("date: setting the date is not supported by Redox"); + show_error!("setting the date is not supported by Redox"); 1 } @@ -379,7 +380,7 @@ fn set_system_datetime(date: DateTime) -> i32 { if result != 0 { let error = std::io::Error::last_os_error(); - eprintln!("date: cannot set date: {}", error); + show_error!("cannot set date: {}", error); error.raw_os_error().unwrap() } else { 0 @@ -409,7 +410,7 @@ fn set_system_datetime(date: DateTime) -> i32 { if result == 0 { let error = std::io::Error::last_os_error(); - eprintln!("date: cannot set date: {}", error); + show_error!("cannot set date: {}", error); error.raw_os_error().unwrap() } else { 0 diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index af889d093..aec97fc24 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -22,6 +22,7 @@ use std::env; use std::io::{self, Write}; use std::iter::Iterator; use std::process::Command; +use uucore::display::Quotable; use uucore::error::{UResult, USimpleError}; const USAGE: &str = "env [OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]"; @@ -93,7 +94,7 @@ fn load_config_file(opts: &mut Options) -> UResult<()> { }; let conf = conf.map_err(|error| { - eprintln!("env: error: \"{}\": {}", file, error); + show_error!("{}: {}", file.maybe_quote(), error); 1 })?; diff --git a/src/uu/od/src/multifilereader.rs b/src/uu/od/src/multifilereader.rs index 90796b2eb..1b3aba03d 100644 --- a/src/uu/od/src/multifilereader.rs +++ b/src/uu/od/src/multifilereader.rs @@ -59,7 +59,7 @@ impl<'b> MultifileReader<'b> { // print an error at the time that the file is needed, // then move on the the next file. // This matches the behavior of the original `od` - eprintln!("{}: {}: {}", uucore::util_name(), fname.maybe_quote(), e); + show_error!("{}: {}", fname.maybe_quote(), e); self.any_err = true } } @@ -92,7 +92,7 @@ impl<'b> io::Read for MultifileReader<'b> { Ok(0) => break, Ok(n) => n, Err(e) => { - eprintln!("{}: I/O: {}", uucore::util_name(), e); + show_error!("I/O: {}", e); self.any_err = true; break; } diff --git a/src/uu/printf/src/memo.rs b/src/uu/printf/src/memo.rs index f5d41aeb6..a87d4fa89 100644 --- a/src/uu/printf/src/memo.rs +++ b/src/uu/printf/src/memo.rs @@ -9,7 +9,7 @@ use itertools::put_back_n; use std::iter::Peekable; use std::slice::Iter; use uucore::display::Quotable; -use uucore::show_error; +use uucore::show_warning; use crate::tokenize::sub::Sub; use crate::tokenize::token::{Token, Tokenizer}; @@ -20,8 +20,8 @@ pub struct Memo { } fn warn_excess_args(first_arg: &str) { - show_error!( - "warning: ignoring excess arguments, starting with {}", + show_warning!( + "ignoring excess arguments, starting with {}", first_arg.quote() ); } diff --git a/src/uu/printf/src/tokenize/num_format/num_format.rs b/src/uu/printf/src/tokenize/num_format/num_format.rs index 74666ad8e..a9fee58e1 100644 --- a/src/uu/printf/src/tokenize/num_format/num_format.rs +++ b/src/uu/printf/src/tokenize/num_format/num_format.rs @@ -8,7 +8,7 @@ use std::env; use std::vec::Vec; use uucore::display::Quotable; -use uucore::show_error; +use uucore::{show_error, show_warning}; use super::format_field::{FieldType, FormatField}; use super::formatter::{Base, FormatPrimitive, Formatter, InitialPrefix}; @@ -30,8 +30,8 @@ fn warn_char_constant_ign(remaining_bytes: Vec) { Ok(_) => {} Err(e) => { if let env::VarError::NotPresent = e { - show_error!( - "warning: {:?}: character(s) following character \ + show_warning!( + "{:?}: character(s) following character \ constant have been ignored", &*remaining_bytes ); diff --git a/src/uu/test/src/parser.rs b/src/uu/test/src/parser.rs index 0d68cea39..ce4c0dec0 100644 --- a/src/uu/test/src/parser.rs +++ b/src/uu/test/src/parser.rs @@ -11,6 +11,7 @@ use std::ffi::OsString; use std::iter::Peekable; use uucore::display::Quotable; +use uucore::show_error; /// Represents one of the binary comparison operators for strings, integers, or files #[derive(Debug, PartialEq)] @@ -207,7 +208,7 @@ impl Parser { // case 2: error if end of stream is `( ` [symbol] => { - eprintln!("test: missing argument after ‘{:?}’", symbol); + show_error!("missing argument after ‘{:?}’", symbol); std::process::exit(2); } From 918603909bd32e6ac7a3f95c5140d17b02239579 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 7 Sep 2021 19:48:28 +0200 Subject: [PATCH 05/21] Update hashsum quoting FIXME instructions --- src/uu/hashsum/src/hashsum.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index 2081d7612..f820b083e 100644 --- a/src/uu/hashsum/src/hashsum.rs +++ b/src/uu/hashsum/src/hashsum.rs @@ -547,10 +547,15 @@ where ) ) .to_ascii_lowercase(); - // FIXME: (How) should these be quoted? - // They seem like they would be processed programmatically, and - // our ordinary quoting might interfere, but newlines should be - // sanitized probably + // FIXME: Filenames with newlines should be treated specially. + // GNU appears to replace newlines by \n and backslashes by + // \\ and prepend a backslash (to the hash or filename) if it did + // this escaping. + // Different sorts of output (checking vs outputting hashes) may + // handle this differently. Compare carefully to GNU. + // If you can, try to preserve invalid unicode using OsStr(ing)Ext + // and display it using uucore::display::print_verbatim(). This is + // easier (and more important) on Unix than on Windows. if sum == real_sum { if !options.quiet { println!("{}: OK", ck_filename); From 1ef2574b085f11643522c720b6351ae88221001a Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 7 Sep 2021 20:15:30 +0200 Subject: [PATCH 06/21] Replace backtick quoting --- src/uu/chroot/src/chroot.rs | 2 +- src/uu/fmt/src/fmt.rs | 6 +++--- src/uu/sort/src/sort.rs | 8 ++++---- tests/by-util/test_sort.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 240b5eafe..40799d009 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -150,7 +150,7 @@ fn set_context(root: &Path, options: &clap::ArgMatches) { Some(u) => { let s: Vec<&str> = u.split(':').collect(); if s.len() != 2 || s.iter().any(|&spec| spec.is_empty()) { - crash!(1, "invalid userspec: `{}`", u) + crash!(1, "invalid userspec: {}", u.quote()) }; s } diff --git a/src/uu/fmt/src/fmt.rs b/src/uu/fmt/src/fmt.rs index df5d971e5..669c98b14 100644 --- a/src/uu/fmt/src/fmt.rs +++ b/src/uu/fmt/src/fmt.rs @@ -133,7 +133,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fmt_opts.width = match s.parse::() { Ok(t) => t, Err(e) => { - crash!(1, "Invalid WIDTH specification: `{}': {}", s, e); + crash!(1, "Invalid WIDTH specification: {}: {}", s.quote(), e); } }; if fmt_opts.width > MAX_WIDTH { @@ -150,7 +150,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fmt_opts.goal = match s.parse::() { Ok(t) => t, Err(e) => { - crash!(1, "Invalid GOAL specification: `{}': {}", s, e); + crash!(1, "Invalid GOAL specification: {}: {}", s.quote(), e); } }; if !matches.is_present(OPT_WIDTH) { @@ -164,7 +164,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { fmt_opts.tabwidth = match s.parse::() { Ok(t) => t, Err(e) => { - crash!(1, "Invalid TABWIDTH specification: `{}': {}", s, e); + crash!(1, "Invalid TABWIDTH specification: {}: {}", s.quote(), e); } }; }; diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index bab6dd770..c8a429e53 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -767,19 +767,19 @@ impl KeyPosition { let field = field_and_char .next() - .ok_or_else(|| format!("invalid key `{}`", key))?; + .ok_or_else(|| format!("invalid key {}", key.quote()))?; let char = field_and_char.next(); let field = field .parse() - .map_err(|e| format!("failed to parse field index `{}`: {}", field, e))?; + .map_err(|e| format!("failed to parse field index {}: {}", field.quote(), e))?; if field == 0 { return Err("field index can not be 0".to_string()); } let char = char.map_or(Ok(default_char_index), |char| { char.parse() - .map_err(|e| format!("failed to parse character index `{}`: {}", char, e)) + .map_err(|e| format!("failed to parse character index {}: {}", char.quote(), e)) })?; Ok(Self { @@ -890,7 +890,7 @@ impl FieldSelector { 'R' => key_settings.set_sort_mode(SortMode::Random)?, 'r' => key_settings.reverse = true, 'V' => key_settings.set_sort_mode(SortMode::Version)?, - c => return Err(format!("invalid option: `{}`", c)), + c => return Err(format!("invalid option: '{}'", c)), } } Ok(ignore_blanks) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index b389c9011..2aa26ad24 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -531,7 +531,7 @@ fn test_keys_invalid_field() { new_ucmd!() .args(&["-k", "1."]) .fails() - .stderr_only("sort: failed to parse key '1.': failed to parse character index ``: cannot parse integer from empty string"); + .stderr_only("sort: failed to parse key '1.': failed to parse character index '': cannot parse integer from empty string"); } #[test] @@ -539,7 +539,7 @@ fn test_keys_invalid_field_option() { new_ucmd!() .args(&["-k", "1.1x"]) .fails() - .stderr_only("sort: failed to parse key '1.1x': invalid option: `x`"); + .stderr_only("sort: failed to parse key '1.1x': invalid option: 'x'"); } #[test] From e4aad6d971ae79f84504f98809154801e5d25530 Mon Sep 17 00:00:00 2001 From: 353fc443 <353fc443@pm.me> Date: Thu, 9 Sep 2021 17:59:02 +0000 Subject: [PATCH 07/21] uptime: added UResult --- src/uu/uptime/src/uptime.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/uu/uptime/src/uptime.rs b/src/uu/uptime/src/uptime.rs index e58f398db..f649b96b6 100644 --- a/src/uu/uptime/src/uptime.rs +++ b/src/uu/uptime/src/uptime.rs @@ -17,6 +17,8 @@ extern crate uucore; pub use uucore::libc; use uucore::libc::time_t; +use uucore::error::{UResult, USimpleError}; + static ABOUT: &str = "Display the current time, the length of time the system has been up,\n\ the number of users on the system, and the average number of jobs\n\ in the run queue over the last 1, 5 and 15 minutes."; @@ -36,21 +38,20 @@ fn usage() -> String { format!("{0} [OPTION]...", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let matches = uu_app().usage(&usage[..]).get_matches_from(args); let (boot_time, user_count) = process_utmpx(); let uptime = get_uptime(boot_time); if uptime < 0 { - show_error!("could not retrieve system uptime"); - - 1 + Err(USimpleError::new(1, "could not retrieve system uptime")) } else { if matches.is_present(options::SINCE) { let initial_date = Local.timestamp(Utc::now().timestamp() - uptime, 0); println!("{}", initial_date.format("%Y-%m-%d %H:%M:%S")); - return 0; + return Ok(()); } print_time(); @@ -59,7 +60,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { print_nusers(user_count); print_loadavg(); - 0 + Ok(()) } } From ed258e3c9c411f5d9c41950d10d6f094b8fa07e6 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Thu, 9 Sep 2021 21:48:17 +0200 Subject: [PATCH 08/21] touch: add --ref as an alias (#2641) --- src/uu/touch/src/touch.rs | 3 ++- tests/by-util/test_touch.rs | 24 ++++++++++++++++-------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 8dd7bf7d2..6997def09 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -6,7 +6,7 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -// spell-checker:ignore (ToDO) filetime strptime utcoff strs datetime MMDDhhmm +// spell-checker:ignore (ToDO) filetime strptime utcoff strs datetime MMDDhhmm clapv pub extern crate filetime; @@ -176,6 +176,7 @@ pub fn uu_app() -> App<'static, 'static> { Arg::with_name(options::sources::REFERENCE) .short("r") .long(options::sources::REFERENCE) + .alias("ref") // clapv3 .help("use this file's times instead of the current time") .value_name("FILE"), ) diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index e8a17bbca..983d14fe2 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -6,6 +6,7 @@ use self::touch::filetime::{self, FileTime}; extern crate time; use crate::common::util::*; +use std::fs::remove_file; use std::path::PathBuf; fn get_file_times(at: &AtPath, path: &str) -> (FileTime, FileTime) { @@ -323,7 +324,8 @@ fn test_touch_no_dereference() { #[test] fn test_touch_reference() { - let (at, mut ucmd) = at_and_ucmd!(); + let scenario = TestScenario::new("touch"); + let (at, mut _ucmd) = (scenario.fixtures.clone(), scenario.ucmd()); let file_a = "test_touch_reference_a"; let file_b = "test_touch_reference_b"; let start_of_year = str_to_filetime("%Y%m%d%H%M", "201501010000"); @@ -331,15 +333,21 @@ fn test_touch_reference() { at.touch(file_a); set_file_times(&at, file_a, start_of_year, start_of_year); assert!(at.file_exists(file_a)); + for &opt in &["-r", "--ref", "--reference"] { + scenario + .ccmd("touch") + .args(&[opt, file_a, file_b]) + .succeeds() + .no_stderr(); - ucmd.args(&["-r", file_a, file_b]).succeeds().no_stderr(); + assert!(at.file_exists(file_b)); - assert!(at.file_exists(file_b)); - - let (atime, mtime) = get_file_times(&at, file_b); - assert_eq!(atime, mtime); - assert_eq!(atime, start_of_year); - assert_eq!(mtime, start_of_year); + let (atime, mtime) = get_file_times(&at, file_b); + assert_eq!(atime, mtime); + assert_eq!(atime, start_of_year); + assert_eq!(mtime, start_of_year); + let _ = remove_file(file_b); + } } #[test] From 75e5c42e4038d79bb0fbc69a22815a8042d2a699 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 10 Sep 2021 08:15:23 +0200 Subject: [PATCH 09/21] chown: support '.' as 'user.group' separator (like ':') (#2638) * chown: support '.' as 'user.group' separator (like ':') It also manages the case: user.name:group (valid too) Should fix: gnu/tests/chown/separator.sh --- src/uu/chown/src/chown.rs | 65 ++++++++++++++++------ tests/by-util/test_chown.rs | 106 ++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 18 deletions(-) diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 5525f9325..f9412a768 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -5,7 +5,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) COMFOLLOW Passwd RFILE RFILE's derefer dgid duid +// spell-checker:ignore (ToDO) COMFOLLOW Passwd RFILE RFILE's derefer dgid duid groupname #[macro_use] extern crate uucore; @@ -31,7 +31,7 @@ fn get_usage() -> String { fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Option, IfFrom)> { let filter = if let Some(spec) = matches.value_of(options::FROM) { - match parse_spec(spec)? { + match parse_spec(spec, ':')? { (Some(uid), None) => IfFrom::User(uid), (None, Some(gid)) => IfFrom::Group(gid), (Some(uid), Some(gid)) => IfFrom::UserGroup(uid, gid), @@ -49,7 +49,7 @@ fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Optio dest_gid = Some(meta.gid()); dest_uid = Some(meta.uid()); } else { - let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap())?; + let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap(), ':')?; dest_uid = u; dest_gid = g; } @@ -166,23 +166,49 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn parse_spec(spec: &str) -> UResult<(Option, Option)> { - let args = spec.split_terminator(':').collect::>(); - let usr_only = args.len() == 1 && !args[0].is_empty(); - let grp_only = args.len() == 2 && args[0].is_empty(); - let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty(); - let uid = if usr_only || usr_grp { - Some( - Passwd::locate(args[0]) - .map_err(|_| USimpleError::new(1, format!("invalid user: {}", spec.quote())))? - .uid(), - ) +/// Parse the username and groupname +/// +/// In theory, it should be username:groupname +/// but ... +/// it can user.name:groupname +/// or username.groupname +/// +/// # Arguments +/// +/// * `spec` - The input from the user +/// * `sep` - Should be ':' or '.' +fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { + assert!(['.', ':'].contains(&sep)); + let mut args = spec.splitn(2, sep); + let user = args.next().unwrap_or(""); + let group = args.next().unwrap_or(""); + + let uid = if !user.is_empty() { + Some(match Passwd::locate(user) { + Ok(u) => u.uid(), // We have been able to get the uid + Err(_) => + // we have NOT been able to find the uid + // but we could be in the case where we have user.group + { + if spec.contains('.') && !spec.contains(':') && sep == ':' { + // but the input contains a '.' but not a ':' + // we might have something like username.groupname + // So, try to parse it this way + return parse_spec(spec, '.'); + } else { + return Err(USimpleError::new( + 1, + format!("invalid user: {}", spec.quote()), + ))?; + } + } + }) } else { None }; - let gid = if grp_only || usr_grp { + let gid = if !group.is_empty() { Some( - Group::locate(args[1]) + Group::locate(group) .map_err(|_| USimpleError::new(1, format!("invalid group: {}", spec.quote())))? .gid(), ) @@ -198,7 +224,10 @@ mod test { #[test] fn test_parse_spec() { - assert!(matches!(parse_spec(":"), Ok((None, None)))); - assert!(format!("{}", parse_spec("::").err().unwrap()).starts_with("invalid group: ")); + assert!(matches!(parse_spec(":", ':'), Ok((None, None)))); + assert!(matches!(parse_spec(".", ':'), Ok((None, None)))); + assert!(matches!(parse_spec(".", '.'), Ok((None, None)))); + assert!(format!("{}", parse_spec("::", ':').err().unwrap()).starts_with("invalid group: ")); + assert!(format!("{}", parse_spec("..", ':').err().unwrap()).starts_with("invalid group: ")); } } diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 725e4b244..d5ebb4600 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -139,6 +139,14 @@ fn test_chown_only_owner_colon() { .succeeds() .stderr_contains(&"retained as"); + scene + .ucmd() + .arg(format!("{}.", user_name)) + .arg("--verbose") + .arg(file1) + .succeeds() + .stderr_contains(&"retained as"); + scene .ucmd() .arg("root:") @@ -180,6 +188,14 @@ fn test_chown_only_colon() { .arg(file1) .fails() .stderr_contains(&"invalid group: '::'"); + + scene + .ucmd() + .arg("..") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group: '..'"); } #[test] @@ -232,6 +248,22 @@ fn test_chown_owner_group() { } result.stderr_contains(&"retained as"); + scene + .ucmd() + .arg("root:root:root") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group"); + + scene + .ucmd() + .arg("root.root.root") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"invalid group"); + // TODO: on macos group name is not recognized correctly: "chown: invalid group: 'root:root' #[cfg(any(windows, all(unix, not(target_os = "macos"))))] scene @@ -243,6 +275,67 @@ fn test_chown_owner_group() { .stderr_contains(&"failed to change"); } +#[test] +// FixME: Fails on freebsd because of chown: invalid group: 'root:root' +#[cfg(not(target_os = "freebsd"))] +fn test_chown_various_input() { + // test chown username:group file.txt + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let result = scene.cmd("whoami").run(); + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { + return; + } + + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); + + let file1 = "test_chown_file1"; + at.touch(file1); + + let result = scene.cmd("id").arg("-gn").run(); + if skipping_test_is_okay(&result, "id: cannot find name for group ID") { + return; + } + let group_name = String::from(result.stdout_str().trim()); + assert!(!group_name.is_empty()); + + let result = scene + .ucmd() + .arg(format!("{}:{}", user_name, group_name)) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "chown: invalid group:") { + return; + } + result.stderr_contains(&"retained as"); + + // check that username.groupname is understood + let result = scene + .ucmd() + .arg(format!("{}.{}", user_name, group_name)) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "chown: invalid group:") { + return; + } + result.stderr_contains(&"retained as"); + + // Fails as user.name doesn't exist in the CI + // but it is valid + scene + .ucmd() + .arg(format!("{}:{}", "user.name", "groupname")) + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"chown: invalid user: 'user.name:groupname'"); +} + #[test] // FixME: on macos & freebsd group name is not recognized correctly: "chown: invalid group: ':groupname' #[cfg(any( @@ -405,6 +498,19 @@ fn test_chown_owner_group_id() { } result.stderr_contains(&"retained as"); + let result = scene + .ucmd() + .arg(format!("{}.{}", user_id, group_id)) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "invalid user") { + // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" + // stderr: "chown: invalid user: '1001.116' + return; + } + result.stderr_contains(&"retained as"); + scene .ucmd() .arg("0:0") From d05410383fa5f82ceaab25fd8e83da0bca184e55 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 10 Sep 2021 18:56:12 +0200 Subject: [PATCH 10/21] chown: Fix clippy warning to fix CI --- src/uu/chown/src/chown.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index f9412a768..1cd71d3f5 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -199,7 +199,7 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { return Err(USimpleError::new( 1, format!("invalid user: {}", spec.quote()), - ))?; + )); } } }) From fc77e51b64dde3c59012d91910a1feb3f7bb3fa7 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 9 Sep 2021 22:24:51 -0400 Subject: [PATCH 11/21] printf: derive Default impl for FormatPrimitive --- src/uu/printf/src/tokenize/num_format/formatter.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/uu/printf/src/tokenize/num_format/formatter.rs b/src/uu/printf/src/tokenize/num_format/formatter.rs index 0438f78bf..790c338ae 100644 --- a/src/uu/printf/src/tokenize/num_format/formatter.rs +++ b/src/uu/printf/src/tokenize/num_format/formatter.rs @@ -11,6 +11,7 @@ use super::format_field::FormatField; // output for a number, organized together // to allow for easy generalization of output manipulation // (e.g. max number of digits after decimal) +#[derive(Default)] pub struct FormatPrimitive { pub prefix: Option, pub pre_decimal: Option, @@ -18,17 +19,6 @@ pub struct FormatPrimitive { pub suffix: Option, } -impl Default for FormatPrimitive { - fn default() -> FormatPrimitive { - FormatPrimitive { - prefix: None, - pre_decimal: None, - post_decimal: None, - suffix: None, - } - } -} - #[derive(Clone, PartialEq)] pub enum Base { Ten = 10, From 91d39de16e3098e90d2e9dba8415a0083f3a24b4 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 10 Sep 2021 19:25:45 +0200 Subject: [PATCH 12/21] sort: derive Default impl for FieldSelector --- src/uu/sort/src/sort.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index c8a429e53..fe286aa6d 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -800,7 +800,7 @@ impl Default for KeyPosition { } } -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Debug, Default)] struct FieldSelector { from: KeyPosition, to: Option, @@ -812,18 +812,6 @@ struct FieldSelector { needs_selection: bool, } -impl Default for FieldSelector { - fn default() -> Self { - Self { - from: Default::default(), - to: None, - settings: Default::default(), - needs_tokens: false, - needs_selection: false, - } - } -} - impl FieldSelector { /// Splits this position into the actual position and the attached options. fn split_key_options(position: &str) -> (&str, &str) { From 8f901ee860cd8f28395a81ae7786cf6b8e74cfe7 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 18:10:13 +0200 Subject: [PATCH 13/21] tty: Make test_stdout_fail() less flaky --- tests/by-util/test_tty.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_tty.rs b/tests/by-util/test_tty.rs index f31aa67ee..ed490e7ab 100644 --- a/tests/by-util/test_tty.rs +++ b/tests/by-util/test_tty.rs @@ -66,10 +66,23 @@ fn test_wrong_argument() { #[test] // FixME: freebsd panic -#[cfg(not(any(windows, target_os = "freebsd")))] +#[cfg(all(unix, not(target_os = "freebsd")))] fn test_stdout_fail() { - let mut child = new_ucmd!().run_no_wait(); - drop(child.stdout.take()); - let status = child.wait().unwrap(); + use std::process::{Command, Stdio}; + let ts = TestScenario::new(util_name!()); + // Sleep inside a shell to ensure the process doesn't finish before we've + // closed its stdout + let mut proc = Command::new("sh") + .arg("-c") + .arg(format!( + "sleep 0.2; exec {} {}", + ts.bin_path.to_str().unwrap(), + ts.util_name + )) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + drop(proc.stdout.take()); + let status = proc.wait().unwrap(); assert_eq!(status.code(), Some(3)); } From c1079e0b1cb42e01e0e11afaab31886197cb4bb5 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 19:20:53 +0200 Subject: [PATCH 14/21] Move common pipe and splice functions into uucore This cuts down on repetitive unsafe code and repetitive code in general. --- Cargo.lock | 1 + src/uu/cat/Cargo.toml | 2 +- src/uu/cat/src/cat.rs | 30 ++++++++-------- src/uu/cat/src/splice.rs | 46 ++++++------------------ src/uu/wc/Cargo.toml | 2 +- src/uu/wc/src/count_fast.rs | 40 ++++++--------------- src/uu/yes/Cargo.toml | 2 +- src/uu/yes/src/splice.rs | 54 ++++------------------------ src/uucore/Cargo.toml | 2 ++ src/uucore/src/lib/features.rs | 2 ++ src/uucore/src/lib/features/pipes.rs | 54 ++++++++++++++++++++++++++++ src/uucore/src/lib/lib.rs | 2 ++ 12 files changed, 106 insertions(+), 131 deletions(-) create mode 100644 src/uucore/src/lib/features/pipes.rs diff --git a/Cargo.lock b/Cargo.lock index 808f62e15..ef00d9b9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3276,6 +3276,7 @@ dependencies = [ "getopts", "lazy_static", "libc", + "nix 0.20.0", "once_cell", "termion", "thiserror", diff --git a/src/uu/cat/Cargo.toml b/src/uu/cat/Cargo.toml index d80514385..d4f137d7e 100644 --- a/src/uu/cat/Cargo.toml +++ b/src/uu/cat/Cargo.toml @@ -18,7 +18,7 @@ path = "src/cat.rs" clap = { version = "2.33", features = ["wrap_help"] } thiserror = "1.0" atty = "0.2" -uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["fs"] } +uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["fs", "pipes"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } [target.'cfg(unix)'.dependencies] diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 9459ee86a..baf8af6d5 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -29,8 +29,6 @@ use std::os::unix::io::AsRawFd; /// Linux splice support #[cfg(any(target_os = "linux", target_os = "android"))] mod splice; -#[cfg(any(target_os = "linux", target_os = "android"))] -use std::os::unix::io::RawFd; /// Unix domain socket support #[cfg(unix)] @@ -137,10 +135,18 @@ struct OutputState { one_blank_kept: bool, } +#[cfg(unix)] +trait FdReadable: Read + AsRawFd {} +#[cfg(not(unix))] +trait FdReadable: Read {} + +#[cfg(unix)] +impl FdReadable for T where T: Read + AsRawFd {} +#[cfg(not(unix))] +impl FdReadable for T where T: Read {} + /// Represents an open file handle, stream, or other device -struct InputHandle { - #[cfg(any(target_os = "linux", target_os = "android"))] - file_descriptor: RawFd, +struct InputHandle { reader: R, is_interactive: bool, } @@ -297,7 +303,7 @@ pub fn uu_app() -> App<'static, 'static> { ) } -fn cat_handle( +fn cat_handle( handle: &mut InputHandle, options: &OutputOptions, state: &mut OutputState, @@ -319,8 +325,6 @@ fn cat_path( if path == "-" { let stdin = io::stdin(); let mut handle = InputHandle { - #[cfg(any(target_os = "linux", target_os = "android"))] - file_descriptor: stdin.as_raw_fd(), reader: stdin, is_interactive: atty::is(atty::Stream::Stdin), }; @@ -333,8 +337,6 @@ fn cat_path( let socket = UnixStream::connect(path)?; socket.shutdown(Shutdown::Write)?; let mut handle = InputHandle { - #[cfg(any(target_os = "linux", target_os = "android"))] - file_descriptor: socket.as_raw_fd(), reader: socket, is_interactive: false, }; @@ -347,8 +349,6 @@ fn cat_path( return Err(CatError::OutputIsInput); } let mut handle = InputHandle { - #[cfg(any(target_os = "linux", target_os = "android"))] - file_descriptor: file.as_raw_fd(), reader: file, is_interactive: false, }; @@ -437,14 +437,14 @@ fn get_input_type(path: &str) -> CatResult { /// Writes handle to stdout with no configuration. This allows a /// simple memory copy. -fn write_fast(handle: &mut InputHandle) -> CatResult<()> { +fn write_fast(handle: &mut InputHandle) -> CatResult<()> { let stdout = io::stdout(); let mut stdout_lock = stdout.lock(); #[cfg(any(target_os = "linux", target_os = "android"))] { // If we're on Linux or Android, try to use the splice() system call // for faster writing. If it works, we're done. - if !splice::write_fast_using_splice(handle, stdout_lock.as_raw_fd())? { + if !splice::write_fast_using_splice(handle, &stdout_lock)? { return Ok(()); } } @@ -462,7 +462,7 @@ fn write_fast(handle: &mut InputHandle) -> CatResult<()> { /// Outputs file contents to stdout in a line-by-line fashion, /// propagating any errors that might occur. -fn write_lines( +fn write_lines( handle: &mut InputHandle, options: &OutputOptions, state: &mut OutputState, diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs index 108997c4a..66630060c 100644 --- a/src/uu/cat/src/splice.rs +++ b/src/uu/cat/src/splice.rs @@ -1,10 +1,9 @@ -use super::{CatResult, InputHandle}; +use super::{CatResult, FdReadable, InputHandle}; -use nix::fcntl::{splice, SpliceFFlags}; -use nix::unistd::{self, pipe}; -use std::fs::File; -use std::io::Read; -use std::os::unix::io::{FromRawFd, RawFd}; +use nix::unistd; +use std::os::unix::io::{AsRawFd, RawFd}; + +use uucore::pipes::{pipe, splice, splice_exact}; const BUF_SIZE: usize = 1024 * 16; @@ -16,36 +15,25 @@ const BUF_SIZE: usize = 1024 * 16; /// The `bool` in the result value indicates if we need to fall back to normal /// copying or not. False means we don't have to. #[inline] -pub(super) fn write_fast_using_splice( +pub(super) fn write_fast_using_splice( handle: &mut InputHandle, - write_fd: RawFd, + write_fd: &impl AsRawFd, ) -> CatResult { let (pipe_rd, pipe_wr) = pipe()?; - // Ensure the pipe is closed when the function returns. - // SAFETY: The file descriptors do not have other owners. - let _handles = unsafe { (File::from_raw_fd(pipe_rd), File::from_raw_fd(pipe_wr)) }; - loop { - match splice( - handle.file_descriptor, - None, - pipe_wr, - None, - BUF_SIZE, - SpliceFFlags::empty(), - ) { + match splice(&handle.reader, &pipe_wr, BUF_SIZE) { Ok(n) => { if n == 0 { return Ok(false); } - if splice_exact(pipe_rd, write_fd, n).is_err() { + if splice_exact(&pipe_rd, write_fd, n).is_err() { // If the first splice manages to copy to the intermediate // pipe, but the second splice to stdout fails for some reason // we can recover by copying the data that we have from the // intermediate pipe to stdout using normal read/write. Then // we tell the caller to fall back. - copy_exact(pipe_rd, write_fd, n)?; + copy_exact(pipe_rd.as_raw_fd(), write_fd.as_raw_fd(), n)?; return Ok(true); } } @@ -56,20 +44,6 @@ pub(super) fn write_fast_using_splice( } } -/// Splice wrapper which handles short writes. -#[inline] -fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> { - let mut left = num_bytes; - loop { - let written = splice(read_fd, None, write_fd, None, left, SpliceFFlags::empty())?; - left -= written; - if left == 0 { - break; - } - } - Ok(()) -} - /// Caller must ensure that `num_bytes <= BUF_SIZE`, otherwise this function /// will panic. The way we use this function in `write_fast_using_splice` /// above is safe because `splice` is set to write at most `BUF_SIZE` to the diff --git a/src/uu/wc/Cargo.toml b/src/uu/wc/Cargo.toml index 80d6014da..5884f3746 100644 --- a/src/uu/wc/Cargo.toml +++ b/src/uu/wc/Cargo.toml @@ -16,7 +16,7 @@ path = "src/wc.rs" [dependencies] clap = { version = "2.33", features = ["wrap_help"] } -uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } +uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["pipes"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } bytecount = "0.6.2" utf-8 = "0.7.6" diff --git a/src/uu/wc/src/count_fast.rs b/src/uu/wc/src/count_fast.rs index b4078d6be..9351b6871 100644 --- a/src/uu/wc/src/count_fast.rs +++ b/src/uu/wc/src/count_fast.rs @@ -3,7 +3,7 @@ use crate::word_count::WordCount; use super::WordCountable; #[cfg(any(target_os = "linux", target_os = "android"))] -use std::fs::{File, OpenOptions}; +use std::fs::OpenOptions; use std::io::{self, ErrorKind, Read}; #[cfg(unix)] @@ -11,34 +11,17 @@ use libc::S_IFREG; #[cfg(unix)] use nix::sys::stat; #[cfg(any(target_os = "linux", target_os = "android"))] -use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; +use std::os::unix::io::AsRawFd; #[cfg(any(target_os = "linux", target_os = "android"))] use libc::S_IFIFO; #[cfg(any(target_os = "linux", target_os = "android"))] -use nix::fcntl::{splice, SpliceFFlags}; -#[cfg(any(target_os = "linux", target_os = "android"))] -use nix::unistd::pipe; +use uucore::pipes::{pipe, splice, splice_exact}; const BUF_SIZE: usize = 16 * 1024; #[cfg(any(target_os = "linux", target_os = "android"))] const SPLICE_SIZE: usize = 128 * 1024; -/// Splice wrapper which handles short writes -#[cfg(any(target_os = "linux", target_os = "android"))] -#[inline] -fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> { - let mut left = num_bytes; - loop { - let written = splice(read_fd, None, write_fd, None, left, SpliceFFlags::empty())?; - left -= written; - if left == 0 { - break; - } - } - Ok(()) -} - /// This is a Linux-specific function to count the number of bytes using the /// `splice` system call, which is faster than using `read`. /// @@ -46,13 +29,14 @@ fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Resul /// caller will fall back to a simpler method. #[inline] #[cfg(any(target_os = "linux", target_os = "android"))] -fn count_bytes_using_splice(fd: RawFd) -> Result { +fn count_bytes_using_splice(fd: &impl AsRawFd) -> Result { let null_file = OpenOptions::new() .write(true) .open("/dev/null") .map_err(|_| 0_usize)?; - let null = null_file.as_raw_fd(); - let null_rdev = stat::fstat(null).map_err(|_| 0_usize)?.st_rdev; + let null_rdev = stat::fstat(null_file.as_raw_fd()) + .map_err(|_| 0_usize)? + .st_rdev; if (stat::major(null_rdev), stat::minor(null_rdev)) != (1, 3) { // This is not a proper /dev/null, writing to it is probably bad // Bit of an edge case, but it has been known to happen @@ -60,17 +44,13 @@ fn count_bytes_using_splice(fd: RawFd) -> Result { } let (pipe_rd, pipe_wr) = pipe().map_err(|_| 0_usize)?; - // Ensure the pipe is closed when the function returns. - // SAFETY: The file descriptors do not have other owners. - let _handles = unsafe { (File::from_raw_fd(pipe_rd), File::from_raw_fd(pipe_wr)) }; - let mut byte_count = 0; loop { - match splice(fd, None, pipe_wr, None, SPLICE_SIZE, SpliceFFlags::empty()) { + match splice(fd, &pipe_wr, SPLICE_SIZE) { Ok(0) => break, Ok(res) => { byte_count += res; - if splice_exact(pipe_rd, null, res).is_err() { + if splice_exact(&pipe_rd, &null_file, res).is_err() { return Err(byte_count); } } @@ -106,7 +86,7 @@ pub(crate) fn count_bytes_fast(handle: &mut T) -> (usize, Opti // Else, if we're on Linux and our file is a FIFO pipe // (or stdin), we use splice to count the number of bytes. if (stat.st_mode & S_IFIFO) != 0 { - match count_bytes_using_splice(fd) { + match count_bytes_using_splice(handle) { Ok(n) => return (n, None), Err(n) => byte_count = n, } diff --git a/src/uu/yes/Cargo.toml b/src/uu/yes/Cargo.toml index b963d4974..ea2aae3b1 100644 --- a/src/uu/yes/Cargo.toml +++ b/src/uu/yes/Cargo.toml @@ -16,7 +16,7 @@ path = "src/yes.rs" [dependencies] clap = { version = "2.33", features = ["wrap_help"] } -uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } +uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["pipes"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] diff --git a/src/uu/yes/src/splice.rs b/src/uu/yes/src/splice.rs index b0573bc9e..6f025d6a9 100644 --- a/src/uu/yes/src/splice.rs +++ b/src/uu/yes/src/splice.rs @@ -16,18 +16,11 @@ //! make any effort to rescue data from the pipe if splice() fails, we can //! just fall back and start over from the beginning. -use std::{ - fs::File, - io, - os::unix::io::{AsRawFd, FromRawFd}, -}; +use std::{io, os::unix::io::AsRawFd}; -use nix::{ - errno::Errno, - fcntl::SpliceFFlags, - libc::S_IFIFO, - sys::{stat::fstat, uio::IoVec}, -}; +use nix::{errno::Errno, libc::S_IFIFO, sys::stat::fstat}; + +use uucore::pipes::{pipe, splice_exact, vmsplice}; pub(crate) fn splice_data(bytes: &[u8], out: &impl AsRawFd) -> Result<()> { let is_pipe = fstat(out.as_raw_fd())?.st_mode & S_IFIFO != 0; @@ -36,7 +29,7 @@ pub(crate) fn splice_data(bytes: &[u8], out: &impl AsRawFd) -> Result<()> { loop { let mut bytes = bytes; while !bytes.is_empty() { - let len = vmsplice(out, bytes)?; + let len = vmsplice(out, bytes).map_err(maybe_unsupported)?; bytes = &bytes[len..]; } } @@ -45,14 +38,8 @@ pub(crate) fn splice_data(bytes: &[u8], out: &impl AsRawFd) -> Result<()> { loop { let mut bytes = bytes; while !bytes.is_empty() { - let len = vmsplice(&write, bytes)?; - let mut remaining = len; - while remaining > 0 { - match splice(&read, out, remaining)? { - 0 => panic!("Unexpected end of pipe"), - n => remaining -= n, - }; - } + let len = vmsplice(&write, bytes).map_err(maybe_unsupported)?; + splice_exact(&read, out, len).map_err(maybe_unsupported)?; bytes = &bytes[len..]; } } @@ -81,30 +68,3 @@ fn maybe_unsupported(error: nix::Error) -> Error { _ => error.into(), } } - -fn splice(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Result { - nix::fcntl::splice( - source.as_raw_fd(), - None, - target.as_raw_fd(), - None, - len, - SpliceFFlags::empty(), - ) - .map_err(maybe_unsupported) -} - -fn vmsplice(target: &impl AsRawFd, bytes: &[u8]) -> Result { - nix::fcntl::vmsplice( - target.as_raw_fd(), - &[IoVec::from_slice(bytes)], - SpliceFFlags::empty(), - ) - .map_err(maybe_unsupported) -} - -fn pipe() -> nix::Result<(File, File)> { - let (read, write) = nix::unistd::pipe()?; - // SAFETY: The file descriptors do not have other owners. - unsafe { Ok((File::from_raw_fd(read), File::from_raw_fd(write))) } -} diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index a04f565aa..80a3a115b 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -31,6 +31,7 @@ data-encoding-macro = { version="0.1.12", optional=true } z85 = { version="3.0.3", optional=true } libc = { version="0.2.15", optional=true } once_cell = "1.8.0" +nix = { version="0.20", optional=true } [dev-dependencies] clap = "2.33.3" @@ -57,3 +58,4 @@ signals = [] utf8 = [] utmpx = ["time", "libc", "dns-lookup"] wide = [] +pipes = ["nix"] diff --git a/src/uucore/src/lib/features.rs b/src/uucore/src/lib/features.rs index 60be88664..42d8ffbe7 100644 --- a/src/uucore/src/lib/features.rs +++ b/src/uucore/src/lib/features.rs @@ -19,6 +19,8 @@ pub mod mode; pub mod entries; #[cfg(all(unix, feature = "perms"))] pub mod perms; +#[cfg(all(unix, feature = "pipes"))] +pub mod pipes; #[cfg(all(unix, feature = "process"))] pub mod process; diff --git a/src/uucore/src/lib/features/pipes.rs b/src/uucore/src/lib/features/pipes.rs new file mode 100644 index 000000000..dfb062992 --- /dev/null +++ b/src/uucore/src/lib/features/pipes.rs @@ -0,0 +1,54 @@ +/// Thin pipe-related wrappers around functions from the `nix` crate. +use std::fs::File; +#[cfg(any(target_os = "linux", target_os = "android"))] +use std::os::unix::io::AsRawFd; +use std::os::unix::io::FromRawFd; + +#[cfg(any(target_os = "linux", target_os = "android"))] +use nix::{fcntl::SpliceFFlags, sys::uio::IoVec}; + +pub use nix::{Error, Result}; + +/// A wrapper around [`nix::unistd::Pipe`] that ensures the pipe is cleaned up. +pub fn pipe() -> Result<(File, File)> { + let (read, write) = nix::unistd::pipe()?; + // SAFETY: The file descriptors do not have other owners. + unsafe { Ok((File::from_raw_fd(read), File::from_raw_fd(write))) } +} + +/// Less noisy wrapper around [`nix::fcntl::splice`]. +#[cfg(any(target_os = "linux", target_os = "android"))] +pub fn splice(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Result { + nix::fcntl::splice( + source.as_raw_fd(), + None, + target.as_raw_fd(), + None, + len, + SpliceFFlags::empty(), + ) +} + +/// Splice wrapper which fully finishes the write. +/// +/// Panics if `source` runs out of data before `len` bytes have been moved. +#[cfg(any(target_os = "linux", target_os = "android"))] +pub fn splice_exact(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Result<()> { + let mut left = len; + while left != 0 { + let written = splice(source, target, left)?; + assert_ne!(written, 0, "unexpected end of data"); + left -= written; + } + Ok(()) +} + +/// Use vmsplice() to copy data from memory into a pipe. +#[cfg(any(target_os = "linux", target_os = "android"))] +pub fn vmsplice(target: &impl AsRawFd, bytes: &[u8]) -> Result { + nix::fcntl::vmsplice( + target.as_raw_fd(), + &[IoVec::from_slice(bytes)], + SpliceFFlags::empty(), + ) +} diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 925246c24..79cc2afc3 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -49,6 +49,8 @@ pub use crate::features::mode; pub use crate::features::entries; #[cfg(all(unix, feature = "perms"))] pub use crate::features::perms; +#[cfg(all(unix, feature = "pipes"))] +pub use crate::features::pipes; #[cfg(all(unix, feature = "process"))] pub use crate::features::process; #[cfg(all(unix, not(target_os = "fuchsia"), feature = "signals"))] From d002810a47a9604f578377fead4ca2efe5eda158 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 20:10:28 +0200 Subject: [PATCH 15/21] cat: Do not assume complete writes --- src/uu/cat/src/splice.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs index 66630060c..5ce5f2b62 100644 --- a/src/uu/cat/src/splice.rs +++ b/src/uu/cat/src/splice.rs @@ -44,21 +44,23 @@ pub(super) fn write_fast_using_splice( } } -/// Caller must ensure that `num_bytes <= BUF_SIZE`, otherwise this function -/// will panic. The way we use this function in `write_fast_using_splice` -/// above is safe because `splice` is set to write at most `BUF_SIZE` to the -/// pipe. -#[inline] +/// Move exactly `num_bytes` bytes from `read_fd` to `write_fd`. +/// +/// Panics if not enough bytes can be read. fn copy_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> { let mut left = num_bytes; let mut buf = [0; BUF_SIZE]; - loop { - let read = unistd::read(read_fd, &mut buf[..left])?; - let written = unistd::write(write_fd, &buf[..read])?; - left -= written; - if left == 0 { - break; + while left > 0 { + let read = unistd::read(read_fd, &mut buf)?; + assert_ne!(read, 0, "unexpected end of pipe"); + let mut written = 0; + while written < read { + match unistd::write(write_fd, &buf[written..read])? { + 0 => panic!(), + n => written += n, + } } + left -= read; } Ok(()) } From 23647be07ab827491de4d9803c561e7e074bce7c Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 20:12:12 +0200 Subject: [PATCH 16/21] cat: Use larger splice size This raises performance by 10-20% or so. --- src/uu/cat/src/splice.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs index 5ce5f2b62..26802c7e6 100644 --- a/src/uu/cat/src/splice.rs +++ b/src/uu/cat/src/splice.rs @@ -5,6 +5,7 @@ use std::os::unix::io::{AsRawFd, RawFd}; use uucore::pipes::{pipe, splice, splice_exact}; +const SPLICE_SIZE: usize = 1024 * 128; const BUF_SIZE: usize = 1024 * 16; /// This function is called from `write_fast()` on Linux and Android. The @@ -22,7 +23,7 @@ pub(super) fn write_fast_using_splice( let (pipe_rd, pipe_wr) = pipe()?; loop { - match splice(&handle.reader, &pipe_wr, BUF_SIZE) { + match splice(&handle.reader, &pipe_wr, SPLICE_SIZE) { Ok(n) => { if n == 0 { return Ok(false); From b7d697753c5cf72b62443e206ca99ac8b6ccad99 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sat, 28 Aug 2021 00:38:00 +0200 Subject: [PATCH 17/21] unlink: Simplify, remove unsafe, move to core This makes it no longer possible to pass multiple filenames, but every other implementation I tried (GNU, busybox, FreeBSD, sbase) also forbids that so I think it's for the best. --- Cargo.lock | 1 - Cargo.toml | 2 +- src/uu/unlink/Cargo.toml | 1 - src/uu/unlink/src/unlink.rs | 98 ++++++------------------------------ tests/by-util/test_unlink.rs | 40 ++++++++++----- 5 files changed, 42 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 808f62e15..34b2dac52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3197,7 +3197,6 @@ name = "uu_unlink" version = "0.0.7" dependencies = [ "clap", - "libc", "uucore", "uucore_procs", ] diff --git a/Cargo.toml b/Cargo.toml index 3a2c5f12a..58b6aa52a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,7 @@ feat_common_core = [ "touch", "unexpand", "uniq", + "unlink", "wc", "yes", ] @@ -182,7 +183,6 @@ feat_require_unix = [ "timeout", "tty", "uname", - "unlink", ] # "feat_require_unix_utmpx" == set of utilities requiring unix utmp/utmpx support # * ref: diff --git a/src/uu/unlink/Cargo.toml b/src/uu/unlink/Cargo.toml index 3f13a7231..558d18422 100644 --- a/src/uu/unlink/Cargo.toml +++ b/src/uu/unlink/Cargo.toml @@ -16,7 +16,6 @@ path = "src/unlink.rs" [dependencies] clap = { version = "2.33", features = ["wrap_help"] } -libc = "0.2.42" uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/unlink/src/unlink.rs b/src/uu/unlink/src/unlink.rs index 7c66708e0..1b4e4c998 100644 --- a/src/uu/unlink/src/unlink.rs +++ b/src/uu/unlink/src/unlink.rs @@ -7,102 +7,32 @@ /* last synced with: unlink (GNU coreutils) 8.21 */ -// spell-checker:ignore (ToDO) lstat IFLNK IFMT IFREG - #[macro_use] extern crate uucore; -use clap::{crate_version, App, Arg}; -use libc::{lstat, stat, unlink}; -use libc::{S_IFLNK, S_IFMT, S_IFREG}; -use std::ffi::CString; -use std::io::{Error, ErrorKind}; -use uucore::display::Quotable; -use uucore::InvalidEncodingHandling; +use std::fs::remove_file; +use std::path::Path; -static ABOUT: &str = "Unlink the file at [FILE]."; +use clap::{crate_version, App, Arg}; + +use uucore::display::Quotable; +use uucore::error::{FromIo, UResult}; + +static ABOUT: &str = "Unlink the file at FILE."; static OPT_PATH: &str = "FILE"; -fn usage() -> String { - format!("{} [OPTION]... FILE", uucore::execution_phrase()) -} +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let matches = uu_app().get_matches_from(args); -pub fn uumain(args: impl uucore::Args) -> i32 { - let args = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); + let path: &Path = matches.value_of_os(OPT_PATH).unwrap().as_ref(); - let usage = usage(); - - let matches = uu_app().usage(&usage[..]).get_matches_from(args); - - let paths: Vec = matches - .values_of(OPT_PATH) - .map(|v| v.map(ToString::to_string).collect()) - .unwrap_or_default(); - - if paths.is_empty() { - crash!( - 1, - "missing operand\nTry '{0} --help' for more information.", - uucore::execution_phrase() - ); - } else if paths.len() > 1 { - crash!( - 1, - "extra operand: '{1}'\nTry '{0} --help' for more information.", - uucore::execution_phrase(), - paths[1] - ); - } - - let c_string = CString::new(paths[0].clone()).unwrap(); // unwrap() cannot fail, the string comes from argv so it cannot contain a \0. - - let st_mode = { - #[allow(deprecated)] - let mut buf: stat = unsafe { std::mem::uninitialized() }; - let result = unsafe { lstat(c_string.as_ptr(), &mut buf as *mut stat) }; - - if result < 0 { - crash!( - 1, - "Cannot stat {}: {}", - paths[0].quote(), - Error::last_os_error() - ); - } - - buf.st_mode & S_IFMT - }; - - let result = if st_mode != S_IFREG && st_mode != S_IFLNK { - Err(Error::new( - ErrorKind::Other, - "Not a regular file or symlink", - )) - } else { - let result = unsafe { unlink(c_string.as_ptr()) }; - - if result < 0 { - Err(Error::last_os_error()) - } else { - Ok(()) - } - }; - - match result { - Ok(_) => (), - Err(e) => { - crash!(1, "cannot unlink '{0}': {1}", paths[0], e); - } - } - - 0 + remove_file(path).map_err_context(|| format!("cannot unlink {}", path.quote())) } pub fn uu_app() -> App<'static, 'static> { App::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) - .arg(Arg::with_name(OPT_PATH).hidden(true).multiple(true)) + .arg(Arg::with_name(OPT_PATH).required(true).hidden(true)) } diff --git a/tests/by-util/test_unlink.rs b/tests/by-util/test_unlink.rs index 36c978734..6b4fc41da 100644 --- a/tests/by-util/test_unlink.rs +++ b/tests/by-util/test_unlink.rs @@ -23,23 +23,24 @@ fn test_unlink_multiple_files() { at.touch(file_a); at.touch(file_b); - ucmd.arg(file_a).arg(file_b).fails().stderr_is(&format!( - "{0}: extra operand: 'test_unlink_multiple_file_b'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + ucmd.arg(file_a) + .arg(file_b) + .fails() + .stderr_contains("USAGE"); } #[test] fn test_unlink_directory() { let (at, mut ucmd) = at_and_ucmd!(); - let dir = "test_unlink_empty_directory"; + let dir = "dir"; at.mkdir(dir); - ucmd.arg(dir).fails().stderr_is( - "unlink: cannot unlink 'test_unlink_empty_directory': Not a regular file \ - or symlink\n", + let res = ucmd.arg(dir).fails(); + let stderr = res.stderr_str(); + assert!( + stderr == "unlink: cannot unlink 'dir': Is a directory\n" + || stderr == "unlink: cannot unlink 'dir': Permission denied\n" ); } @@ -47,8 +48,21 @@ fn test_unlink_directory() { fn test_unlink_nonexistent() { let file = "test_unlink_nonexistent"; - new_ucmd!().arg(file).fails().stderr_is( - "unlink: Cannot stat 'test_unlink_nonexistent': No such file or directory \ - (os error 2)\n", - ); + new_ucmd!() + .arg(file) + .fails() + .stderr_is("unlink: cannot unlink 'test_unlink_nonexistent': No such file or directory\n"); +} + +#[test] +fn test_unlink_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.touch("foo"); + at.symlink_file("foo", "bar"); + + ucmd.arg("bar").succeeds().no_stderr(); + + assert!(at.file_exists("foo")); + assert!(!at.file_exists("bar")); } From d6a84851159cfe9025baef192598b378d2425e71 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 10 Sep 2021 22:03:51 +0200 Subject: [PATCH 18/21] uucore::pipes: Expand documentation --- src/uucore/src/lib/features/pipes.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/pipes.rs b/src/uucore/src/lib/features/pipes.rs index dfb062992..b375982dd 100644 --- a/src/uucore/src/lib/features/pipes.rs +++ b/src/uucore/src/lib/features/pipes.rs @@ -10,6 +10,9 @@ use nix::{fcntl::SpliceFFlags, sys::uio::IoVec}; pub use nix::{Error, Result}; /// A wrapper around [`nix::unistd::Pipe`] that ensures the pipe is cleaned up. +/// +/// Returns two `File` objects: everything written to the second can be read +/// from the first. pub fn pipe() -> Result<(File, File)> { let (read, write) = nix::unistd::pipe()?; // SAFETY: The file descriptors do not have other owners. @@ -17,6 +20,14 @@ pub fn pipe() -> Result<(File, File)> { } /// Less noisy wrapper around [`nix::fcntl::splice`]. +/// +/// Up to `len` bytes are moved from `source` to `target`. Returns the number +/// of successfully moved bytes. +/// +/// At least one of `source` and `target` must be some sort of pipe. +/// To get around this requirement, consider splicing from your source into +/// a [`pipe`] and then from the pipe into your target (with `splice_exact`): +/// this is still very efficient. #[cfg(any(target_os = "linux", target_os = "android"))] pub fn splice(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Result { nix::fcntl::splice( @@ -31,6 +42,8 @@ pub fn splice(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Resul /// Splice wrapper which fully finishes the write. /// +/// Exactly `len` bytes are moved from `source` into `target`. +/// /// Panics if `source` runs out of data before `len` bytes have been moved. #[cfg(any(target_os = "linux", target_os = "android"))] pub fn splice_exact(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Result<()> { @@ -43,7 +56,9 @@ pub fn splice_exact(source: &impl AsRawFd, target: &impl AsRawFd, len: usize) -> Ok(()) } -/// Use vmsplice() to copy data from memory into a pipe. +/// Copy data from `bytes` into `target`, which must be a pipe. +/// +/// Returns the number of successfully copied bytes. #[cfg(any(target_os = "linux", target_os = "android"))] pub fn vmsplice(target: &impl AsRawFd, bytes: &[u8]) -> Result { nix::fcntl::vmsplice( From 4a20ef1850603f6d78704a6bbd5be33cbae299d0 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sun, 12 Sep 2021 13:17:55 +0200 Subject: [PATCH 19/21] chcon: Remove unused Options.dereference to fix Clippy --- src/uu/chcon/src/chcon.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/uu/chcon/src/chcon.rs b/src/uu/chcon/src/chcon.rs index 98ebebf34..9664f69f5 100644 --- a/src/uu/chcon/src/chcon.rs +++ b/src/uu/chcon/src/chcon.rs @@ -281,7 +281,6 @@ pub fn uu_app() -> App<'static, 'static> { #[derive(Debug)] struct Options { verbose: bool, - dereference: bool, preserve_root: bool, recursive_mode: RecursiveMode, affect_symlink_referent: bool, @@ -331,9 +330,6 @@ fn parse_command_line(config: clap::App, args: impl uucore::Args) -> Result Result Date: Sun, 12 Sep 2021 12:24:38 -0400 Subject: [PATCH 20/21] wc: remove mutable min_width parameter Remove mutability from the `min_width` parameter to the `print_stats()` function in `wc`. --- src/uu/wc/src/wc.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index 6917eb137..16522a0a7 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -424,8 +424,15 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { // The width is the number of digits needed to print the number of // bytes in the largest file. This is true regardless of whether // the `settings` indicate that the bytes will be displayed. + // + // If we only need to display a single number, set this to 0 to + // prevent leading spaces. let mut failure = false; - let max_width = max_width(&inputs); + let max_width = if settings.number_enabled() <= 1 { + 0 + } else { + max_width(&inputs) + }; let mut total_word_count = WordCount::default(); @@ -475,20 +482,10 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { } } -fn print_stats( - settings: &Settings, - result: &TitledWordCount, - mut min_width: usize, -) -> io::Result<()> { +fn print_stats(settings: &Settings, result: &TitledWordCount, min_width: usize) -> io::Result<()> { let stdout = io::stdout(); let mut stdout_lock = stdout.lock(); - if settings.number_enabled() <= 1 { - // Prevent a leading space in case we only need to display a single - // number. - min_width = 0; - } - let mut is_first: bool = true; if settings.show_lines { From 3e0b3c93ef5765a271111c2d49c6acfe7b10c63e Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 18:19:50 +0200 Subject: [PATCH 21/21] mktemp: Do not use unsafe --- src/uu/mktemp/src/mktemp.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 59e82569a..81a3521e9 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -229,25 +229,24 @@ fn parse_template<'a>( pub fn dry_exec(mut tmpdir: PathBuf, prefix: &str, rand: usize, suffix: &str) -> UResult<()> { let len = prefix.len() + suffix.len() + rand; - let mut buf = String::with_capacity(len); - buf.push_str(prefix); - buf.extend(iter::repeat('X').take(rand)); - buf.push_str(suffix); + let mut buf = Vec::with_capacity(len); + buf.extend(prefix.as_bytes()); + buf.extend(iter::repeat(b'X').take(rand)); + buf.extend(suffix.as_bytes()); // Randomize. - unsafe { - // We guarantee utf8. - let bytes = &mut buf.as_mut_vec()[prefix.len()..prefix.len() + rand]; - rand::thread_rng().fill(bytes); - for byte in bytes.iter_mut() { - *byte = match *byte % 62 { - v @ 0..=9 => (v + b'0'), - v @ 10..=35 => (v - 10 + b'a'), - v @ 36..=61 => (v - 36 + b'A'), - _ => unreachable!(), - } + let bytes = &mut buf[prefix.len()..prefix.len() + rand]; + rand::thread_rng().fill(bytes); + for byte in bytes.iter_mut() { + *byte = match *byte % 62 { + v @ 0..=9 => (v + b'0'), + v @ 10..=35 => (v - 10 + b'a'), + v @ 36..=61 => (v - 36 + b'A'), + _ => unreachable!(), } } + // We guarantee utf8. + let buf = String::from_utf8(buf).unwrap(); tmpdir.push(buf); println_verbatim(tmpdir).map_err_context(|| "failed to print directory name".to_owned()) }