From 8885263ad50ad74163c8444f5da9e9b7ad1833f1 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 26 Dec 2021 18:25:34 +0100 Subject: [PATCH 01/10] cksum: use UIoError --- src/uu/cksum/src/cksum.rs | 37 ++++++-------------------------- src/uucore/src/lib/mods/error.rs | 30 +++++++++++++++++++++----- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index de47ea977..a2d5112ec 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -11,8 +11,7 @@ use std::fs::File; use std::io::{self, stdin, BufReader, Read}; use std::path::Path; use uucore::display::Quotable; -use uucore::error::UResult; -use uucore::error::USimpleError; +use uucore::error::{FromIo, UResult}; use uucore::show; use uucore::InvalidEncodingHandling; @@ -85,22 +84,7 @@ fn cksum(fname: &str) -> io::Result<(u32, usize)> { let mut rd: Box = match fname { "-" => Box::new(stdin()), _ => { - let path = &Path::new(fname); - if path.is_dir() { - return Err(std::io::Error::new( - io::ErrorKind::InvalidInput, - "Is a directory", - )); - }; - // Silent the warning as we want to the error message - #[allow(clippy::question_mark)] - if path.metadata().is_err() { - return Err(std::io::Error::new( - io::ErrorKind::NotFound, - "No such file or directory", - )); - }; - file = File::open(&path)?; + file = File::open(Path::new(fname))?; Box::new(BufReader::new(file)) } }; @@ -136,23 +120,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; if files.is_empty() { - match cksum("-") { - Ok((crc, size)) => println!("{} {}", crc, size), - Err(err) => { - return Err(USimpleError::new(2, format!("{}", err))); - } - } + let (crc, size) = cksum("-")?; + println!("{} {}", crc, size); return Ok(()); } for fname in &files { - match cksum(fname.as_ref()) { + match cksum(fname.as_ref()).map_err_context(|| format!("{}", fname.maybe_quote())) { Ok((crc, size)) => println!("{} {} {}", crc, size, fname), - Err(err) => show!(USimpleError::new( - 2, - format!("{}: {}", fname.maybe_quote(), err) - )), - } + Err(err) => show!(err), + }; } Ok(()) } diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index c04a0f2f1..37231576f 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -371,7 +371,7 @@ impl UError for UUsageError { /// ``` #[derive(Debug)] pub struct UIoError { - context: String, + context: Option, inner: std::io::Error, } @@ -379,7 +379,7 @@ impl UIoError { #[allow(clippy::new_ret_no_self)] pub fn new>(kind: std::io::ErrorKind, context: S) -> Box { Box::new(Self { - context: context.into(), + context: Some(context.into()), inner: kind.into(), }) } @@ -435,7 +435,11 @@ impl Display for UIoError { capitalize(&mut message); &message }; - write!(f, "{}: {}", self.context, message) + if let Some(ctx) = &self.context { + write!(f, "{}: {}", ctx, message) + } else { + write!(f, "{}", message) + } } } @@ -464,7 +468,7 @@ pub trait FromIo { impl FromIo> for std::io::Error { fn map_err_context(self, context: impl FnOnce() -> String) -> Box { Box::new(UIoError { - context: (context)(), + context: Some((context)()), inner: self, }) } @@ -479,12 +483,28 @@ impl FromIo> for std::io::Result { impl FromIo> for std::io::ErrorKind { fn map_err_context(self, context: impl FnOnce() -> String) -> Box { Box::new(UIoError { - context: (context)(), + context: Some((context)()), inner: std::io::Error::new(self, ""), }) } } +impl From for UIoError { + fn from(f: std::io::Error) -> UIoError { + UIoError { + context: None, + inner: f, + } + } +} + +impl From for Box { + fn from(f: std::io::Error) -> Box { + let u_error: UIoError = f.into(); + Box::new(u_error) as Box + } +} + /// Shorthand to construct [`UIoError`]-instances. /// /// This macro serves as a convenience call to quickly construct instances of From 7ae9e0a7eb65146f90cb90403af0914b110f60cd Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 26 Dec 2021 21:46:57 +0100 Subject: [PATCH 02/10] cksum: accept directories as empty files --- src/uu/cksum/src/cksum.rs | 12 +++++++++--- tests/by-util/test_cksum.rs | 5 ++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/uu/cksum/src/cksum.rs b/src/uu/cksum/src/cksum.rs index a2d5112ec..ca87da2a8 100644 --- a/src/uu/cksum/src/cksum.rs +++ b/src/uu/cksum/src/cksum.rs @@ -80,12 +80,18 @@ fn cksum(fname: &str) -> io::Result<(u32, usize)> { let mut crc = 0u32; let mut size = 0usize; - let file; let mut rd: Box = match fname { "-" => Box::new(stdin()), _ => { - file = File::open(Path::new(fname))?; - Box::new(BufReader::new(file)) + let p = Path::new(fname); + + // Directories should not give an error, but should be interpreted + // as empty files to match GNU semantics. + if p.is_dir() { + Box::new(BufReader::new(io::empty())) as Box + } else { + Box::new(BufReader::new(File::open(p)?)) as Box + } } }; diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index bf31ceb18..66bdba9e3 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -74,9 +74,8 @@ fn test_invalid_file() { at.mkdir(folder_name); ts.ucmd() .arg(folder_name) - .fails() - .no_stdout() - .stderr_contains("cksum: asdf: Is a directory"); + .succeeds() + .stdout_only("4294967295 0 asdf\n"); } // Make sure crc is correct for files larger than 32 bytes From b5522e1132053ff77d9051aa54c40543f401591a Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Wed, 29 Dec 2021 19:59:38 -0500 Subject: [PATCH 03/10] runcon: return UResult from uumain() function --- src/uu/runcon/src/errors.rs | 55 +++++++++++++++--- src/uu/runcon/src/runcon.rs | 107 +++++++++++++----------------------- 2 files changed, 85 insertions(+), 77 deletions(-) diff --git a/src/uu/runcon/src/errors.rs b/src/uu/runcon/src/errors.rs index 5f8258de0..082b55055 100644 --- a/src/uu/runcon/src/errors.rs +++ b/src/uu/runcon/src/errors.rs @@ -1,12 +1,22 @@ use std::ffi::OsString; -use std::fmt::Write; +use std::fmt::{Display, Formatter, Write}; use std::io; use std::str::Utf8Error; use uucore::display::Quotable; +use uucore::error::UError; pub(crate) type Result = std::result::Result; +// This list is NOT exhaustive. This command might perform an `execvp()` to run +// a different program. When that happens successfully, the exit status of this +// process will be the exit status of that program. +pub(crate) mod error_exit_status { + pub const NOT_FOUND: i32 = 127; + pub const COULD_NOT_EXECUTE: i32 = 126; + pub const ANOTHER_ERROR: i32 = libc::EXIT_FAILURE; +} + #[derive(thiserror::Error, Debug)] pub(crate) enum Error { #[error("No command is specified")] @@ -63,13 +73,44 @@ impl Error { } } -pub(crate) fn report_full_error(mut err: &dyn std::error::Error) -> String { - let mut desc = String::with_capacity(256); - write!(&mut desc, "{}", err).unwrap(); +pub(crate) fn write_full_error(writer: &mut W, err: &dyn std::error::Error) -> std::fmt::Result +where + W: Write, +{ + write!(writer, "{}", err)?; + let mut err = err; while let Some(source) = err.source() { err = source; - write!(&mut desc, ": {}", err).unwrap(); + write!(writer, ": {}", err)?; + } + write!(writer, ".")?; + Ok(()) +} + +#[derive(Debug)] +pub(crate) struct RunconError { + inner: Error, + code: i32, +} + +impl RunconError { + pub(crate) fn new(e: Error) -> RunconError { + RunconError::with_code(error_exit_status::ANOTHER_ERROR, e) + } + + pub(crate) fn with_code(code: i32, e: Error) -> RunconError { + RunconError { inner: e, code } + } +} + +impl std::error::Error for RunconError {} +impl UError for RunconError { + fn code(&self) -> i32 { + self.code + } +} +impl Display for RunconError { + fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { + write_full_error(f, &self.inner) } - desc.push('.'); - desc } diff --git a/src/uu/runcon/src/runcon.rs b/src/uu/runcon/src/runcon.rs index b2f1468bd..595cf3e65 100644 --- a/src/uu/runcon/src/runcon.rs +++ b/src/uu/runcon/src/runcon.rs @@ -1,6 +1,6 @@ // spell-checker:ignore (vars) RFILE -use uucore::{show_error, show_usage_error}; +use uucore::error::{UResult, UUsageError}; use clap::{App, Arg}; use selinux::{OpaqueSecurityContext, SecurityClass, SecurityContext}; @@ -13,7 +13,8 @@ use std::{io, ptr}; mod errors; -use errors::{report_full_error, Error, Result}; +use errors::error_exit_status; +use errors::{Error, Result, RunconError}; const VERSION: &str = env!("CARGO_PKG_VERSION"); const ABOUT: &str = "Run command with specified security context."; @@ -35,16 +36,6 @@ pub mod options { pub const RANGE: &str = "range"; } -// This list is NOT exhaustive. This command might perform an `execvp()` to run -// a different program. When that happens successfully, the exit status of this -// process will be the exit status of that program. -mod error_exit_status { - pub const SUCCESS: i32 = libc::EXIT_SUCCESS; - pub const NOT_FOUND: i32 = 127; - pub const COULD_NOT_EXECUTE: i32 = 126; - pub const ANOTHER_ERROR: i32 = libc::EXIT_FAILURE; -} - fn get_usage() -> String { format!( "{0} [CONTEXT COMMAND [ARG...]]\n \ @@ -53,7 +44,8 @@ fn get_usage() -> String { ) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = get_usage(); let config = uu_app().usage(usage.as_ref()); @@ -65,39 +57,28 @@ pub fn uumain(args: impl uucore::Args) -> i32 { match r.kind { clap::ErrorKind::HelpDisplayed | clap::ErrorKind::VersionDisplayed => { println!("{}", r); - return error_exit_status::SUCCESS; + return Ok(()); } _ => {} } } - - show_usage_error!("{}.\n", r); - return error_exit_status::ANOTHER_ERROR; + return Err(UUsageError::new( + error_exit_status::ANOTHER_ERROR, + format!("{}", r), + )); } }; match &options.mode { - CommandLineMode::Print => { - if let Err(r) = print_current_context() { - show_error!("{}", report_full_error(&r)); - return error_exit_status::ANOTHER_ERROR; - } - } - + CommandLineMode::Print => print_current_context().map_err(|e| RunconError::new(e).into()), CommandLineMode::PlainContext { context, command } => { - let (exit_status, err) = - if let Err(err) = get_plain_context(context).and_then(set_next_exec_context) { - (error_exit_status::ANOTHER_ERROR, err) - } else { - // On successful execution, the following call never returns, - // and this process image is replaced. - execute_command(command, &options.arguments) - }; - - show_error!("{}", report_full_error(&err)); - return exit_status; + get_plain_context(context) + .and_then(set_next_exec_context) + .map_err(RunconError::new)?; + // On successful execution, the following call never returns, + // and this process image is replaced. + execute_command(command, &options.arguments) } - CommandLineMode::CustomContext { compute_transition_context, user, @@ -106,34 +87,26 @@ pub fn uumain(args: impl uucore::Args) -> i32 { range, command, } => { - if let Some(command) = command { - let (exit_status, err) = if let Err(err) = get_custom_context( - *compute_transition_context, - user.as_deref(), - role.as_deref(), - the_type.as_deref(), - range.as_deref(), - command, - ) - .and_then(set_next_exec_context) - { - (error_exit_status::ANOTHER_ERROR, err) - } else { + match command { + Some(command) => { + get_custom_context( + *compute_transition_context, + user.as_deref(), + role.as_deref(), + the_type.as_deref(), + range.as_deref(), + command, + ) + .and_then(set_next_exec_context) + .map_err(RunconError::new)?; // On successful execution, the following call never returns, // and this process image is replaced. execute_command(command, &options.arguments) - }; - - show_error!("{}", report_full_error(&err)); - return exit_status; - } else if let Err(r) = print_current_context() { - show_error!("{}", report_full_error(&r)); - return error_exit_status::ANOTHER_ERROR; + } + None => print_current_context().map_err(|e| RunconError::new(e).into()), } } } - - error_exit_status::SUCCESS } pub fn uu_app() -> App<'static, 'static> { @@ -406,25 +379,19 @@ fn get_custom_context( Ok(osc) } -/// The actual return type of this function should be `Result` +/// The actual return type of this function should be `UResult`. /// However, until the *never* type is stabilized, one way to indicate to the /// compiler the only valid return type is to say "if this returns, it will /// always return an error". -fn execute_command(command: &OsStr, arguments: &[OsString]) -> (i32, Error) { - let c_command = match os_str_to_c_string(command) { - Ok(v) => v, - Err(r) => return (error_exit_status::ANOTHER_ERROR, r), - }; +fn execute_command(command: &OsStr, arguments: &[OsString]) -> UResult<()> { + let c_command = os_str_to_c_string(command).map_err(RunconError::new)?; - let argv_storage: Vec = match arguments + let argv_storage: Vec = arguments .iter() .map(AsRef::as_ref) .map(os_str_to_c_string) .collect::>() - { - Ok(v) => v, - Err(r) => return (error_exit_status::ANOTHER_ERROR, r), - }; + .map_err(RunconError::new)?; let mut argv: Vec<*const c_char> = Vec::with_capacity(arguments.len().saturating_add(2)); argv.push(c_command.as_ptr()); @@ -441,7 +408,7 @@ fn execute_command(command: &OsStr, arguments: &[OsString]) -> (i32, Error) { }; let err = Error::from_io1("Executing command", command, err); - (exit_status, err) + Err(RunconError::with_code(exit_status, err).into()) } fn os_str_to_c_string(s: &OsStr) -> Result { From 1f937b07602c4a7ee65d2bb538feb738f8c75334 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 30 Dec 2021 14:42:54 -0500 Subject: [PATCH 04/10] split: return UResult from uumain() function --- src/uu/split/src/platform/unix.rs | 2 + src/uu/split/src/split.rs | 88 ++++++++++++++----------------- 2 files changed, 43 insertions(+), 47 deletions(-) diff --git a/src/uu/split/src/platform/unix.rs b/src/uu/split/src/platform/unix.rs index a115d1959..448b8b782 100644 --- a/src/uu/split/src/platform/unix.rs +++ b/src/uu/split/src/platform/unix.rs @@ -2,6 +2,8 @@ use std::env; use std::io::Write; use std::io::{BufWriter, Result}; use std::process::{Child, Command, Stdio}; +use uucore::crash; + /// A writer that writes to a shell_process' stdin /// /// We use a shell process (not directly calling a sub-process) so we can forward the name of the diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index dfc116cb3..39e1cd594 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -7,9 +7,6 @@ // spell-checker:ignore (ToDO) PREFIXaa -#[macro_use] -extern crate uucore; - mod platform; use clap::{crate_version, App, Arg, ArgMatches}; @@ -20,6 +17,7 @@ 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::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::parse_size::parse_size; static OPT_BYTES: &str = "bytes"; @@ -53,7 +51,8 @@ size is 1000, and default PREFIX is 'x'. With no INPUT, or when INPUT is ) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let long_usage = get_long_usage(); @@ -83,15 +82,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { settings.additional_suffix = matches.value_of(OPT_ADDITIONAL_SUFFIX).unwrap().to_owned(); settings.verbose = matches.occurrences_of("verbose") > 0; - settings.strategy = Strategy::from(&matches); + settings.strategy = Strategy::from(&matches)?; settings.input = matches.value_of(ARG_INPUT).unwrap().to_owned(); settings.prefix = matches.value_of(ARG_PREFIX).unwrap().to_owned(); if matches.occurrences_of(OPT_FILTER) > 0 { if cfg!(windows) { // see https://github.com/rust-lang/rust/issues/29494 - show_error!("{} is currently not supported in this platform", OPT_FILTER); - exit!(-1); + return Err(USimpleError::new( + -1, + format!("{} is currently not supported in this platform", OPT_FILTER), + )); } else { settings.filter = Some(matches.value_of(OPT_FILTER).unwrap().to_owned()); } @@ -193,7 +194,7 @@ enum Strategy { impl Strategy { /// Parse a strategy from the command-line arguments. - fn from(matches: &ArgMatches) -> Self { + fn from(matches: &ArgMatches) -> UResult { // Check that the user is not specifying more than one strategy. // // Note: right now, this exact behavior cannot be handled by @@ -204,26 +205,26 @@ impl Strategy { matches.occurrences_of(OPT_BYTES), matches.occurrences_of(OPT_LINE_BYTES), ) { - (0, 0, 0) => Strategy::Lines(1000), + (0, 0, 0) => Ok(Strategy::Lines(1000)), (1, 0, 0) => { let s = matches.value_of(OPT_LINES).unwrap(); - let n = - parse_size(s).unwrap_or_else(|e| crash!(1, "invalid number of lines: {}", e)); - Strategy::Lines(n) + let n = parse_size(s) + .map_err(|e| USimpleError::new(1, format!("invalid number of lines: {}", e)))?; + Ok(Strategy::Lines(n)) } (0, 1, 0) => { let s = matches.value_of(OPT_BYTES).unwrap(); - let n = - parse_size(s).unwrap_or_else(|e| crash!(1, "invalid number of bytes: {}", e)); - Strategy::Bytes(n) + let n = parse_size(s) + .map_err(|e| USimpleError::new(1, format!("invalid number of bytes: {}", e)))?; + Ok(Strategy::Bytes(n)) } (0, 0, 1) => { let s = matches.value_of(OPT_LINE_BYTES).unwrap(); - let n = - parse_size(s).unwrap_or_else(|e| crash!(1, "invalid number of bytes: {}", e)); - Strategy::LineBytes(n) + let n = parse_size(s) + .map_err(|e| USimpleError::new(1, format!("invalid number of bytes: {}", e)))?; + Ok(Strategy::LineBytes(n)) } - _ => crash!(1, "cannot split in more than one way"), + _ => Err(UUsageError::new(1, "cannot split in more than one way")), } } } @@ -249,7 +250,7 @@ trait Splitter { &mut self, reader: &mut BufReader>, writer: &mut BufWriter>, - ) -> u128; + ) -> std::io::Result; } struct LineSplitter { @@ -269,21 +270,17 @@ impl Splitter for LineSplitter { &mut self, reader: &mut BufReader>, writer: &mut BufWriter>, - ) -> u128 { + ) -> std::io::Result { let mut bytes_consumed = 0u128; let mut buffer = String::with_capacity(1024); for _ in 0..self.lines_per_split { - let bytes_read = reader - .read_line(&mut buffer) - .unwrap_or_else(|_| crash!(1, "error reading bytes from input file")); + let bytes_read = reader.read_line(&mut buffer)?; // If we ever read 0 bytes then we know we've hit EOF. if bytes_read == 0 { - return bytes_consumed; + return Ok(bytes_consumed); } - writer - .write_all(buffer.as_bytes()) - .unwrap_or_else(|_| crash!(1, "error writing bytes to output file")); + writer.write_all(buffer.as_bytes())?; // Empty out the String buffer since `read_line` appends instead of // replaces. buffer.clear(); @@ -291,7 +288,7 @@ impl Splitter for LineSplitter { bytes_consumed += bytes_read as u128; } - bytes_consumed + Ok(bytes_consumed) } } @@ -312,7 +309,7 @@ impl Splitter for ByteSplitter { &mut self, reader: &mut BufReader>, writer: &mut BufWriter>, - ) -> u128 { + ) -> std::io::Result { // We buffer reads and writes. We proceed until `bytes_consumed` is // equal to `self.bytes_per_split` or we reach EOF. let mut bytes_consumed = 0u128; @@ -329,22 +326,18 @@ impl Splitter for ByteSplitter { // than BUFFER_SIZE in this branch. (self.bytes_per_split - bytes_consumed) as usize }; - let bytes_read = reader - .read(&mut buffer[0..bytes_desired]) - .unwrap_or_else(|_| crash!(1, "error reading bytes from input file")); + let bytes_read = reader.read(&mut buffer[0..bytes_desired])?; // If we ever read 0 bytes then we know we've hit EOF. if bytes_read == 0 { - return bytes_consumed; + return Ok(bytes_consumed); } - writer - .write_all(&buffer[0..bytes_read]) - .unwrap_or_else(|_| crash!(1, "error writing bytes to output file")); + writer.write_all(&buffer[0..bytes_read])?; bytes_consumed += bytes_read as u128; } - bytes_consumed + Ok(bytes_consumed) } } @@ -380,17 +373,16 @@ fn num_prefix(i: usize, width: usize) -> String { c } -fn split(settings: &Settings) -> i32 { +fn split(settings: &Settings) -> UResult<()> { let mut reader = BufReader::new(if settings.input == "-" { Box::new(stdin()) as Box } else { - let r = File::open(Path::new(&settings.input)).unwrap_or_else(|_| { - crash!( - 1, + let r = File::open(Path::new(&settings.input)).map_err_context(|| { + format!( "cannot open {} for reading: No such file or directory", settings.input.quote() ) - }); + })?; Box::new(r) as Box }); @@ -416,10 +408,12 @@ fn split(settings: &Settings) -> i32 { filename.push_str(settings.additional_suffix.as_ref()); let mut writer = platform::instantiate_current_writer(&settings.filter, filename.as_str()); - let bytes_consumed = splitter.consume(&mut reader, &mut writer); + let bytes_consumed = splitter + .consume(&mut reader, &mut writer) + .map_err_context(|| "input/output error".to_string())?; writer .flush() - .unwrap_or_else(|e| crash!(1, "error flushing to output file: {}", e)); + .map_err_context(|| "error flushing to output file".to_string())?; // If we didn't write anything we should clean up the empty file, and // break from the loop. @@ -428,12 +422,12 @@ fn split(settings: &Settings) -> i32 { // Complicated, I know... if settings.filter.is_none() { remove_file(filename) - .unwrap_or_else(|e| crash!(1, "error removing empty file: {}", e)); + .map_err_context(|| "error removing empty file".to_string())?; } break; } fileno += 1; } - 0 + Ok(()) } From 3339060ecedc7f0f791d7cc13d166c8893d4ea7d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 31 Dec 2021 14:57:00 -0500 Subject: [PATCH 05/10] tsort: return UResult from uumain() function --- src/uu/tsort/src/tsort.rs | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 11798db13..1b4f5bf49 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -5,16 +5,13 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. - -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; 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::error::{FromIo, UResult, USimpleError}; use uucore::InvalidEncodingHandling; static SUMMARY: &str = "Topological sort the strings in FILE. @@ -26,7 +23,8 @@ mod options { pub const FILE: &str = "file"; } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); @@ -43,13 +41,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { stdin_buf = stdin(); &mut stdin_buf as &mut dyn Read } else { - file_buf = match File::open(Path::new(&input)) { - Ok(a) => a, - _ => { - show_error!("{}: No such file or directory", input.maybe_quote()); - return 1; - } - }; + file_buf = File::open(Path::new(&input)).map_err_context(|| input.to_string())?; &mut file_buf as &mut dyn Read }); @@ -69,11 +61,15 @@ 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.maybe_quote() - ), + _ => { + return Err(USimpleError::new( + 1, + format!( + "{}: input contains an odd number of tokens", + input.maybe_quote() + ), + )) + } } } } @@ -84,14 +80,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { g.run_tsort(); if !g.is_acyclic() { - crash!(1, "{}, input contains a loop:", input); + return Err(USimpleError::new( + 1, + format!("{}, input contains a loop:", input), + )); } for x in &g.result { println!("{}", x); } - 0 + Ok(()) } pub fn uu_app() -> App<'static, 'static> { From cb92db322b905ae4a9526702e0a758e895323c2f Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 31 Dec 2021 14:42:15 -0500 Subject: [PATCH 06/10] timeout: return UResult from uumain() function --- src/uu/timeout/src/timeout.rs | 76 ++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index f686dde3b..42dd256ef 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -17,6 +17,7 @@ use std::io::ErrorKind; use std::process::{Command, Stdio}; use std::time::Duration; use uucore::display::Quotable; +use uucore::error::{UResult, USimpleError}; use uucore::process::ChildExt; use uucore::signals::{signal_by_name_or_value, signal_name_by_value}; use uucore::InvalidEncodingHandling; @@ -99,7 +100,8 @@ impl Config { } } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); @@ -188,32 +190,36 @@ fn timeout( foreground: bool, preserve_status: bool, verbose: bool, -) -> i32 { +) -> UResult<()> { if !foreground { unsafe { libc::setpgid(0, 0) }; } - let mut process = match Command::new(&cmd[0]) + let mut process = Command::new(&cmd[0]) .args(&cmd[1..]) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) .spawn() - { - Ok(p) => p, - Err(err) => { - show_error!("failed to execute process: {}", err); - if err.kind() == ErrorKind::NotFound { + .map_err(|err| { + let status_code = if err.kind() == ErrorKind::NotFound { // FIXME: not sure which to use - return 127; + 127 } else { // FIXME: this may not be 100% correct... - return 126; - } - } - }; + 126 + }; + USimpleError::new(status_code, format!("failed to execute process: {}", err)) + })?; unblock_sigchld(); match process.wait_or_timeout(duration) { - Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), + Ok(Some(status)) => { + let status_code = status.code().unwrap_or_else(|| status.signal().unwrap()); + if status_code == 0 { + Ok(()) + } else { + Err(status_code.into()) + } + } Ok(None) => { if verbose { show_error!( @@ -222,38 +228,50 @@ fn timeout( cmd[0].quote() ); } - crash_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); + process + .send_signal(signal) + .map_err(|e| USimpleError::new(ERR_EXIT_STATUS, format!("{}", e)))?; if let Some(kill_after) = kill_after { match process.wait_or_timeout(kill_after) { Ok(Some(status)) => { if preserve_status { - status.code().unwrap_or_else(|| status.signal().unwrap()) + let status_code = + status.code().unwrap_or_else(|| status.signal().unwrap()); + if status_code == 0 { + Ok(()) + } else { + Err(status_code.into()) + } } else { - 124 + Err(124.into()) } } Ok(None) => { if verbose { show_error!("sending signal KILL to command {}", cmd[0].quote()); } - crash_if_err!( - ERR_EXIT_STATUS, - process.send_signal( - uucore::signals::signal_by_name_or_value("KILL").unwrap() - ) - ); - crash_if_err!(ERR_EXIT_STATUS, process.wait()); - 137 + process + .send_signal(uucore::signals::signal_by_name_or_value("KILL").unwrap()) + .map_err(|e| USimpleError::new(ERR_EXIT_STATUS, format!("{}", e)))?; + process + .wait() + .map_err(|e| USimpleError::new(ERR_EXIT_STATUS, format!("{}", e)))?; + Err(137.into()) } - Err(_) => 124, + Err(_) => Err(124.into()), } } else { - 124 + Err(124.into()) } } Err(_) => { - crash_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); - ERR_EXIT_STATUS + // We're going to return ERR_EXIT_STATUS regardless of + // whether `send_signal()` succeeds or fails, so just + // ignore the return value. + process + .send_signal(signal) + .map_err(|e| USimpleError::new(ERR_EXIT_STATUS, format!("{}", e)))?; + Err(ERR_EXIT_STATUS.into()) } } } From c80e44fb0823acdf11eae7e26156f7802e938f70 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 2 Jan 2022 19:48:52 -0500 Subject: [PATCH 07/10] pr: return UResult from uumain() function --- src/uu/pr/src/pr.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 5ec61db8c..ea6fb86a8 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -25,6 +25,7 @@ use std::io::{stdin, stdout, BufRead, BufReader, Lines, Read, Write}; use std::os::unix::fs::FileTypeExt; use uucore::display::Quotable; +use uucore::error::UResult; type IOError = std::io::Error; @@ -174,7 +175,8 @@ pub fn uu_app() -> clap::App<'static, 'static> { clap::App::new(uucore::util_name()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(uucore::InvalidEncodingHandling::Ignore) .accept_any(); @@ -388,7 +390,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if matches.opt_present("version") { println!("{} {}", NAME, VERSION); - return 0; + return Ok(()); } let mut files = matches.free.clone(); @@ -412,7 +414,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(options) => options, Err(err) => { print_error(&matches, err); - return 1; + return Err(1.into()); } }; @@ -430,11 +432,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { _ => 0, }; if status != 0 { - return status; + return Err(status.into()); } } - - 0 + Ok(()) } /// Returns re-written arguments which are passed to the program. @@ -470,7 +471,7 @@ fn print_error(matches: &Matches, err: PrError) { } } -fn print_usage(opts: &mut getopts::Options, matches: &Matches) -> i32 { +fn print_usage(opts: &mut getopts::Options, matches: &Matches) -> UResult<()> { println!("{} {} -- print files", NAME, VERSION); println!(); println!( @@ -508,10 +509,9 @@ fn print_usage(opts: &mut getopts::Options, matches: &Matches) -> i32 { options::COLUMN_OPTION ); if matches.free.is_empty() { - return 1; + return Err(1.into()); } - - 0 + Ok(()) } fn parse_usize(matches: &Matches, opt: &str) -> Option> { From 49dca9adcb35bf503e4efbbaf8bb7444c28cc879 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 2 Jan 2022 19:59:15 -0500 Subject: [PATCH 08/10] realpath: return UResult from uumain() function --- src/uu/realpath/src/realpath.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index d13aed6c7..de8833341 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -17,6 +17,7 @@ use std::{ }; use uucore::{ display::{print_verbatim, Quotable}, + error::{FromIo, UResult}, fs::{canonicalize, MissingHandling, ResolveMode}, }; @@ -36,7 +37,8 @@ fn usage() -> String { format!("{0} [OPTION]... FILE...", 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); @@ -60,16 +62,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } else { MissingHandling::Normal }; - let mut retcode = 0; for path in &paths { - if let Err(e) = resolve_path(path, strip, zero, logical, can_mode) { - if !quiet { - show_error!("{}: {}", path.maybe_quote(), e); - } - retcode = 1 - }; + let result = resolve_path(path, strip, zero, logical, can_mode); + if !quiet { + show_if_err!(result.map_err_context(|| path.maybe_quote().to_string())); + } } - retcode + // Although we return `Ok`, it is possible that a call to + // `show!()` above has set the exit code for the program to a + // non-zero integer. + Ok(()) } pub fn uu_app() -> App<'static, 'static> { From 30b24255414dbd18cd804b28766248fb0eb5be13 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+electricboogie@users.noreply.github.com> Date: Thu, 6 Jan 2022 14:58:56 -0600 Subject: [PATCH 09/10] Fix newline when only dirs in base directory, and test --- src/uu/ls/src/ls.rs | 26 +++++++++++++------------- tests/by-util/test_ls.rs | 13 +++++++++++++ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 079dbfb94..78dcb68e1 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1366,8 +1366,16 @@ fn list(locs: Vec<&Path>, config: Config) -> UResult<()> { display_items(&files, &config, &mut out); - for dir in &dirs { - enter_directory(dir, &config, initial_locs_len, &mut out); + for (pos, dir) in dirs.iter().enumerate() { + // Print dir heading - name... 'total' comes after error display + if initial_locs_len > 1 || config.recursive { + if pos.eq(&0usize) && files.is_empty() { + let _ = writeln!(out, "{}:", dir.p_buf.display()); + } else { + let _ = writeln!(out, "\n{}:", dir.p_buf.display()); + } + } + enter_directory(dir, &config, &mut out); } Ok(()) @@ -1437,12 +1445,7 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { true } -fn enter_directory( - dir: &PathData, - config: &Config, - initial_locs_len: usize, - out: &mut BufWriter, -) { +fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) { // Create vec of entries with initial dot files let mut entries: Vec = if config.files == Files::All { vec![ @@ -1508,10 +1511,6 @@ fn enter_directory( sort_entries(&mut vec_path_data, config); entries.append(&mut vec_path_data); - // Print dir heading - name... - if initial_locs_len > 1 || config.recursive { - let _ = writeln!(out, "\n{}:", dir.p_buf.display()); - } // ...and total if config.format == Format::Long { display_total(&entries, config, out); @@ -1525,7 +1524,8 @@ fn enter_directory( .skip(if config.files == Files::All { 2 } else { 0 }) .filter(|p| p.file_type().map(|ft| ft.is_dir()).unwrap_or(false)) { - enter_directory(e, config, 0, out); + let _ = writeln!(out, "\n{}:", e.p_buf.display()); + enter_directory(e, config, out); } } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 4749e2c29..0e9801676 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -107,6 +107,19 @@ fn test_ls_io_errors() { .stdout_contains("some-dir4"); } +#[test] +fn test_ls_only_dirs_formatting() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("some-dir1"); + at.mkdir("some-dir2"); + at.mkdir("some-dir3"); + + scene.ucmd().arg("-1").arg("-R").succeeds().stdout_only( + ".:\nsome-dir1\nsome-dir2\nsome-dir3\n\n./some-dir1:\n\n./some-dir2:\n\n./some-dir3:\n", + ); +} + #[test] fn test_ls_walk_glob() { let scene = TestScenario::new(util_name!()); From 882b8ddd7bda0afcc5a8fde0ed7c044d691ec2ef Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+electricboogie@users.noreply.github.com> Date: Thu, 6 Jan 2022 15:41:53 -0600 Subject: [PATCH 10/10] Fix test dir names for Windows --- tests/by-util/test_ls.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 0e9801676..b156d9ffe 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -115,9 +115,18 @@ fn test_ls_only_dirs_formatting() { at.mkdir("some-dir2"); at.mkdir("some-dir3"); - scene.ucmd().arg("-1").arg("-R").succeeds().stdout_only( - ".:\nsome-dir1\nsome-dir2\nsome-dir3\n\n./some-dir1:\n\n./some-dir2:\n\n./some-dir3:\n", - ); + #[cfg(unix)] + { + scene.ucmd().arg("-1").arg("-R").succeeds().stdout_only( + ".:\nsome-dir1\nsome-dir2\nsome-dir3\n\n./some-dir1:\n\n./some-dir2:\n\n./some-dir3:\n", + ); + } + #[cfg(windows)] + { + scene.ucmd().arg("-1").arg("-R").succeeds().stdout_only( + ".:\nsome-dir1\nsome-dir2\nsome-dir3\n\n.\\some-dir1:\n\n.\\some-dir2:\n\n.\\some-dir3:\n", + ); + } } #[test]