diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 8bf6c091b..dad53f20c 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -90,3 +90,42 @@ jobs: with: name: gnu-result path: gnu-result.json + - name: Download the result + uses: dawidd6/action-download-artifact@v2 + with: + workflow: GnuTests.yml + name: gnu-result + repo: uutils/coreutils + branch: master + path: dl + - name: Download the log + uses: dawidd6/action-download-artifact@v2 + with: + workflow: GnuTests.yml + name: test-report + repo: uutils/coreutils + branch: master + path: dl + - name: Compare failing tests against master + shell: bash + run: | + OLD_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" dl/test-suite.log | sort) + NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" gnu/tests/test-suite.log | sort) + for LINE in $OLD_FAILING + do + if ! grep -Fxq $LINE<<<"$NEW_FAILING"; then + echo "::warning ::Congrats! The gnu test $LINE is now passing!" + fi + done + for LINE in $NEW_FAILING + do + if ! grep -Fxq $LINE<<<"$OLD_FAILING" + then + echo "::error ::GNU test failed: $LINE. $LINE is passing on 'master'. Maybe you have to rebase?" + fi + done + - name: Compare against master results + shell: bash + run: | + mv dl/gnu-result.json master-gnu-result.json + python uutils/util/compare_gnu_result.py diff --git a/Cargo.lock b/Cargo.lock index b954fffaf..8cf7cddcb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -713,9 +713,9 @@ checksum = "31a7a908b8f32538a2143e59a6e4e2508988832d5d4d6f7c156b3cbc762643a5" [[package]] name = "filetime" -version = "0.2.14" +version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8" +checksum = "975ccf83d8d9d0d84682850a38c8169027be83368805971cc4f238c2b245bc98" dependencies = [ "cfg-if 1.0.0", "libc", @@ -1631,9 +1631,9 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "selinux" -version = "0.1.3" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd525eeb189eb26c8471463186bba87644e3d8a9c7ae392adaf9ec45ede574bc" +checksum = "1aa2f705dd871c2eb90888bb2d44b13218b34f5c7318c3971df62f799d0143eb" dependencies = [ "bitflags", "libc", @@ -2023,6 +2023,7 @@ dependencies = [ "unix_socket", "uucore", "uucore_procs", + "winapi-util", ] [[package]] @@ -2032,7 +2033,7 @@ dependencies = [ "clap", "fts-sys", "libc", - "selinux-sys", + "selinux", "thiserror", "uucore", "uucore_procs", diff --git a/Cargo.toml b/Cargo.toml index df60206ca..abf6a42de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -241,7 +241,7 @@ clap = { version = "2.33", features = ["wrap_help"] } lazy_static = { version="1.3" } textwrap = { version="=0.11.0", features=["term_size"] } # !maint: [2020-05-10; rivy] unstable crate using undocumented features; pinned currently, will review uucore = { version=">=0.0.9", package="uucore", path="src/uucore" } -selinux = { version="0.1.3", optional = true } +selinux = { version="0.2.1", optional = true } # * uutils uu_test = { optional=true, version="0.0.7", package="uu_test", path="src/uu/test" } # diff --git a/README.md b/README.md index 1b2ddf67f..bc3cc0a98 100644 --- a/README.md +++ b/README.md @@ -365,25 +365,24 @@ To contribute to uutils, please see [CONTRIBUTING](CONTRIBUTING.md). | Done | Semi-Done | To Do | |-----------|-----------|--------| -| arch | cp | chcon | -| base32 | date | runcon | -| base64 | dd | stty | +| arch | cp | runcon | +| base32 | date | stty | +| base64 | dd | | | basename | df | | | basenc | expr | | | cat | install | | | chcon | join | | -| cat | ls | | -| chgrp | more | | -| chmod | numfmt | | -| chown | od (`--strings` and 128-bit data types missing) | | -| chroot | pr | | -| cksum | printf | | -| comm | sort | | -| csplit | split | | -| cut | tac | | -| dircolors | tail | | -| dirname | test | | -| du | | | +| chgrp | ls | | +| chmod | more | | +| chown | numfmt | | +| chroot | od (`--strings` and 128-bit data types missing) | | +| cksum | pr | | +| comm | printf | | +| csplit | sort | | +| cut | split | | +| dircolors | tac | | +| dirname | tail | | +| du | test | | | echo | | | | env | | | | expand | | | diff --git a/src/uu/cat/Cargo.toml b/src/uu/cat/Cargo.toml index b0721cee0..d80514385 100644 --- a/src/uu/cat/Cargo.toml +++ b/src/uu/cat/Cargo.toml @@ -23,9 +23,10 @@ uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_p [target.'cfg(unix)'.dependencies] unix_socket = "0.5.0" +nix = "0.20.0" -[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] -nix = "0.20" +[target.'cfg(windows)'.dependencies] +winapi-util = "0.1.5" [[bin]] name = "cat" diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index f340fa9fa..b9d07bcda 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -22,11 +22,14 @@ use std::io::{self, Read, Write}; use thiserror::Error; use uucore::error::UResult; +#[cfg(unix)] +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::{AsRawFd, RawFd}; +use std::os::unix::io::RawFd; /// Unix domain socket support #[cfg(unix)] @@ -59,6 +62,8 @@ enum CatError { }, #[error("Is a directory")] IsDirectory, + #[error("input file is output file")] + OutputIsInput, } type CatResult = Result; @@ -123,6 +128,12 @@ struct OutputState { /// Whether the output cursor is at the beginning of a new line at_line_start: bool, + + /// Whether we skipped a \r, which still needs to be printed + skipped_carriage_return: bool, + + /// Whether we have already printed a blank line + one_blank_kept: bool, } /// Represents an open file handle, stream, or other device @@ -297,7 +308,13 @@ fn cat_handle( } } -fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> CatResult<()> { +fn cat_path( + path: &str, + options: &OutputOptions, + state: &mut OutputState, + #[cfg(unix)] out_info: &nix::sys::stat::FileStat, + #[cfg(windows)] out_info: &winapi_util::file::Information, +) -> CatResult<()> { if path == "-" { let stdin = io::stdin(); let mut handle = InputHandle { @@ -324,6 +341,10 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat } _ => { let file = File::open(path)?; + #[cfg(any(windows, unix))] + if same_file(out_info, &file) { + return Err(CatError::OutputIsInput); + } let mut handle = InputHandle { #[cfg(any(target_os = "linux", target_os = "android"))] file_descriptor: file.as_raw_fd(), @@ -335,18 +356,42 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat } } +#[cfg(unix)] +fn same_file(a_info: &nix::sys::stat::FileStat, b: &File) -> bool { + let b_info = nix::sys::stat::fstat(b.as_raw_fd()).unwrap(); + b_info.st_size != 0 && b_info.st_dev == a_info.st_dev && b_info.st_ino == a_info.st_ino +} + +#[cfg(windows)] +fn same_file(a_info: &winapi_util::file::Information, b: &File) -> bool { + let b_info = winapi_util::file::information(b).unwrap(); + b_info.file_size() != 0 + && b_info.volume_serial_number() == a_info.volume_serial_number() + && b_info.file_index() == a_info.file_index() +} + fn cat_files(files: Vec, options: &OutputOptions) -> UResult<()> { + #[cfg(windows)] + let out_info = winapi_util::file::information(&std::io::stdout()).unwrap(); + #[cfg(unix)] + let out_info = nix::sys::stat::fstat(std::io::stdout().as_raw_fd()).unwrap(); + let mut state = OutputState { line_number: 1, at_line_start: true, + skipped_carriage_return: false, + one_blank_kept: false, }; let mut error_messages: Vec = Vec::new(); for path in &files { - if let Err(err) = cat_path(path, options, &mut state) { + if let Err(err) = cat_path(path, options, &mut state, &out_info) { error_messages.push(format!("{}: {}", path, err)); } } + if state.skipped_carriage_return { + print!("\r"); + } if error_messages.is_empty() { Ok(()) } else { @@ -424,7 +469,6 @@ fn write_lines( let mut in_buf = [0; 1024 * 31]; let stdout = io::stdout(); let mut writer = stdout.lock(); - let mut one_blank_kept = false; while let Ok(n) = handle.reader.read(&mut in_buf) { if n == 0 { @@ -435,8 +479,13 @@ fn write_lines( while pos < n { // skip empty line_number enumerating them if needed if in_buf[pos] == b'\n' { - if !state.at_line_start || !options.squeeze_blank || !one_blank_kept { - one_blank_kept = true; + // \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$ + if state.skipped_carriage_return && options.show_ends { + writer.write_all(b"^M")?; + state.skipped_carriage_return = false; + } + if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept { + state.one_blank_kept = true; if state.at_line_start && options.number == NumberingMode::All { write!(&mut writer, "{0:6}\t", state.line_number)?; state.line_number += 1; @@ -450,7 +499,12 @@ fn write_lines( pos += 1; continue; } - one_blank_kept = false; + if state.skipped_carriage_return { + writer.write_all(b"\r")?; + state.skipped_carriage_return = false; + state.at_line_start = false; + } + state.one_blank_kept = false; if state.at_line_start && options.number != NumberingMode::None { write!(&mut writer, "{0:6}\t", state.line_number)?; state.line_number += 1; @@ -465,17 +519,22 @@ fn write_lines( write_to_end(&in_buf[pos..], &mut writer) }; // end of buffer? - if offset == 0 { + if offset + pos == in_buf.len() { state.at_line_start = false; break; } - // print suitable end of line - writer.write_all(options.end_of_line().as_bytes())?; - if handle.is_interactive { - writer.flush()?; + if in_buf[pos + offset] == b'\r' { + state.skipped_carriage_return = true; + } else { + assert_eq!(in_buf[pos + offset], b'\n'); + // print suitable end of line + writer.write_all(options.end_of_line().as_bytes())?; + if handle.is_interactive { + writer.flush()?; + } + state.at_line_start = true; } - state.at_line_start = true; - pos += offset; + pos += offset + 1; } } @@ -483,17 +542,19 @@ fn write_lines( } // write***_to_end methods -// Write all symbols till end of line or end of buffer is reached -// Return the (number of written symbols + 1) or 0 if the end of buffer is reached +// Write all symbols till \n or \r or end of buffer is reached +// We need to stop at \r because it may be written as ^M depending on the byte after and settings; +// however, write_nonprint_to_end doesn't need to stop at \r because it will always write \r as ^M. +// Return the number of written symbols fn write_to_end(in_buf: &[u8], writer: &mut W) -> usize { - match in_buf.iter().position(|c| *c == b'\n') { + match in_buf.iter().position(|c| *c == b'\n' || *c == b'\r') { Some(p) => { writer.write_all(&in_buf[..p]).unwrap(); - p + 1 + p } None => { writer.write_all(in_buf).unwrap(); - 0 + in_buf.len() } } } @@ -501,20 +562,25 @@ fn write_to_end(in_buf: &[u8], writer: &mut W) -> usize { fn write_tab_to_end(mut in_buf: &[u8], writer: &mut W) -> usize { let mut count = 0; loop { - match in_buf.iter().position(|c| *c == b'\n' || *c == b'\t') { + match in_buf + .iter() + .position(|c| *c == b'\n' || *c == b'\t' || *c == b'\r') + { Some(p) => { writer.write_all(&in_buf[..p]).unwrap(); if in_buf[p] == b'\n' { - return count + p + 1; - } else { + return count + p; + } else if in_buf[p] == b'\t' { writer.write_all(b"^I").unwrap(); in_buf = &in_buf[p + 1..]; count += p + 1; + } else { + return count + p; } } None => { writer.write_all(in_buf).unwrap(); - return 0; + return in_buf.len(); } }; } @@ -539,11 +605,7 @@ fn write_nonprint_to_end(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> .unwrap(); count += 1; } - if count != in_buf.len() { - count + 1 - } else { - 0 - } + count } #[cfg(test)] diff --git a/src/uu/chcon/Cargo.toml b/src/uu/chcon/Cargo.toml index 7e7f82c3b..56fbef9ba 100644 --- a/src/uu/chcon/Cargo.toml +++ b/src/uu/chcon/Cargo.toml @@ -17,7 +17,7 @@ path = "src/chcon.rs" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version = ">=0.0.9", package="uucore", path="../../uucore", features=["entries", "fs", "perms"] } uucore_procs = { version = ">=0.0.6", package="uucore_procs", path="../../uucore_procs" } -selinux-sys = { version = "0.5" } +selinux = { version = "0.2" } fts-sys = { version = "0.2" } thiserror = { version = "1.0" } libc = { version = "0.2" } diff --git a/src/uu/chcon/src/chcon.rs b/src/uu/chcon/src/chcon.rs index 0e8b4b440..2a5efba46 100644 --- a/src/uu/chcon/src/chcon.rs +++ b/src/uu/chcon/src/chcon.rs @@ -5,14 +5,18 @@ use uucore::{executable, show_error, show_usage_error, show_warning}; use clap::{App, Arg}; +use selinux::{OpaqueSecurityContext, SecurityContext}; +use std::borrow::Cow; use std::ffi::{CStr, CString, OsStr, OsString}; -use std::fmt::Write; -use std::os::raw::{c_char, c_int}; +use std::os::raw::c_int; use std::path::{Path, PathBuf}; -use std::{fs, io, ptr, slice}; +use std::{fs, io}; -type Result = std::result::Result; +mod errors; +mod fts; + +use errors::*; static VERSION: &str = env!("CARGO_PKG_VERSION"); static ABOUT: &str = "Change the SELinux security context of each FILE to CONTEXT. \n\ @@ -81,15 +85,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let context = match &options.mode { CommandLineMode::ReferenceBased { reference } => { - let result = selinux::FileContext::new(reference, true) - .and_then(|r| { - if r.is_empty() { - Err(io::Error::from_raw_os_error(libc::ENODATA)) - } else { - Ok(r) - } - }) - .map_err(|r| Error::io1("Getting security context", reference, r)); + let result = match SecurityContext::of_path(reference, true, false) { + Ok(Some(context)) => Ok(context), + + Ok(None) => { + let err = io::Error::from_raw_os_error(libc::ENODATA); + Err(Error::from_io1("Getting security context", reference, err)) + } + + Err(r) => Err(Error::from_selinux("Getting security context", r)), + }; match result { Err(r) => { @@ -102,33 +107,24 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } CommandLineMode::ContextBased { context } => { - match selinux::SecurityContext::security_check_context(context) - .map_err(|r| Error::io1("Checking security context", context, r)) - { - Err(r) => { - show_error!("{}.", report_full_error(&r)); - return libc::EXIT_FAILURE; - } + let c_context = match os_str_to_c_string(context) { + Ok(context) => context, - Ok(Some(false)) => { + Err(_r) => { show_error!("Invalid security context '{}'.", context.to_string_lossy()); return libc::EXIT_FAILURE; } - - Ok(Some(true)) | Ok(None) => {} - } - - let c_context = if let Ok(value) = os_str_to_c_string(context) { - value - } else { - show_error!("Invalid security context '{}'.", context.to_string_lossy()); - return libc::EXIT_FAILURE; }; - SELinuxSecurityContext::String(c_context) + if SecurityContext::from_c_str(&c_context, false).check() == Some(false) { + show_error!("Invalid security context '{}'.", context.to_string_lossy()); + return libc::EXIT_FAILURE; + } + + SELinuxSecurityContext::String(Some(c_context)) } - CommandLineMode::Custom { .. } => SELinuxSecurityContext::default(), + CommandLineMode::Custom { .. } => SELinuxSecurityContext::String(None), }; let root_dev_ino = if options.preserve_root && options.recursive_mode.is_recursive() { @@ -282,65 +278,6 @@ pub fn uu_app() -> App<'static, 'static> { .arg(Arg::with_name("FILE").multiple(true).min_values(1)) } -fn report_full_error(mut err: &dyn std::error::Error) -> String { - let mut desc = String::with_capacity(256); - write!(&mut desc, "{}", err).unwrap(); - while let Some(source) = err.source() { - err = source; - write!(&mut desc, ". {}", err).unwrap(); - } - desc -} - -#[derive(thiserror::Error, Debug)] -enum Error { - #[error("No context is specified")] - MissingContext, - - #[error("No files are specified")] - MissingFiles, - - #[error("{0}")] - ArgumentsMismatch(String), - - #[error(transparent)] - CommandLine(#[from] clap::Error), - - #[error("{operation} failed")] - Io { - operation: &'static str, - source: io::Error, - }, - - #[error("{operation} failed on '{}'", .operand1.to_string_lossy())] - Io1 { - operation: &'static str, - operand1: OsString, - source: io::Error, - }, -} - -impl Error { - fn io1(operation: &'static str, operand1: impl Into, source: io::Error) -> Self { - Self::Io1 { - operation, - operand1: operand1.into(), - source, - } - } - - #[cfg(unix)] - fn io1_c_str(operation: &'static str, operand1: &CStr, source: io::Error) -> Self { - if operand1.to_bytes().is_empty() { - Self::Io { operation, source } - } else { - use std::os::unix::ffi::OsStrExt; - - Self::io1(operation, OsStr::from_bytes(operand1.to_bytes()), source) - } - } -} - #[derive(Debug)] struct Options { verbose: bool, @@ -500,39 +437,29 @@ fn process_files( root_dev_ino: Option<(libc::ino_t, libc::dev_t)>, ) -> Vec { let fts_options = options.recursive_mode.fts_open_options(); - let mut fts = match fts::FTS::new(options.files.iter(), fts_options, None) { + let mut fts = match fts::FTS::new(options.files.iter(), fts_options) { Ok(fts) => fts, - Err(source) => { - return vec![Error::Io { - operation: "fts_open()", - source, - }] - } + Err(err) => return vec![err], }; - let mut results = vec![]; - + let mut errors = Vec::default(); loop { match fts.read_next_entry() { Ok(true) => { if let Err(err) = process_file(options, context, &mut fts, root_dev_ino) { - results.push(err); + errors.push(err); } } Ok(false) => break, - Err(source) => { - results.push(Error::Io { - operation: "fts_read()", - source, - }); - + Err(err) => { + errors.push(err); break; } } } - results + errors } fn process_file( @@ -541,46 +468,40 @@ fn process_file( fts: &mut fts::FTS, root_dev_ino: Option<(libc::ino_t, libc::dev_t)>, ) -> Result<()> { - let entry = fts.last_entry_mut().unwrap(); + let mut entry = fts.last_entry_ref().unwrap(); - let file_full_name = if entry.fts_path.is_null() { - None - } else { - let fts_path_size = usize::from(entry.fts_pathlen).saturating_add(1); - - // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid. - let bytes = unsafe { slice::from_raw_parts(entry.fts_path.cast(), fts_path_size) }; - CStr::from_bytes_with_nul(bytes).ok() - } - .ok_or_else(|| Error::Io { - operation: "File name validation", - source: io::ErrorKind::InvalidInput.into(), + let file_full_name = entry.path().map(PathBuf::from).ok_or_else(|| { + Error::from_io("File name validation", io::ErrorKind::InvalidInput.into()) })?; - let fts_access_path = ptr_to_c_str(entry.fts_accpath) - .map_err(|r| Error::io1_c_str("File name validation", file_full_name, r))?; + let fts_access_path = entry.access_path().ok_or_else(|| { + let err = io::ErrorKind::InvalidInput.into(); + Error::from_io1("File name validation", &file_full_name, err) + })?; - let err = |s, k: io::ErrorKind| Error::io1_c_str(s, file_full_name, k.into()); + let err = |s, k: io::ErrorKind| Error::from_io1(s, &file_full_name, k.into()); let fts_err = |s| { - let r = io::Error::from_raw_os_error(entry.fts_errno); - Err(Error::io1_c_str(s, file_full_name, r)) + let r = io::Error::from_raw_os_error(entry.errno()); + Err(Error::from_io1(s, &file_full_name, r)) }; // SAFETY: If `entry.fts_statp` is not null, then is is assumed to be valid. - let file_dev_ino = unsafe { entry.fts_statp.as_ref() } - .map(|stat| (stat.st_ino, stat.st_dev)) - .ok_or_else(|| err("Getting meta data", io::ErrorKind::InvalidInput))?; + let file_dev_ino = if let Some(stat) = entry.stat() { + (stat.st_ino, stat.st_dev) + } else { + return Err(err("Getting meta data", io::ErrorKind::InvalidInput)); + }; let mut result = Ok(()); - match c_int::from(entry.fts_info) { + match entry.flags() { fts_sys::FTS_D => { if options.recursive_mode.is_recursive() { if root_dev_ino_check(root_dev_ino, file_dev_ino) { // This happens e.g., with "chcon -R --preserve-root ... /" // and with "chcon -RH --preserve-root ... symlink-to-root". - root_dev_ino_warn(file_full_name); + root_dev_ino_warn(&file_full_name); // Tell fts not to traverse into this hierarchy. let _ignored = fts.set(fts_sys::FTS_SKIP); @@ -607,8 +528,8 @@ fn process_file( // that modify permissions, it is possible that the file in question is accessible when // control reaches this point. So, if this is the first time we've seen the FTS_NS for // this file, tell fts_read to stat it "again". - if entry.fts_level == 0 && entry.fts_number == 0 { - entry.fts_number = 1; + if entry.level() == 0 && entry.number() == 0 { + entry.set_number(1); let _ignored = fts.set(fts_sys::FTS_AGAIN); return Ok(()); } @@ -621,8 +542,8 @@ fn process_file( fts_sys::FTS_DNR => result = fts_err("Reading directory"), fts_sys::FTS_DC => { - if cycle_warning_required(options.recursive_mode.fts_open_options(), entry) { - emit_cycle_warning(file_full_name); + if cycle_warning_required(options.recursive_mode.fts_open_options(), &entry) { + emit_cycle_warning(&file_full_name); return Err(err("Reading cyclic directory", io::ErrorKind::InvalidData)); } } @@ -630,11 +551,11 @@ fn process_file( _ => {} } - if c_int::from(entry.fts_info) == fts_sys::FTS_DP + if entry.flags() == fts_sys::FTS_DP && result.is_ok() && root_dev_ino_check(root_dev_ino, file_dev_ino) { - root_dev_ino_warn(file_full_name); + root_dev_ino_warn(&file_full_name); result = Err(err("Modifying root path", io::ErrorKind::PermissionDenied)); } @@ -656,24 +577,10 @@ fn process_file( result } -fn set_file_security_context( - path: &Path, - context: *const c_char, - follow_symbolic_links: bool, -) -> Result<()> { - let mut file_context = selinux::FileContext::from_ptr(context as *mut c_char); - if file_context.context.is_null() { - Err(io::Error::from(io::ErrorKind::InvalidInput)) - } else { - file_context.set_for_file(path, follow_symbolic_links) - } - .map_err(|r| Error::io1("Setting security context", path, r)) -} - fn change_file_context( options: &Options, context: &SELinuxSecurityContext, - file: &CStr, + path: &Path, ) -> Result<()> { match &options.mode { CommandLineMode::Custom { @@ -682,91 +589,88 @@ fn change_file_context( the_type, range, } => { - let path = PathBuf::from(c_str_to_os_string(file)); - let file_context = selinux::FileContext::new(&path, options.affect_symlink_referent) - .map_err(|r| Error::io1("Getting security context", &path, r))?; + let err0 = || -> Result<()> { + // If the file doesn't have a context, and we're not setting all of the context + // components, there isn't really an obvious default. Thus, we just give up. + let op = "Applying partial security context to unlabeled file"; + let err = io::ErrorKind::InvalidInput.into(); + Err(Error::from_io1(op, path, err)) + }; - // If the file doesn't have a context, and we're not setting all of the context - // components, there isn't really an obvious default. Thus, we just give up. - if file_context.is_empty() { - return Err(Error::io1( - "Applying partial security context to unlabeled file", - path, - io::ErrorKind::InvalidInput.into(), - )); - } + let file_context = + match SecurityContext::of_path(path, options.affect_symlink_referent, false) { + Ok(Some(context)) => context, - let mut se_context = selinux::SecurityContext::new(file_context.as_ptr()) - .map_err(|r| Error::io1("Creating security context", &path, r))?; + Ok(None) => return err0(), + Err(r) => return Err(Error::from_selinux("Getting security context", r)), + }; - if let Some(user) = user { - se_context - .set_user(user) - .map_err(|r| Error::io1("Setting security context user", &path, r))?; - } + let c_file_context = match file_context.to_c_string() { + Ok(Some(context)) => context, - if let Some(role) = role { - se_context - .set_role(role) - .map_err(|r| Error::io1("Setting security context role", &path, r))?; - } + Ok(None) => return err0(), + Err(r) => return Err(Error::from_selinux("Getting security context", r)), + }; - if let Some(the_type) = the_type { - se_context - .set_type(the_type) - .map_err(|r| Error::io1("Setting security context type", &path, r))?; - } + let se_context = + OpaqueSecurityContext::from_c_str(c_file_context.as_ref()).map_err(|_r| { + let err = io::ErrorKind::InvalidInput.into(); + Error::from_io1("Creating security context", path, err) + })?; - if let Some(range) = range { - se_context - .set_range(range) - .map_err(|r| Error::io1("Setting security context range", &path, r))?; + type SetValueProc = fn(&OpaqueSecurityContext, &CStr) -> selinux::errors::Result<()>; + + let list: &[(&Option, SetValueProc)] = &[ + (user, OpaqueSecurityContext::set_user), + (role, OpaqueSecurityContext::set_role), + (the_type, OpaqueSecurityContext::set_type), + (range, OpaqueSecurityContext::set_range), + ]; + + for (new_value, set_value_proc) in list { + if let Some(new_value) = new_value { + let c_new_value = os_str_to_c_string(new_value).map_err(|_r| { + let err = io::ErrorKind::InvalidInput.into(); + Error::from_io1("Creating security context", path, err) + })?; + + set_value_proc(&se_context, &c_new_value) + .map_err(|r| Error::from_selinux("Setting security context user", r))?; + } } let context_string = se_context - .str_bytes() - .map_err(|r| Error::io1("Getting security context", &path, r))?; + .to_c_string() + .map_err(|r| Error::from_selinux("Getting security context", r))?; - if !file_context.is_empty() && file_context.as_bytes() == context_string { + if c_file_context.as_ref().to_bytes() == context_string.as_ref().to_bytes() { Ok(()) // Nothing to change. } else { - set_file_security_context( - &path, - context_string.as_ptr().cast(), - options.affect_symlink_referent, - ) + SecurityContext::from_c_str(&context_string, false) + .set_for_path(path, options.affect_symlink_referent, false) + .map_err(|r| Error::from_selinux("Setting security context", r)) } } CommandLineMode::ReferenceBased { .. } | CommandLineMode::ContextBased { .. } => { - let path = PathBuf::from(c_str_to_os_string(file)); - let ctx_ptr = context.as_ptr() as *mut c_char; - set_file_security_context(&path, ctx_ptr, options.affect_symlink_referent) + if let Some(c_context) = context.to_c_string()? { + SecurityContext::from_c_str(c_context.as_ref(), false) + .set_for_path(path, options.affect_symlink_referent, false) + .map_err(|r| Error::from_selinux("Setting security context", r)) + } else { + let err = io::ErrorKind::InvalidInput.into(); + Err(Error::from_io1("Setting security context", path, err)) + } } } } #[cfg(unix)] -fn c_str_to_os_string(s: &CStr) -> OsString { - use std::os::unix::ffi::OsStringExt; - - OsString::from_vec(s.to_bytes().to_vec()) -} - -#[cfg(unix)] -pub(crate) fn os_str_to_c_string(s: &OsStr) -> io::Result { +pub(crate) fn os_str_to_c_string(s: &OsStr) -> Result { use std::os::unix::ffi::OsStrExt; - CString::new(s.as_bytes()).map_err(|_r| io::ErrorKind::InvalidInput.into()) -} - -/// SAFETY: -/// - If `p` is not null, then it is assumed to be a valid null-terminated C string. -/// - The returned `CStr` must not live more than the data pointed-to by `p`. -fn ptr_to_c_str<'s>(p: *const c_char) -> io::Result<&'s CStr> { - ptr::NonNull::new(p as *mut c_char) - .map(|p| unsafe { CStr::from_ptr(p.as_ptr()) }) - .ok_or_else(|| io::ErrorKind::InvalidInput.into()) + CString::new(s.as_bytes()) + .map_err(|_r| Error::from_io("CString::new()", io::ErrorKind::InvalidInput.into())) } /// Call `lstat()` to get the device and inode numbers for `/`. @@ -776,7 +680,7 @@ fn get_root_dev_ino() -> Result<(libc::ino_t, libc::dev_t)> { fs::symlink_metadata("/") .map(|md| (md.ino(), md.dev())) - .map_err(|r| Error::io1("std::fs::symlink_metadata", "/", r)) + .map_err(|r| Error::from_io1("std::fs::symlink_metadata", "/", r)) } fn root_dev_ino_check( @@ -786,8 +690,8 @@ fn root_dev_ino_check( root_dev_ino.map_or(false, |root_dev_ino| root_dev_ino == dir_dev_ino) } -fn root_dev_ino_warn(dir_name: &CStr) { - if dir_name.to_bytes() == b"/" { +fn root_dev_ino_warn(dir_name: &Path) { + if dir_name.as_os_str() == "/" { show_warning!( "It is dangerous to operate recursively on '/'. \ Use --{} to override this failsafe.", @@ -810,370 +714,37 @@ fn root_dev_ino_warn(dir_name: &CStr) { // However, when invoked with "-P -R", it deserves a warning. // The fts_options parameter records the options that control this aspect of fts's behavior, // so test that. -fn cycle_warning_required(fts_options: c_int, entry: &fts_sys::FTSENT) -> bool { +fn cycle_warning_required(fts_options: c_int, entry: &fts::EntryRef) -> bool { // When dereferencing no symlinks, or when dereferencing only those listed on the command line // and we're not processing a command-line argument, then a cycle is a serious problem. ((fts_options & fts_sys::FTS_PHYSICAL) != 0) - && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.fts_level != 0) + && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.level() != 0) } -fn emit_cycle_warning(file_name: &CStr) { +fn emit_cycle_warning(file_name: &Path) { show_warning!( "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.to_string_lossy() + file_name.display() ) } #[derive(Debug)] -enum SELinuxSecurityContext { - File(selinux::FileContext), - String(CString), +enum SELinuxSecurityContext<'t> { + File(SecurityContext<'t>), + String(Option), } -impl Default for SELinuxSecurityContext { - fn default() -> Self { - Self::String(CString::default()) - } -} - -impl SELinuxSecurityContext { - #[cfg(unix)] - fn as_ptr(&self) -> *const c_char { +impl<'t> SELinuxSecurityContext<'t> { + fn to_c_string(&self) -> Result>> { match self { - SELinuxSecurityContext::File(context) => context.as_ptr(), - SELinuxSecurityContext::String(context) => context.to_bytes_with_nul().as_ptr().cast(), - } - } -} - -mod fts { - use std::ffi::{CStr, CString, OsStr}; - use std::os::raw::c_int; - use std::{io, iter, ptr}; - - use super::os_str_to_c_string; - - pub(crate) type FTSOpenCallBack = unsafe extern "C" fn( - arg1: *mut *const fts_sys::FTSENT, - arg2: *mut *const fts_sys::FTSENT, - ) -> c_int; - - #[derive(Debug)] - pub(crate) struct FTS { - fts: ptr::NonNull, - entry: *mut fts_sys::FTSENT, - } - - impl FTS { - pub(crate) fn new( - paths: I, - options: c_int, - compar: Option, - ) -> io::Result - where - I: IntoIterator, - I::Item: AsRef, - { - let files_paths = paths - .into_iter() - .map(|s| os_str_to_c_string(s.as_ref())) - .collect::>>()?; - - if files_paths.is_empty() { - return Err(io::ErrorKind::InvalidInput.into()); - } - - let path_argv = files_paths - .iter() - .map(CString::as_ref) - .map(CStr::as_ptr) - .chain(iter::once(ptr::null())) - .collect::>(); - - // SAFETY: We assume calling fts_open() is safe: - // - `path_argv` is an array holding at least one path, and null-terminated. - let r = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, compar) }; - let fts = ptr::NonNull::new(r).ok_or_else(io::Error::last_os_error)?; - - Ok(Self { - fts, - entry: ptr::null_mut(), - }) - } - - pub(crate) fn read_next_entry(&mut self) -> io::Result { - // SAFETY: We assume calling fts_read() is safe with a non-null `fts` - // pointer assumed to be valid. - self.entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) }; - if self.entry.is_null() { - let r = io::Error::last_os_error(); - if let Some(0) = r.raw_os_error() { - Ok(false) - } else { - Err(r) - } - } else { - Ok(true) - } - } - - pub(crate) fn last_entry_mut(&mut self) -> Option<&mut fts_sys::FTSENT> { - // SAFETY: If `self.entry` is not null, then is is assumed to be valid. - unsafe { self.entry.as_mut() } - } - - pub(crate) fn set(&mut self, instr: c_int) -> io::Result<()> { - let fts = self.fts.as_ptr(); - - let entry = self - .last_entry_mut() - .ok_or_else(|| io::Error::from(io::ErrorKind::UnexpectedEof))?; - - // SAFETY: We assume calling fts_set() is safe with non-null `fts` - // and `entry` pointers assumed to be valid. - if unsafe { fts_sys::fts_set(fts, entry, instr) } == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } - } - } - - impl Drop for FTS { - fn drop(&mut self) { - // SAFETY: We assume calling fts_close() is safe with a non-null `fts` - // pointer assumed to be valid. - unsafe { fts_sys::fts_close(self.fts.as_ptr()) }; - } - } -} - -mod selinux { - use std::ffi::OsStr; - use std::os::raw::c_char; - use std::path::Path; - use std::{io, ptr, slice}; - - use super::os_str_to_c_string; - - #[derive(Debug)] - pub(crate) struct SecurityContext(ptr::NonNull); - - impl SecurityContext { - pub(crate) fn new(context_str: *const c_char) -> io::Result { - if context_str.is_null() { - Err(io::ErrorKind::InvalidInput.into()) - } else { - // SAFETY: We assume calling context_new() is safe with - // a non-null `context_str` pointer assumed to be valid. - let p = unsafe { selinux_sys::context_new(context_str) }; - ptr::NonNull::new(p) - .ok_or_else(io::Error::last_os_error) - .map(Self) - } - } - - pub(crate) fn is_selinux_enabled() -> bool { - // SAFETY: We assume calling is_selinux_enabled() is always safe. - unsafe { selinux_sys::is_selinux_enabled() != 0 } - } - - pub(crate) fn security_check_context(context: &OsStr) -> io::Result> { - let c_context = os_str_to_c_string(context)?; - - // SAFETY: We assume calling security_check_context() is safe with - // a non-null `context` pointer assumed to be valid. - if unsafe { selinux_sys::security_check_context(c_context.as_ptr()) } == 0 { - Ok(Some(true)) - } else if Self::is_selinux_enabled() { - Ok(Some(false)) - } else { - Ok(None) - } - } - - pub(crate) fn str_bytes(&self) -> io::Result<&[u8]> { - // SAFETY: We assume calling context_str() is safe with - // a non-null `context` pointer assumed to be valid. - let p = unsafe { selinux_sys::context_str(self.0.as_ptr()) }; - if p.is_null() { - Err(io::ErrorKind::InvalidInput.into()) - } else { - let len = unsafe { libc::strlen(p.cast()) }.saturating_add(1); - Ok(unsafe { slice::from_raw_parts(p.cast(), len) }) - } - } - - pub(crate) fn set_user(&mut self, user: &OsStr) -> io::Result<()> { - let c_user = os_str_to_c_string(user)?; - - // SAFETY: We assume calling context_user_set() is safe with non-null - // `context` and `user` pointers assumed to be valid. - if unsafe { selinux_sys::context_user_set(self.0.as_ptr(), c_user.as_ptr()) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } - } - - pub(crate) fn set_role(&mut self, role: &OsStr) -> io::Result<()> { - let c_role = os_str_to_c_string(role)?; - - // SAFETY: We assume calling context_role_set() is safe with non-null - // `context` and `role` pointers assumed to be valid. - if unsafe { selinux_sys::context_role_set(self.0.as_ptr(), c_role.as_ptr()) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } - } - - pub(crate) fn set_type(&mut self, the_type: &OsStr) -> io::Result<()> { - let c_type = os_str_to_c_string(the_type)?; - - // SAFETY: We assume calling context_type_set() is safe with non-null - // `context` and `the_type` pointers assumed to be valid. - if unsafe { selinux_sys::context_type_set(self.0.as_ptr(), c_type.as_ptr()) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } - } - - pub(crate) fn set_range(&mut self, range: &OsStr) -> io::Result<()> { - let c_range = os_str_to_c_string(range)?; - - // SAFETY: We assume calling context_range_set() is safe with non-null - // `context` and `range` pointers assumed to be valid. - if unsafe { selinux_sys::context_range_set(self.0.as_ptr(), c_range.as_ptr()) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } - } - } - - impl Drop for SecurityContext { - fn drop(&mut self) { - // SAFETY: We assume calling context_free() is safe with - // a non-null `context` pointer assumed to be valid. - unsafe { selinux_sys::context_free(self.0.as_ptr()) } - } - } - - #[derive(Debug)] - pub(crate) struct FileContext { - pub context: *mut c_char, - pub len: usize, - pub allocated: bool, - } - - impl FileContext { - pub(crate) fn new(path: &Path, follow_symbolic_links: bool) -> io::Result { - let c_path = os_str_to_c_string(path.as_os_str())?; - let mut context: *mut c_char = ptr::null_mut(); - - // SAFETY: We assume calling getfilecon()/lgetfilecon() is safe with - // non-null `path` and `context` pointers assumed to be valid. - let len = if follow_symbolic_links { - unsafe { selinux_sys::getfilecon(c_path.as_ptr(), &mut context) } - } else { - unsafe { selinux_sys::lgetfilecon(c_path.as_ptr(), &mut context) } - }; - - if len == -1 { - let err = io::Error::last_os_error(); - if let Some(libc::ENODATA) = err.raw_os_error() { - Ok(Self::default()) - } else { - Err(err) - } - } else if context.is_null() { - Ok(Self::default()) - } else { - Ok(Self { - context, - len: len as usize, - allocated: true, - }) - } - } - - pub(crate) fn from_ptr(context: *mut c_char) -> Self { - if context.is_null() { - Self::default() - } else { - // SAFETY: We assume calling strlen() is safe with a non-null - // `context` pointer assumed to be valid. - let len = unsafe { libc::strlen(context) }; - Self { - context, - len, - allocated: false, - } - } - } - - pub(crate) fn set_for_file( - &mut self, - path: &Path, - follow_symbolic_links: bool, - ) -> io::Result<()> { - let c_path = os_str_to_c_string(path.as_os_str())?; - - // SAFETY: We assume calling setfilecon()/lsetfilecon() is safe with - // non-null `path` and `context` pointers assumed to be valid. - let r = if follow_symbolic_links { - unsafe { selinux_sys::setfilecon(c_path.as_ptr(), self.context) } - } else { - unsafe { selinux_sys::lsetfilecon(c_path.as_ptr(), self.context) } - }; - - if r == -1 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } - } - - pub(crate) fn as_ptr(&self) -> *const c_char { - self.context - } - - pub(crate) fn is_empty(&self) -> bool { - self.context.is_null() || self.len == 0 - } - - pub(crate) fn as_bytes(&self) -> &[u8] { - if self.context.is_null() { - &[] - } else { - // SAFETY: `self.0.context` is a non-null pointer that is assumed to be valid. - unsafe { slice::from_raw_parts(self.context.cast(), self.len) } - } - } - } - - impl Default for FileContext { - fn default() -> Self { - Self { - context: ptr::null_mut(), - len: 0, - allocated: false, - } - } - } - - impl Drop for FileContext { - fn drop(&mut self) { - if self.allocated && !self.context.is_null() { - // SAFETY: We assume calling freecon() is safe with a non-null - // `context` pointer assumed to be valid. - unsafe { selinux_sys::freecon(self.context) } - } + Self::File(context) => context + .to_c_string() + .map_err(|r| Error::from_selinux("SELinuxSecurityContext::to_c_string()", r)), + + Self::String(context) => Ok(context.as_deref().map(Cow::Borrowed)), } } } diff --git a/src/uu/chcon/src/errors.rs b/src/uu/chcon/src/errors.rs new file mode 100644 index 000000000..ecbd7d409 --- /dev/null +++ b/src/uu/chcon/src/errors.rs @@ -0,0 +1,71 @@ +use std::ffi::OsString; +use std::fmt::Write; +use std::io; + +pub(crate) type Result = std::result::Result; + +#[derive(thiserror::Error, Debug)] +pub(crate) enum Error { + #[error("No context is specified")] + MissingContext, + + #[error("No files are specified")] + MissingFiles, + + #[error("{0}")] + ArgumentsMismatch(String), + + #[error(transparent)] + CommandLine(#[from] clap::Error), + + #[error("{operation} failed")] + SELinux { + operation: &'static str, + source: selinux::errors::Error, + }, + + #[error("{operation} failed")] + Io { + operation: &'static str, + source: io::Error, + }, + + #[error("{operation} failed on '{}'", .operand1.to_string_lossy())] + Io1 { + operation: &'static str, + operand1: OsString, + source: io::Error, + }, +} + +impl Error { + pub(crate) fn from_io(operation: &'static str, source: io::Error) -> Self { + Self::Io { operation, source } + } + + pub(crate) fn from_io1( + operation: &'static str, + operand1: impl Into, + source: io::Error, + ) -> Self { + Self::Io1 { + operation, + operand1: operand1.into(), + source, + } + } + + pub(crate) fn from_selinux(operation: &'static str, source: selinux::errors::Error) -> Self { + Self::SELinux { operation, source } + } +} + +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(); + while let Some(source) = err.source() { + err = source; + write!(&mut desc, ". {}", err).unwrap(); + } + desc +} diff --git a/src/uu/chcon/src/fts.rs b/src/uu/chcon/src/fts.rs new file mode 100644 index 000000000..89dd6184d --- /dev/null +++ b/src/uu/chcon/src/fts.rs @@ -0,0 +1,193 @@ +use std::ffi::{CStr, CString, OsStr}; +use std::marker::PhantomData; +use std::os::raw::{c_int, c_long, c_short}; +use std::path::Path; +use std::ptr::NonNull; +use std::{io, iter, ptr, slice}; + +use crate::errors::{Error, Result}; +use crate::os_str_to_c_string; + +#[derive(Debug)] +pub(crate) struct FTS { + fts: ptr::NonNull, + + entry: Option>, + _phantom_data: PhantomData, +} + +impl FTS { + pub(crate) fn new(paths: I, options: c_int) -> Result + where + I: IntoIterator, + I::Item: AsRef, + { + let files_paths: Vec = paths + .into_iter() + .map(|s| os_str_to_c_string(s.as_ref())) + .collect::>()?; + + if files_paths.is_empty() { + return Err(Error::from_io( + "FTS::new()", + io::ErrorKind::InvalidInput.into(), + )); + } + + let path_argv: Vec<_> = files_paths + .iter() + .map(CString::as_ref) + .map(CStr::as_ptr) + .chain(iter::once(ptr::null())) + .collect(); + + // SAFETY: We assume calling fts_open() is safe: + // - `path_argv` is an array holding at least one path, and null-terminated. + // - `compar` is None. + let fts = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, None) }; + + let fts = ptr::NonNull::new(fts) + .ok_or_else(|| Error::from_io("fts_open()", io::Error::last_os_error()))?; + + Ok(Self { + fts, + entry: None, + _phantom_data: PhantomData, + }) + } + + pub(crate) fn last_entry_ref(&mut self) -> Option { + self.entry.map(move |entry| EntryRef::new(self, entry)) + } + + pub(crate) fn read_next_entry(&mut self) -> Result { + // SAFETY: We assume calling fts_read() is safe with a non-null `fts` + // pointer assumed to be valid. + let new_entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) }; + + self.entry = NonNull::new(new_entry); + if self.entry.is_none() { + let r = io::Error::last_os_error(); + if let Some(0) = r.raw_os_error() { + Ok(false) + } else { + Err(Error::from_io("fts_read()", r)) + } + } else { + Ok(true) + } + } + + pub(crate) fn set(&mut self, instr: c_int) -> Result<()> { + let fts = self.fts.as_ptr(); + let entry = self + .entry + .ok_or_else(|| Error::from_io("FTS::set()", io::ErrorKind::UnexpectedEof.into()))?; + + // SAFETY: We assume calling fts_set() is safe with non-null `fts` + // and `entry` pointers assumed to be valid. + if unsafe { fts_sys::fts_set(fts, entry.as_ptr(), instr) } == -1 { + Err(Error::from_io("fts_set()", io::Error::last_os_error())) + } else { + Ok(()) + } + } +} + +impl Drop for FTS { + fn drop(&mut self) { + // SAFETY: We assume calling fts_close() is safe with a non-null `fts` + // pointer assumed to be valid. + unsafe { fts_sys::fts_close(self.fts.as_ptr()) }; + } +} + +#[derive(Debug)] +pub(crate) struct EntryRef<'fts> { + pub(crate) pointer: ptr::NonNull, + + _fts: PhantomData<&'fts FTS>, + _phantom_data: PhantomData, +} + +impl<'fts> EntryRef<'fts> { + fn new(_fts: &'fts FTS, entry: ptr::NonNull) -> Self { + Self { + pointer: entry, + _fts: PhantomData, + _phantom_data: PhantomData, + } + } + + fn as_ref(&self) -> &fts_sys::FTSENT { + // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid. + unsafe { self.pointer.as_ref() } + } + + fn as_mut(&mut self) -> &mut fts_sys::FTSENT { + // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid. + unsafe { self.pointer.as_mut() } + } + + pub(crate) fn flags(&self) -> c_int { + c_int::from(self.as_ref().fts_info) + } + + pub(crate) fn errno(&self) -> c_int { + self.as_ref().fts_errno + } + + pub(crate) fn level(&self) -> c_short { + self.as_ref().fts_level + } + + pub(crate) fn number(&self) -> c_long { + self.as_ref().fts_number + } + + pub(crate) fn set_number(&mut self, new_number: c_long) { + self.as_mut().fts_number = new_number; + } + + pub(crate) fn path(&self) -> Option<&Path> { + let entry = self.as_ref(); + if entry.fts_pathlen == 0 { + return None; + } + + NonNull::new(entry.fts_path) + .map(|path_ptr| { + let path_size = usize::from(entry.fts_pathlen).saturating_add(1); + + // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid. + unsafe { slice::from_raw_parts(path_ptr.as_ptr().cast(), path_size) } + }) + .and_then(|bytes| CStr::from_bytes_with_nul(bytes).ok()) + .map(c_str_to_os_str) + .map(Path::new) + } + + pub(crate) fn access_path(&self) -> Option<&Path> { + ptr::NonNull::new(self.as_ref().fts_accpath) + .map(|path_ptr| { + // SAFETY: `entry.fts_accpath` is a non-null pointer that is assumed to be valid. + unsafe { CStr::from_ptr(path_ptr.as_ptr()) } + }) + .map(c_str_to_os_str) + .map(Path::new) + } + + pub(crate) fn stat(&self) -> Option<&libc::stat> { + ptr::NonNull::new(self.as_ref().fts_statp).map(|stat_ptr| { + // SAFETY: `entry.fts_statp` is a non-null pointer that is assumed to be valid. + unsafe { stat_ptr.as_ref() } + }) + } +} + +#[cfg(unix)] +fn c_str_to_os_str(s: &CStr) -> &OsStr { + use std::os::unix::ffi::OsStrExt; + + OsStr::from_bytes(s.to_bytes()) +} diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index 4fb9ab489..dd851c504 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -161,12 +161,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Verbosity::Normal }; - let dest_gid: u32; - if let Some(file) = matches.value_of(options::REFERENCE) { + let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) { match fs::metadata(&file) { - Ok(meta) => { - dest_gid = meta.gid(); - } + Ok(meta) => Some(meta.gid()), Err(e) => { show_error!("failed to get attributes of '{}': {}", file, e); return 1; @@ -174,16 +171,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } else { let group = matches.value_of(options::ARG_GROUP).unwrap_or_default(); - match entries::grp2gid(group) { - Ok(g) => { - dest_gid = g; - } - _ => { - show_error!("invalid group: {}", group); - return 1; + if group.is_empty() { + None + } else { + match entries::grp2gid(group) { + Ok(g) => Some(g), + _ => { + show_error!("invalid group: {}", group); + return 1; + } } } - } + }; let executor = Chgrper { bit_flag, @@ -278,7 +277,7 @@ pub fn uu_app() -> App<'static, 'static> { } struct Chgrper { - dest_gid: gid_t, + dest_gid: Option, bit_flag: u8, verbosity: Verbosity, files: Vec, diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 53cf61e83..7df263c5d 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -14,7 +14,7 @@ use uucore::fs::resolve_relative_path; use uucore::libc::{gid_t, uid_t}; use uucore::perms::{wrap_chown, Verbosity}; -use uucore::error::{FromIo, UError, UResult, USimpleError}; +use uucore::error::{FromIo, UResult, USimpleError}; use clap::{crate_version, App, Arg}; @@ -107,10 +107,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if recursive { if bit_flag == FTS_PHYSICAL { if derefer == 1 { - return Err(USimpleError::new( - 1, - "-R --dereference requires -H or -L".to_string(), - )); + return Err(USimpleError::new(1, "-R --dereference requires -H or -L")); } derefer = 0; } @@ -324,7 +321,7 @@ impl Chowner { ret |= self.traverse(f); } if ret != 0 { - return Err(UError::from(ret)); + return Err(ret.into()); } Ok(()) } diff --git a/src/uu/csplit/src/split_name.rs b/src/uu/csplit/src/split_name.rs index 758216414..44ea2a5af 100644 --- a/src/uu/csplit/src/split_name.rs +++ b/src/uu/csplit/src/split_name.rs @@ -47,7 +47,7 @@ impl SplitName { }), Some(custom) => { let spec = - Regex::new(r"(?P%(?P[0#-])(?P\d+)?(?P[diuoxX]))") + Regex::new(r"(?P%((?P[0#-])(?P\d+)?)?(?P[diuoxX]))") .unwrap(); let mut captures_iter = spec.captures_iter(&custom); let custom_fn: Box String> = match captures_iter.next() { @@ -60,6 +60,21 @@ impl SplitName { Some(m) => m.as_str().parse::().unwrap(), }; match (captures.name("FLAG"), captures.name("TYPE")) { + (None, Some(ref t)) => match t.as_str() { + "d" | "i" | "u" => Box::new(move |n: usize| -> String { + format!("{}{}{}{}", prefix, before, n, after) + }), + "o" => Box::new(move |n: usize| -> String { + format!("{}{}{:o}{}", prefix, before, n, after) + }), + "x" => Box::new(move |n: usize| -> String { + format!("{}{}{:x}{}", prefix, before, n, after) + }), + "X" => Box::new(move |n: usize| -> String { + format!("{}{}{:X}{}", prefix, before, n, after) + }), + _ => return Err(CsplitError::SuffixFormatIncorrect), + }, (Some(ref f), Some(ref t)) => { match (f.as_str(), t.as_str()) { /* @@ -276,6 +291,12 @@ mod tests { assert_eq!(split_name.get(2), "xx00002"); } + #[test] + fn no_padding_decimal() { + let split_name = SplitName::new(None, Some(String::from("cst-%d-")), None).unwrap(); + assert_eq!(split_name.get(2), "xxcst-2-"); + } + #[test] fn zero_padding_decimal1() { let split_name = SplitName::new(None, Some(String::from("cst-%03d-")), None).unwrap(); diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index baa5fe292..cdfdf0b2d 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -8,7 +8,7 @@ #[macro_use] extern crate uucore; -use uucore::error::UCustomError; +use uucore::error::UError; use uucore::error::UResult; #[cfg(unix)] use uucore::fsext::statfs_fn; @@ -274,7 +274,7 @@ impl Display for DfError { impl Error for DfError {} -impl UCustomError for DfError { +impl UError for DfError { fn code(&self) -> i32 { match self { DfError::InvalidBaseValue(_) => 1, diff --git a/src/uu/dirname/src/dirname.rs b/src/uu/dirname/src/dirname.rs index 63ee57272..e7dcc2195 100644 --- a/src/uu/dirname/src/dirname.rs +++ b/src/uu/dirname/src/dirname.rs @@ -79,7 +79,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { print!("{}", separator); } } else { - return Err(UUsageError::new(1, "missing operand".to_string())); + return Err(UUsageError::new(1, "missing operand")); } Ok(()) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 9c05eb982..d85cc941c 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -32,7 +32,7 @@ use std::path::PathBuf; use std::str::FromStr; use std::time::{Duration, UNIX_EPOCH}; use std::{error::Error, fmt::Display}; -use uucore::error::{UCustomError, UResult}; +use uucore::error::{UError, UResult}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::InvalidEncodingHandling; #[cfg(windows)] @@ -438,7 +438,7 @@ Try '{} --help' for more information.", impl Error for DuError {} -impl UCustomError for DuError { +impl UError for DuError { fn code(&self) -> i32 { match self { Self::InvalidMaxDepthArg(_) => 1, diff --git a/src/uu/factor/src/cli.rs b/src/uu/factor/src/cli.rs index 0f5d21362..7963f162f 100644 --- a/src/uu/factor/src/cli.rs +++ b/src/uu/factor/src/cli.rs @@ -10,6 +10,7 @@ extern crate uucore; use std::error::Error; +use std::fmt::Write as FmtWrite; use std::io::{self, stdin, stdout, BufRead, Write}; mod factor; @@ -28,21 +29,29 @@ mod options { pub static NUMBER: &str = "NUMBER"; } -fn print_factors_str(num_str: &str, w: &mut impl io::Write) -> Result<(), Box> { - num_str - .parse::() - .map_err(|e| e.into()) - .and_then(|x| writeln!(w, "{}:{}", x, factor(x)).map_err(|e| e.into())) +fn print_factors_str( + num_str: &str, + w: &mut io::BufWriter, + factors_buffer: &mut String, +) -> Result<(), Box> { + num_str.parse::().map_err(|e| e.into()).and_then(|x| { + factors_buffer.clear(); + writeln!(factors_buffer, "{}:{}", x, factor(x))?; + w.write_all(factors_buffer.as_bytes())?; + Ok(()) + }) } pub fn uumain(args: impl uucore::Args) -> i32 { let matches = uu_app().get_matches_from(args); let stdout = stdout(); - let mut w = io::BufWriter::new(stdout.lock()); + // We use a smaller buffer here to pass a gnu test. 4KiB appears to be the default pipe size for bash. + let mut w = io::BufWriter::with_capacity(4 * 1024, stdout.lock()); + let mut factors_buffer = String::new(); if let Some(values) = matches.values_of(options::NUMBER) { for number in values { - if let Err(e) = print_factors_str(number, &mut w) { + if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { show_warning!("{}: {}", number, e); } } @@ -51,7 +60,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) { + if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) { show_warning!("{}: {}", number, e); } } diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index 2d64c8376..170788898 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -9,13 +9,12 @@ extern crate uucore; use clap::App; -use uucore::error::{UError, UResult}; -use uucore::executable; +use uucore::{error::UResult, executable}; #[uucore_procs::gen_uumain] pub fn uumain(args: impl uucore::Args) -> UResult<()> { uu_app().get_matches_from(args); - Err(UError::from(1)) + Err(1.into()) } pub fn uu_app() -> App<'static, 'static> { diff --git a/src/uu/id/Cargo.toml b/src/uu/id/Cargo.toml index 518de2729..b58c7fd78 100644 --- a/src/uu/id/Cargo.toml +++ b/src/uu/id/Cargo.toml @@ -18,7 +18,7 @@ path = "src/id.rs" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "process"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } -selinux = { version="0.1.3", optional = true } +selinux = { version="0.2.1", optional = true } [[bin]] name = "id" diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 9f92a4561..16c665273 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -40,10 +40,10 @@ extern crate uucore; use clap::{crate_version, App, Arg}; -#[cfg(all(target_os = "linux", feature = "selinux"))] -use selinux; use std::ffi::CStr; use uucore::entries::{self, Group, Locate, Passwd}; +use uucore::error::UResult; +use uucore::error::{set_exit_code, USimpleError}; pub use uucore::libc; use uucore::libc::{getlogin, uid_t}; use uucore::process::{getegid, geteuid, getgid, getuid}; @@ -123,10 +123,10 @@ struct State { // 1000 10 968 975 // +++ exited with 0 +++ user_specified: bool, - exit_code: i32, } -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 after_help = get_description(); @@ -161,7 +161,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }, user_specified: !users.is_empty(), ids: None, - exit_code: 0, }; let default_format = { @@ -170,14 +169,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; if (state.nflag || state.rflag) && default_format && !state.cflag { - crash!(1, "cannot print only names or real IDs in default format"); + return Err(USimpleError::new( + 1, + "cannot print only names or real IDs in default format", + )); } if state.zflag && default_format && !state.cflag { // NOTE: GNU test suite "id/zero.sh" needs this stderr output: - crash!(1, "option --zero not permitted in default format"); + return Err(USimpleError::new( + 1, + "option --zero not permitted in default format", + )); } if state.user_specified && state.cflag { - crash!(1, "cannot print security context when user specified"); + return Err(USimpleError::new( + 1, + "cannot print security context when user specified", + )); } let delimiter = { @@ -204,11 +212,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 { print!("{}{}", String::from_utf8_lossy(bytes), line_ending); } else { // print error because `cflag` was explicitly requested - crash!(1, "can't get process context"); + return Err(USimpleError::new(1, "can't get process context")); } - return state.exit_code; + return Ok(()); } else { - crash!(1, "--context (-Z) works only on an SELinux-enabled kernel"); + return Err(USimpleError::new( + 1, + "--context (-Z) works only on an SELinux-enabled kernel", + )); } } @@ -220,7 +231,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Ok(p) => Some(p), Err(_) => { show_error!("'{}': no such user", users[i]); - state.exit_code = 1; + set_exit_code(1); if i + 1 >= users.len() { break; } else { @@ -234,17 +245,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if matches.is_present(options::OPT_PASSWORD) { // BSD's `id` ignores all but the first specified user pline(possible_pw.map(|v| v.uid())); - return state.exit_code; + return Ok(()); }; if matches.is_present(options::OPT_HUMAN_READABLE) { // BSD's `id` ignores all but the first specified user pretty(possible_pw); - return state.exit_code; + return Ok(()); } if matches.is_present(options::OPT_AUDIT) { // BSD's `id` ignores specified users auditid(); - return state.exit_code; + return Ok(()); } let (uid, gid) = possible_pw.map(|p| (p.uid(), p.gid())).unwrap_or(( @@ -264,7 +275,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::gid2grp(gid).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", gid); - state.exit_code = 1; + set_exit_code(1); gid.to_string() }) } else { @@ -279,7 +290,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::uid2usr(uid).unwrap_or_else(|_| { show_error!("cannot find name for user ID {}", uid); - state.exit_code = 1; + set_exit_code(1); uid.to_string() }) } else { @@ -304,7 +315,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if state.nflag { entries::gid2grp(id).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", id); - state.exit_code = 1; + set_exit_code(1); id.to_string() }) } else { @@ -332,7 +343,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - state.exit_code + Ok(()) } pub fn uu_app() -> App<'static, 'static> { @@ -560,7 +571,7 @@ fn id_print(state: &mut State, groups: Vec) { uid, entries::uid2usr(uid).unwrap_or_else(|_| { show_error!("cannot find name for user ID {}", uid); - state.exit_code = 1; + set_exit_code(1); uid.to_string() }) ); @@ -569,7 +580,7 @@ fn id_print(state: &mut State, groups: Vec) { gid, entries::gid2grp(gid).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", gid); - state.exit_code = 1; + set_exit_code(1); gid.to_string() }) ); @@ -579,7 +590,7 @@ fn id_print(state: &mut State, groups: Vec) { euid, entries::uid2usr(euid).unwrap_or_else(|_| { show_error!("cannot find name for user ID {}", euid); - state.exit_code = 1; + set_exit_code(1); euid.to_string() }) ); @@ -590,7 +601,7 @@ fn id_print(state: &mut State, groups: Vec) { euid, entries::gid2grp(egid).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", egid); - state.exit_code = 1; + set_exit_code(1); egid.to_string() }) ); @@ -604,7 +615,7 @@ fn id_print(state: &mut State, groups: Vec) { gr, entries::gid2grp(gr).unwrap_or_else(|_| { show_error!("cannot find name for group ID {}", gr); - state.exit_code = 1; + set_exit_code(1); gr.to_string() }) )) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 81d44bb6c..cbbe9c18b 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.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) rwxr sourcepath targetpath +// spell-checker:ignore (ToDO) rwxr sourcepath targetpath Isnt uioerror mod mode; @@ -17,15 +17,17 @@ use file_diff::diff; use filetime::{set_file_times, FileTime}; use uucore::backup_control::{self, BackupMode}; use uucore::entries::{grp2gid, usr2uid}; +use uucore::error::{FromIo, UError, UIoError, UResult, USimpleError}; use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity}; use libc::{getegid, geteuid}; +use std::error::Error; +use std::fmt::{Debug, Display}; use std::fs; use std::fs::File; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use std::process::Command; -use std::result::Result; const DEFAULT_MODE: u32 = 0o755; const DEFAULT_STRIP_PROGRAM: &str = "strip"; @@ -47,6 +49,87 @@ pub struct Behavior { target_dir: Option, } +#[derive(Debug)] +enum InstallError { + Unimplemented(String), + DirNeedsArg(), + CreateDirFailed(PathBuf, std::io::Error), + ChmodFailed(PathBuf), + InvalidTarget(PathBuf), + TargetDirIsntDir(PathBuf), + BackupFailed(PathBuf, PathBuf, std::io::Error), + InstallFailed(PathBuf, PathBuf, std::io::Error), + StripProgramFailed(String), + MetadataFailed(std::io::Error), + NoSuchUser(String), + NoSuchGroup(String), + OmittingDirectory(PathBuf), +} + +impl UError for InstallError { + fn code(&self) -> i32 { + match self { + InstallError::Unimplemented(_) => 2, + _ => 1, + } + } + + fn usage(&self) -> bool { + false + } +} + +impl Error for InstallError {} + +impl Display for InstallError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use InstallError as IE; + match self { + IE::Unimplemented(opt) => write!(f, "Unimplemented feature: {}", opt), + IE::DirNeedsArg() => write!( + f, + "{} with -d requires at least one argument.", + executable!() + ), + IE::CreateDirFailed(dir, e) => { + Display::fmt(&uio_error!(e, "failed to create {}", dir.display()), f) + } + IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.display()), + IE::InvalidTarget(target) => write!( + f, + "invalid target {}: No such file or directory", + target.display() + ), + IE::TargetDirIsntDir(target) => { + write!(f, "target '{}' is not a directory", target.display()) + } + IE::BackupFailed(from, to, e) => Display::fmt( + &uio_error!( + e, + "cannot backup '{}' to '{}'", + from.display(), + to.display() + ), + f, + ), + IE::InstallFailed(from, to, e) => Display::fmt( + &uio_error!( + e, + "cannot install '{}' to '{}'", + from.display(), + to.display() + ), + 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()), + } + } +} + #[derive(Clone, Eq, PartialEq)] pub enum MainFunction { /// Create directories @@ -97,7 +180,8 @@ fn get_usage() -> String { /// /// Returns a program return code. /// -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 matches = uu_app().usage(&usage[..]).get_matches_from(args); @@ -107,17 +191,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - if let Err(s) = check_unimplemented(&matches) { - show_error!("Unimplemented feature: {}", s); - return 2; - } + check_unimplemented(&matches)?; - let behavior = match behavior(&matches) { - Ok(x) => x, - Err(ret) => { - return ret; - } - }; + let behavior = behavior(&matches)?; match behavior.main_function { MainFunction::Directory => directory(paths, behavior), @@ -269,13 +345,13 @@ pub fn uu_app() -> App<'static, 'static> { /// Error datum is a string of the unimplemented argument. /// /// -fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> { +fn check_unimplemented(matches: &ArgMatches) -> UResult<()> { if matches.is_present(OPT_NO_TARGET_DIRECTORY) { - Err("--no-target-directory, -T") + Err(InstallError::Unimplemented(String::from("--no-target-directory, -T")).into()) } else if matches.is_present(OPT_PRESERVE_CONTEXT) { - Err("--preserve-context, -P") + Err(InstallError::Unimplemented(String::from("--preserve-context, -P")).into()) } else if matches.is_present(OPT_CONTEXT) { - Err("--context, -Z") + Err(InstallError::Unimplemented(String::from("--context, -Z")).into()) } else { Ok(()) } @@ -289,7 +365,7 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> { /// /// In event of failure, returns an integer intended as a program return code. /// -fn behavior(matches: &ArgMatches) -> Result { +fn behavior(matches: &ArgMatches) -> UResult { let main_function = if matches.is_present(OPT_DIRECTORY) { MainFunction::Directory } else { @@ -314,10 +390,7 @@ fn behavior(matches: &ArgMatches) -> Result { matches.value_of(OPT_BACKUP), ); let backup_mode = match backup_mode { - Err(err) => { - show_usage_error!("{}", err); - return Err(1); - } + Err(err) => return Err(USimpleError::new(1, err)), Ok(mode) => mode, }; @@ -349,45 +422,46 @@ fn behavior(matches: &ArgMatches) -> Result { /// GNU man pages describe this functionality as creating 'all components of /// the specified directories'. /// -/// Returns an integer intended as a program return code. +/// Returns a Result type with the Err variant containing the error message. /// -fn directory(paths: Vec, b: Behavior) -> i32 { +fn directory(paths: Vec, b: Behavior) -> UResult<()> { if paths.is_empty() { - println!("{} with -d requires at least one argument.", executable!()); - 1 + Err(InstallError::DirNeedsArg().into()) } else { - let mut all_successful = true; - for path in paths.iter().map(Path::new) { // if the path already exist, don't try to create it again if !path.exists() { - // Differently than the primary functionality (MainFunction::Standard), the directory - // functionality should create all ancestors (or components) of a directory regardless - // of the presence of the "-D" flag. - // NOTE: the GNU "install" sets the expected mode only for the target directory. All - // created ancestor directories will have 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) { - show_error!("{}: {}", path.display(), e); - all_successful = false; + // Differently than the primary functionality + // (MainFunction::Standard), the directory functionality should + // create all ancestors (or components) of a directory + // regardless of the presence of the "-D" flag. + // + // NOTE: the GNU "install" sets the expected mode only for the + // target directory. All created ancestor directories will have + // 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())) + { + show!(e); continue; } if b.verbose { - show_error!("creating directory '{}'", path.display()); + println!("creating directory '{}'", path.display()); } } if mode::chmod(path, b.mode()).is_err() { - all_successful = false; + // Error messages are printed by the mode::chmod function! + uucore::error::set_exit_code(1); continue; } } - if all_successful { - 0 - } else { - 1 - } + // If the exit code was set, or show! has been called at least once + // (which sets the exit code as well), function execution will end after + // this return. + Ok(()) } } @@ -401,9 +475,9 @@ fn is_new_file_path(path: &Path) -> bool { /// Perform an install, given a list of paths and behavior. /// -/// Returns an integer intended as a program return code. +/// Returns a Result type with the Err variant containing the error message. /// -fn standard(mut paths: Vec, b: Behavior) -> i32 { +fn standard(mut paths: Vec, b: Behavior) -> UResult<()> { let target: PathBuf = b .target_dir .clone() @@ -418,25 +492,19 @@ fn standard(mut paths: Vec, b: Behavior) -> i32 { if let Some(parent) = target.parent() { if !parent.exists() && b.create_leading { if let Err(e) = fs::create_dir_all(parent) { - show_error!("failed to create {}: {}", parent.display(), e); - return 1; + return Err(InstallError::CreateDirFailed(parent.to_path_buf(), e).into()); } if mode::chmod(parent, b.mode()).is_err() { - show_error!("failed to chmod {}", parent.display()); - return 1; + return Err(InstallError::ChmodFailed(parent.to_path_buf()).into()); } } } if target.is_file() || is_new_file_path(&target) { - copy_file_to_file(&sources[0], &target, &b) + copy(&sources[0], &target, &b) } else { - show_error!( - "invalid target {}: No such file or directory", - target.display() - ); - 1 + Err(InstallError::InvalidTarget(target).into()) } } } @@ -444,34 +512,30 @@ fn standard(mut paths: Vec, b: Behavior) -> i32 { /// Copy some files into a directory. /// /// Prints verbose information and error messages. -/// Returns an integer intended as a program return code. +/// Returns a Result type with the Err variant containing the error message. /// /// # Parameters /// /// _files_ must all exist as non-directories. /// _target_dir_ must be a directory. /// -fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 { +fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UResult<()> { if !target_dir.is_dir() { - show_error!("target '{}' is not a directory", target_dir.display()); - return 1; + return Err(InstallError::TargetDirIsntDir(target_dir.to_path_buf()).into()); } - - let mut all_successful = true; for sourcepath in files.iter() { if !sourcepath.exists() { - show_error!( - "cannot stat '{}': No such file or directory", - sourcepath.display() + let err = UIoError::new( + std::io::ErrorKind::NotFound, + format!("cannot stat '{}'", sourcepath.display()), ); - - all_successful = false; + show!(err); continue; } if sourcepath.is_dir() { - show_error!("omitting directory '{}'", sourcepath.display()); - all_successful = false; + let err = InstallError::OmittingDirectory(sourcepath.to_path_buf()); + show!(err); continue; } @@ -479,37 +543,18 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3 let filename = sourcepath.components().last().unwrap(); targetpath.push(filename); - if copy(sourcepath, &targetpath, b).is_err() { - all_successful = false; - } - } - if all_successful { - 0 - } else { - 1 - } -} - -/// Copy a file to another file. -/// -/// Prints verbose information and error messages. -/// Returns an integer intended as a program return code. -/// -/// # Parameters -/// -/// _file_ must exist as a non-directory. -/// _target_ must be a non-directory -/// -fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 { - if copy(file, target, b).is_err() { - 1 - } else { - 0 + show_if_err!(copy(sourcepath, &targetpath, b)); } + // If the exit code was set, or show! has been called at least once + // (which sets the exit code as well), function execution will end after + // this return. + Ok(()) } /// Copy one file to a new location, changing metadata. /// +/// Returns a Result type with the Err variant containing the error message. +/// /// # Parameters /// /// _from_ must exist as a non-directory. @@ -520,8 +565,8 @@ fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 { /// If the copy system call fails, we print a verbose error and return an empty error value. /// #[allow(clippy::cognitive_complexity)] -fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { - if b.compare && !need_copy(from, to, b) { +fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { + if b.compare && !need_copy(from, to, b)? { return Ok(()); } // Declare the path here as we may need it for the verbose output below. @@ -536,13 +581,12 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { if let Some(ref backup_path) = backup_path { // TODO!! if let Err(err) = fs::rename(to, backup_path) { - show_error!( - "install: cannot backup file '{}' to '{}': {}", - to.display(), - backup_path.display(), - err - ); - return Err(()); + return Err(InstallError::BackupFailed( + to.to_path_buf(), + backup_path.to_path_buf(), + err, + ) + .into()); } } } @@ -552,52 +596,41 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { * https://github.com/rust-lang/rust/issues/79390 */ if let Err(err) = File::create(to) { - show_error!( - "install: cannot install '{}' to '{}': {}", - from.display(), - to.display(), - err + return Err( + InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(), ); - return Err(()); } } else if let Err(err) = fs::copy(from, to) { - show_error!( - "cannot install '{}' to '{}': {}", - from.display(), - to.display(), - err - ); - return Err(()); + return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into()); } if b.strip && cfg!(not(windows)) { match Command::new(&b.strip_program).arg(to).output() { Ok(o) => { if !o.status.success() { - crash!( - 1, - "strip program failed: {}", - String::from_utf8(o.stderr).unwrap_or_default() - ); + return Err(InstallError::StripProgramFailed( + String::from_utf8(o.stderr).unwrap_or_default(), + ) + .into()); } } - Err(e) => crash!(1, "strip program execution failed: {}", e), + Err(e) => return Err(InstallError::StripProgramFailed(e.to_string()).into()), } } if mode::chmod(to, b.mode()).is_err() { - return Err(()); + return Err(InstallError::ChmodFailed(to.to_path_buf()).into()); } if !b.owner.is_empty() { let meta = match fs::metadata(to) { Ok(meta) => meta, - Err(f) => crash!(1, "{}", f.to_string()), + Err(e) => return Err(InstallError::MetadataFailed(e).into()), }; let owner_id = match usr2uid(&b.owner) { Ok(g) => g, - _ => crash!(1, "no such user: {}", b.owner), + _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()), }; let gid = meta.gid(); match wrap_chown( @@ -620,14 +653,14 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { if !b.group.is_empty() { let meta = match fs::metadata(to) { Ok(meta) => meta, - Err(f) => crash!(1, "{}", f.to_string()), + Err(e) => return Err(InstallError::MetadataFailed(e).into()), }; let group_id = match grp2gid(&b.group) { Ok(g) => g, - _ => crash!(1, "no such group: {}", b.group), + _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()), }; - match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) { + match wrap_chgrp(to, &meta, Some(group_id), false, Verbosity::Normal) { Ok(n) => { if !n.is_empty() { show_error!("{}", n); @@ -640,7 +673,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { if b.preserve_timestamps { let meta = match fs::metadata(from) { Ok(meta) => meta, - Err(f) => crash!(1, "{}", f.to_string()), + Err(e) => return Err(InstallError::MetadataFailed(e).into()), }; let modified_time = FileTime::from_last_modification_time(&meta); @@ -664,6 +697,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { } /// Return true if a file is necessary to copy. This is the case when: +/// /// - _from_ or _to_ is nonexistent; /// - either file has a sticky bit or set[ug]id bit, or the user specified one; /// - either file isn't a regular file; @@ -679,14 +713,14 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { /// /// Crashes the program if a nonexistent owner or group is specified in _b_. /// -fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool { +fn need_copy(from: &Path, to: &Path, b: &Behavior) -> UResult { let from_meta = match fs::metadata(from) { Ok(meta) => meta, - Err(_) => return true, + Err(_) => return Ok(true), }; let to_meta = match fs::metadata(to) { Ok(meta) => meta, - Err(_) => return true, + Err(_) => return Ok(true), }; // setuid || setgid || sticky @@ -696,15 +730,15 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool { || from_meta.mode() & extra_mode != 0 || to_meta.mode() & extra_mode != 0 { - return true; + return Ok(true); } if !from_meta.is_file() || !to_meta.is_file() { - return true; + return Ok(true); } if from_meta.len() != to_meta.len() { - return true; + return Ok(true); } // TODO: if -P (#1809) and from/to contexts mismatch, return true. @@ -712,31 +746,31 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool { if !b.owner.is_empty() { let owner_id = match usr2uid(&b.owner) { Ok(id) => id, - _ => crash!(1, "no such user: {}", b.owner), + _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()), }; if owner_id != to_meta.uid() { - return true; + return Ok(true); } } else if !b.group.is_empty() { let group_id = match grp2gid(&b.group) { Ok(id) => id, - _ => crash!(1, "no such group: {}", b.group), + _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()), }; if group_id != to_meta.gid() { - return true; + return Ok(true); } } else { #[cfg(not(target_os = "windows"))] unsafe { if to_meta.uid() != geteuid() || to_meta.gid() != getegid() { - return true; + return Ok(true); } } } if !diff(from.to_str().unwrap(), to.to_str().unwrap()) { - return true; + return Ok(true); } - false + Ok(false) } diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 65bdcf36c..7010ff5e4 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -11,7 +11,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; -use uucore::error::{UCustomError, UResult}; +use uucore::error::{UError, UResult}; use std::borrow::Cow; use std::error::Error; @@ -79,7 +79,7 @@ impl Display for LnError { impl Error for LnError {} -impl UCustomError for LnError { +impl UError for LnError { fn code(&self) -> i32 { match self { Self::TargetIsDirectory(_) => 1, diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ef314dfa8..450acf8cd 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -39,7 +39,7 @@ use std::{ time::Duration, }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; -use uucore::error::{set_exit_code, FromIo, UCustomError, UResult}; +use uucore::error::{set_exit_code, FromIo, UError, UResult}; use unicode_width::UnicodeWidthStr; #[cfg(unix)] @@ -127,13 +127,15 @@ pub mod options { pub static IGNORE: &str = "ignore"; } +const DEFAULT_TERM_WIDTH: u16 = 80; + #[derive(Debug)] enum LsError { InvalidLineWidth(String), NoMetadata(PathBuf), } -impl UCustomError for LsError { +impl UError for LsError { fn code(&self) -> i32 { match self { LsError::InvalidLineWidth(_) => 2, @@ -229,7 +231,7 @@ struct Config { inode: bool, color: Option, long: LongFormat, - width: Option, + width: u16, quoting_style: QuotingStyle, indicator_style: IndicatorStyle, time_style: TimeStyle, @@ -258,16 +260,20 @@ impl Config { // below should never happen as clap already restricts the values. _ => unreachable!("Invalid field for --format"), }, - options::FORMAT, + Some(options::FORMAT), ) } else if options.is_present(options::format::LONG) { - (Format::Long, options::format::LONG) + (Format::Long, Some(options::format::LONG)) } else if options.is_present(options::format::ACROSS) { - (Format::Across, options::format::ACROSS) + (Format::Across, Some(options::format::ACROSS)) } else if options.is_present(options::format::COMMAS) { - (Format::Commas, options::format::COMMAS) + (Format::Commas, Some(options::format::COMMAS)) + } else if options.is_present(options::format::COLUMNS) { + (Format::Columns, Some(options::format::COLUMNS)) + } else if atty::is(atty::Stream::Stdout) { + (Format::Columns, None) } else { - (Format::Columns, options::format::COLUMNS) + (Format::OneLine, None) }; // The -o, -n and -g options are tricky. They cannot override with each @@ -286,9 +292,8 @@ impl Config { // options, but manually whether they have an index that's greater than // the other format options. If so, we set the appropriate format. if format != Format::Long { - let idx = options - .indices_of(opt) - .map(|x| x.max().unwrap()) + let idx = opt + .and_then(|opt| options.indices_of(opt).map(|x| x.max().unwrap())) .unwrap_or(0); if [ options::format::LONG_NO_OWNER, @@ -399,10 +404,25 @@ impl Config { let width = match options.value_of(options::WIDTH) { Some(x) => match x.parse::() { - Ok(u) => Some(u), + Ok(u) => u, Err(_) => return Err(LsError::InvalidLineWidth(x.into()).into()), }, - None => termsize::get().map(|s| s.cols), + None => match termsize::get() { + Some(size) => size.cols, + None => match std::env::var("COLUMNS") { + Ok(columns) => match columns.parse() { + Ok(columns) => columns, + Err(_) => { + show_error!( + "ignoring invalid width in environment variable COLUMNS: '{}'", + columns + ); + DEFAULT_TERM_WIDTH + } + }, + Err(_) => DEFAULT_TERM_WIDTH, + }, + }, }; #[allow(clippy::needless_bool)] @@ -1411,15 +1431,10 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter { - display_grid(names, width, Direction::TopToBottom, out) - } - (Format::Across, Some(width)) => { - display_grid(names, width, Direction::LeftToRight, out) - } - (Format::Commas, width_opt) => { - let term_width = width_opt.unwrap_or(1); + match config.format { + Format::Columns => display_grid(names, config.width, Direction::TopToBottom, out), + Format::Across => display_grid(names, config.width, Direction::LeftToRight, out), + Format::Commas => { let mut current_col = 0; let mut names = names; if let Some(name) = names.next() { @@ -1428,7 +1443,8 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter term_width { + // If the width is 0 we print one single line + if config.width != 0 && current_col + name_width + 1 > config.width { current_col = name_width + 2; let _ = write!(out, ",\n{}", name.contents); } else { @@ -1480,22 +1496,37 @@ fn display_grid( direction: Direction, out: &mut BufWriter, ) { - let mut grid = Grid::new(GridOptions { - filling: Filling::Spaces(2), - direction, - }); - - for name in names { - grid.add(name); - } - - match grid.fit_into_width(width as usize) { - Some(output) => { - let _ = write!(out, "{}", output); + if width == 0 { + // If the width is 0 we print one single line + let mut printed_something = false; + for name in names { + if printed_something { + let _ = write!(out, " "); + } + printed_something = true; + let _ = write!(out, "{}", name.contents); } - // Width is too small for the grid, so we fit it in one column - None => { - let _ = write!(out, "{}", grid.fit_into_columns(1)); + if printed_something { + let _ = writeln!(out); + } + } else { + let mut grid = Grid::new(GridOptions { + filling: Filling::Spaces(2), + direction, + }); + + for name in names { + grid.add(name); + } + + match grid.fit_into_width(width as usize) { + Some(output) => { + let _ = write!(out, "{}", output); + } + // Width is too small for the grid, so we fit it in one column + None => { + let _ = write!(out, "{}", grid.fit_into_columns(1)); + } } } } diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 8a4b472aa..e1dd604a0 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -12,7 +12,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; -use uucore::error::{FromIo, UCustomError, UResult}; +use uucore::error::{FromIo, UError, UResult}; use std::env; use std::error::Error; @@ -49,7 +49,7 @@ enum MkTempError { InvalidTemplate(String), } -impl UCustomError for MkTempError {} +impl UError for MkTempError {} impl Error for MkTempError {} diff --git a/src/uu/nice/src/nice.rs b/src/uu/nice/src/nice.rs index d5a4094d1..49efe32e0 100644 --- a/src/uu/nice/src/nice.rs +++ b/src/uu/nice/src/nice.rs @@ -10,21 +10,13 @@ #[macro_use] extern crate uucore; -use libc::{c_char, c_int, execvp}; +use libc::{c_char, c_int, execvp, PRIO_PROCESS}; use std::ffi::CString; use std::io::Error; use std::ptr; use clap::{crate_version, App, AppSettings, Arg}; -// XXX: PRIO_PROCESS is 0 on at least FreeBSD and Linux. Don't know about Mac OS X. -const PRIO_PROCESS: c_int = 0; - -extern "C" { - fn getpriority(which: c_int, who: c_int) -> c_int; - fn setpriority(which: c_int, who: c_int, prio: c_int) -> c_int; -} - pub mod options { pub static ADJUSTMENT: &str = "adjustment"; pub static COMMAND: &str = "COMMAND"; @@ -50,7 +42,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let mut niceness = unsafe { nix::errno::Errno::clear(); - getpriority(PRIO_PROCESS, 0) + libc::getpriority(PRIO_PROCESS, 0) }; if Error::last_os_error().raw_os_error().unwrap() != 0 { show_error!("getpriority: {}", Error::last_os_error()); @@ -84,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { }; niceness += adjustment; - if unsafe { setpriority(PRIO_PROCESS, 0, niceness) } == -1 { + if unsafe { libc::setpriority(PRIO_PROCESS, 0, niceness) } == -1 { show_warning!("setpriority: {}", Error::last_os_error()); } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index ec5bb595a..359531d4e 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -435,6 +435,7 @@ pub fn uu_app() -> clap::App<'static, 'static> { .long(options::FORMAT) .help("select output format or formats") .multiple(true) + .number_of_values(1) .value_name("TYPE"), ) .arg( diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index ae869ba49..e0b445782 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -45,7 +45,7 @@ use std::path::Path; use std::path::PathBuf; use std::str::Utf8Error; use unicode_width::UnicodeWidthStr; -use uucore::error::{set_exit_code, UCustomError, UResult, USimpleError, UUsageError}; +use uucore::error::{set_exit_code, UError, UResult, USimpleError, UUsageError}; use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::version_cmp::version_cmp; use uucore::InvalidEncodingHandling; @@ -164,7 +164,7 @@ enum SortError { impl Error for SortError {} -impl UCustomError for SortError { +impl UError for SortError { fn code(&self) -> i32 { match self { SortError::Disorder { .. } => 1, @@ -1238,7 +1238,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if separator.len() != 1 { return Err(UUsageError::new( 2, - "separator must be exactly one character long".into(), + "separator must be exactly one character long", )); } settings.separator = Some(separator.chars().next().unwrap()) @@ -1517,7 +1517,7 @@ fn exec( file_merger.write_all(settings, output) } else if settings.check { if files.len() > 1 { - Err(UUsageError::new(2, "only one file allowed with -c".into())) + Err(UUsageError::new(2, "only one file allowed with -c")) } else { check::check(files.first().unwrap(), settings) } diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index dd2b05d0e..49efa676a 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -17,7 +17,7 @@ use clap::{crate_version, App, Arg, ArgGroup}; use filetime::*; use std::fs::{self, File}; use std::path::Path; -use uucore::error::{FromIo, UResult, USimpleError}; +use uucore::error::{FromIo, UError, UResult, USimpleError}; static ABOUT: &str = "Update the access and modification times of each FILE to the current time."; pub mod options { diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 89c30b53b..69491c297 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -49,13 +49,14 @@ fn chgrp>(path: P, gid: gid_t, follow: bool) -> IOResult<()> { pub fn wrap_chgrp>( path: P, meta: &Metadata, - dest_gid: gid_t, + dest_gid: Option, follow: bool, verbosity: Verbosity, ) -> Result { use self::Verbosity::*; let path = path.as_ref(); let mut out: String = String::new(); + let dest_gid = dest_gid.unwrap_or_else(|| meta.gid()); if let Err(e) = chgrp(path, dest_gid, follow) { match verbosity { diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index d82da9feb..664fc9841 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -6,11 +6,11 @@ //! This module provides types to reconcile these exit codes with idiomatic Rust error //! handling. This has a couple advantages over manually using [`std::process::exit`]: //! 1. It enables the use of `?`, `map_err`, `unwrap_or`, etc. in `uumain`. -//! 1. It encourages the use of `UResult`/`Result` in functions in the utils. +//! 1. It encourages the use of [`UResult`]/[`Result`] in functions in the utils. //! 1. The error messages are largely standardized across utils. //! 1. Standardized error messages can be created from external result types //! (i.e. [`std::io::Result`] & `clap::ClapResult`). -//! 1. `set_exit_code` takes away the burden of manually tracking exit codes for non-fatal errors. +//! 1. [`set_exit_code`] takes away the burden of manually tracking exit codes for non-fatal errors. //! //! # Usage //! The signature of a typical util should be: @@ -19,7 +19,7 @@ //! ... //! } //! ``` -//! [`UResult`] is a simple wrapper around [`Result`] with a custom error type: [`UError`]. The +//! [`UResult`] is a simple wrapper around [`Result`] with a custom error trait: [`UError`]. The //! most important difference with types implementing [`std::error::Error`] is that [`UError`]s //! can specify the exit code of the program when they are returned from `uumain`: //! * When `Ok` is returned, the code set with [`set_exit_code`] is used as exit code. If @@ -41,13 +41,15 @@ //! [`set_exit_code`]. See the documentation on that function for more information. //! //! # Guidelines -//! * Use common errors where possible. -//! * Add variants to [`UCommonError`] if an error appears in multiple utils. +//! * Use error types from `uucore` where possible. +//! * Add error types to `uucore` if an error appears in multiple utils. //! * Prefer proper custom error types over [`ExitCode`] and [`USimpleError`]. //! * [`USimpleError`] may be used in small utils with simple error handling. //! * Using [`ExitCode`] is not recommended but can be useful for converting utils to use //! [`UResult`]. +// spell-checker:ignore uioerror + use std::{ error::Error, fmt::{Display, Formatter}, @@ -85,115 +87,10 @@ pub fn set_exit_code(code: i32) { EXIT_CODE.store(code, Ordering::SeqCst); } -/// Should be returned by all utils. -/// -/// Two additional methods are implemented on [`UResult`] on top of the normal [`Result`] methods: -/// `map_err_code` & `map_err_code_message`. -/// -/// These methods are used to convert [`UCommonError`]s into errors with a custom error code and -/// message. -pub type UResult = Result; +/// Result type that should be returned by all utils. +pub type UResult = Result>; -trait UResultTrait { - fn map_err_code(self, mapper: fn(&UCommonError) -> Option) -> Self; - fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self; -} - -impl UResultTrait for UResult { - fn map_err_code(self, mapper: fn(&UCommonError) -> Option) -> Self { - if let Err(UError::Common(error)) = self { - if let Some(code) = mapper(&error) { - Err(UCommonErrorWithCode { code, error }.into()) - } else { - Err(error.into()) - } - } else { - self - } - } - - fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self { - if let Err(UError::Common(ref error)) = self { - if let Some((code, message)) = mapper(error) { - return Err(USimpleError { code, message }.into()); - } - } - self - } -} - -/// The error type of [`UResult`]. -/// -/// `UError::Common` errors are defined in [`uucore`](crate) while `UError::Custom` errors are -/// defined by the utils. -/// ``` -/// use uucore::error::USimpleError; -/// let err = USimpleError::new(1, "Error!!".into()); -/// assert_eq!(1, err.code()); -/// assert_eq!(String::from("Error!!"), format!("{}", err)); -/// ``` -pub enum UError { - Common(UCommonError), - Custom(Box), -} - -impl UError { - /// The error code of [`UResult`] - /// - /// This function defines the error code associated with an instance of - /// [`UResult`]. To associate error codes for self-defined instances of - /// `UResult::Custom` (i.e. [`UCustomError`]), implement the - /// [`code`-function there](UCustomError::code). - pub fn code(&self) -> i32 { - match self { - UError::Common(e) => e.code(), - UError::Custom(e) => e.code(), - } - } - - /// Whether to print usage help for a [`UResult`] - /// - /// Defines if a variant of [`UResult`] should print a short usage message - /// below the error. The usage message is printed when this function returns - /// `true`. To do this for self-defined instances of `UResult::Custom` (i.e. - /// [`UCustomError`]), implement the [`usage`-function - /// there](UCustomError::usage). - pub fn usage(&self) -> bool { - match self { - UError::Common(e) => e.usage(), - UError::Custom(e) => e.usage(), - } - } -} - -impl From for UError { - fn from(v: UCommonError) -> Self { - UError::Common(v) - } -} - -impl From for UError { - fn from(v: i32) -> Self { - UError::Custom(Box::new(ExitCode(v))) - } -} - -impl From for UError { - fn from(v: E) -> Self { - UError::Custom(Box::new(v) as Box) - } -} - -impl Display for UError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - UError::Common(e) => e.fmt(f), - UError::Custom(e) => e.fmt(f), - } - } -} - -/// Custom errors defined by the utils. +/// Custom errors defined by the utils and `uucore`. /// /// All errors should implement [`std::error::Error`], [`std::fmt::Display`] and /// [`std::fmt::Debug`] and have an additional `code` method that specifies the @@ -202,7 +99,7 @@ impl Display for UError { /// An example of a custom error from `ls`: /// /// ``` -/// use uucore::error::{UCustomError, UResult}; +/// use uucore::error::{UError, UResult}; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -215,7 +112,7 @@ impl Display for UError { /// NoMetadata(PathBuf), /// } /// -/// impl UCustomError for LsError { +/// impl UError for LsError { /// fn code(&self) -> i32 { /// match self { /// LsError::InvalidLineWidth(_) => 2, @@ -246,12 +143,12 @@ impl Display for UError { /// } /// ``` /// -/// The call to `into()` is required to convert the [`UCustomError`] to an -/// instance of [`UError`]. +/// The call to `into()` is required to convert the `LsError` to +/// [`Box`]. The implementation for `From` is provided automatically. /// /// A crate like [`quick_error`](https://crates.io/crates/quick-error) might /// also be used, but will still require an `impl` for the `code` method. -pub trait UCustomError: Error + Send { +pub trait UError: Error + Send { /// Error code of a custom error. /// /// Set a return value for each variant of an enum-type to associate an @@ -261,7 +158,7 @@ pub trait UCustomError: Error + Send { /// # Example /// /// ``` - /// use uucore::error::{UCustomError}; + /// use uucore::error::{UError}; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -275,7 +172,7 @@ pub trait UCustomError: Error + Send { /// Bing(), /// } /// - /// impl UCustomError for MyError { + /// impl UError for MyError { /// fn code(&self) -> i32 { /// match self { /// MyError::Foo(_) => 2, @@ -312,7 +209,7 @@ pub trait UCustomError: Error + Send { /// # Example /// /// ``` - /// use uucore::error::{UCustomError}; + /// use uucore::error::{UError}; /// use std::{ /// error::Error, /// fmt::{Display, Debug}, @@ -326,7 +223,7 @@ pub trait UCustomError: Error + Send { /// Bing(), /// } /// - /// impl UCustomError for MyError { + /// impl UError for MyError { /// fn usage(&self) -> bool { /// match self { /// // This will have a short usage help appended @@ -355,47 +252,23 @@ pub trait UCustomError: Error + Send { } } -impl From> for i32 { - fn from(e: Box) -> i32 { - e.code() +impl From for Box +where + T: UError + 'static, +{ + fn from(t: T) -> Box { + Box::new(t) } } -/// A [`UCommonError`] with an overridden exit code. +/// A simple error type with an exit code and a message that implements [`UError`]. /// -/// This exit code is returned instead of the default exit code for the [`UCommonError`]. This is -/// typically created with the either the `UResult::map_err_code` or `UCommonError::with_code` -/// method. -#[derive(Debug)] -pub struct UCommonErrorWithCode { - code: i32, - error: UCommonError, -} - -impl Error for UCommonErrorWithCode {} - -impl Display for UCommonErrorWithCode { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { - self.error.fmt(f) - } -} - -impl UCustomError for UCommonErrorWithCode { - fn code(&self) -> i32 { - self.code - } -} - -/// A simple error type with an exit code and a message that implements [`UCustomError`]. -/// -/// It is typically created with the `UResult::map_err_code_and_message` method. Alternatively, it -/// can be constructed by manually: /// ``` /// use uucore::error::{UResult, USimpleError}; /// let err = USimpleError { code: 1, message: "error!".into()}; /// let res: UResult<()> = Err(err.into()); /// // or using the `new` method: -/// let res: UResult<()> = Err(USimpleError::new(1, "error!".into())); +/// let res: UResult<()> = Err(USimpleError::new(1, "error!")); /// ``` #[derive(Debug)] pub struct USimpleError { @@ -405,8 +278,11 @@ pub struct USimpleError { impl USimpleError { #[allow(clippy::new_ret_no_self)] - pub fn new(code: i32, message: String) -> UError { - UError::Custom(Box::new(Self { code, message })) + pub fn new>(code: i32, message: S) -> Box { + Box::new(Self { + code, + message: message.into(), + }) } } @@ -418,7 +294,7 @@ impl Display for USimpleError { } } -impl UCustomError for USimpleError { +impl UError for USimpleError { fn code(&self) -> i32 { self.code } @@ -432,8 +308,11 @@ pub struct UUsageError { impl UUsageError { #[allow(clippy::new_ret_no_self)] - pub fn new(code: i32, message: String) -> UError { - UError::Custom(Box::new(Self { code, message })) + pub fn new>(code: i32, message: S) -> Box { + Box::new(Self { + code, + message: message.into(), + }) } } @@ -445,7 +324,7 @@ impl Display for UUsageError { } } -impl UCustomError for UUsageError { +impl UError for UUsageError { fn code(&self) -> i32 { self.code } @@ -463,13 +342,13 @@ impl UCustomError 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, UCommonError}; +/// use uucore::error::{FromIo, UResult, UIoError, UError}; /// use std::fs::File; /// use std::path::Path; /// let path = Path::new("test.txt"); /// /// // Manual construction -/// let e: UIoError = UIoError::new( +/// let e: Box = UIoError::new( /// std::io::ErrorKind::NotFound, /// format!("cannot access '{}'", path.display()) /// ); @@ -485,22 +364,17 @@ pub struct UIoError { } impl UIoError { - pub fn new(kind: std::io::ErrorKind, context: String) -> Self { - Self { - context, + #[allow(clippy::new_ret_no_self)] + pub fn new>(kind: std::io::ErrorKind, context: S) -> Box { + Box::new(Self { + context: context.into(), inner: std::io::Error::new(kind, ""), - } - } - - pub fn code(&self) -> i32 { - 1 - } - - pub fn usage(&self) -> bool { - false + }) } } +impl UError for UIoError {} + impl Error for UIoError {} impl Display for UIoError { @@ -535,89 +409,102 @@ impl Display for UIoError { } } -/// Enables the conversion from `std::io::Error` to `UError` and from `std::io::Result` to -/// `UResult`. +/// Enables the conversion from [`std::io::Error`] to [`UError`] and from [`std::io::Result`] to +/// [`UResult`]. pub trait FromIo { fn map_err_context(self, context: impl FnOnce() -> String) -> T; } -impl FromIo for std::io::Error { - fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError { - UIoError { +impl FromIo> for std::io::Error { + fn map_err_context(self, context: impl FnOnce() -> String) -> Box { + Box::new(UIoError { context: (context)(), inner: self, - } + }) } } impl FromIo> for std::io::Result { fn map_err_context(self, context: impl FnOnce() -> String) -> UResult { - self.map_err(|e| UError::Common(UCommonError::Io(e.map_err_context(context)))) + self.map_err(|e| e.map_err_context(context) as Box) } } -impl FromIo for std::io::ErrorKind { - fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError { - UIoError { +impl FromIo> for std::io::ErrorKind { + fn map_err_context(self, context: impl FnOnce() -> String) -> Box { + Box::new(UIoError { context: (context)(), inner: std::io::Error::new(self, ""), - } + }) } } -impl From for UCommonError { - fn from(e: UIoError) -> UCommonError { - UCommonError::Io(e) - } -} - -impl From for UError { - fn from(e: UIoError) -> UError { - let common: UCommonError = e.into(); - common.into() - } -} - -/// Common errors for utilities. +/// Shorthand to construct [`UIoError`]-instances. /// -/// If identical errors appear across multiple utilities, they should be added here. -#[derive(Debug)] -pub enum UCommonError { - Io(UIoError), - // Clap(UClapError), -} - -impl UCommonError { - pub fn with_code(self, code: i32) -> UCommonErrorWithCode { - UCommonErrorWithCode { code, error: self } - } - - pub fn code(&self) -> i32 { - 1 - } - - pub fn usage(&self) -> bool { - false - } -} - -impl From for i32 { - fn from(common: UCommonError) -> i32 { - match common { - UCommonError::Io(e) => e.code(), - } - } -} - -impl Error for UCommonError {} - -impl Display for UCommonError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { - match self { - UCommonError::Io(e) => e.fmt(f), - } - } -} +/// This macro serves as a convenience call to quickly construct instances of +/// [`UIoError`]. It takes: +/// +/// - An instance of [`std::io::Error`] +/// - A `format!`-compatible string and +/// - An arbitrary number of arguments to the format string +/// +/// In exactly this order. It is equivalent to the more verbose code seen in the +/// example. +/// +/// # Examples +/// +/// ``` +/// use uucore::error::UIoError; +/// use uucore::uio_error; +/// +/// let io_err = std::io::Error::new( +/// std::io::ErrorKind::PermissionDenied, "fix me please!" +/// ); +/// +/// let uio_err = UIoError::new( +/// io_err.kind(), +/// format!("Error code: {}", 2) +/// ); +/// +/// let other_uio_err = uio_error!(io_err, "Error code: {}", 2); +/// +/// // prints "fix me please!: Permission denied" +/// println!("{}", uio_err); +/// // prints "Error code: 2: Permission denied" +/// println!("{}", other_uio_err); +/// ``` +/// +/// The [`std::fmt::Display`] impl of [`UIoError`] will then ensure that an +/// appropriate error message relating to the actual error kind of the +/// [`std::io::Error`] is appended to whatever error message is defined in +/// addition (as secondary argument). +/// +/// If you want to show only the error message for the [`std::io::ErrorKind`] +/// that's contained in [`UIoError`], pass the second argument as empty string: +/// +/// ``` +/// use uucore::error::UIoError; +/// use uucore::uio_error; +/// +/// let io_err = std::io::Error::new( +/// std::io::ErrorKind::PermissionDenied, "fix me please!" +/// ); +/// +/// let other_uio_err = uio_error!(io_err, ""); +/// +/// // prints: ": Permission denied" +/// println!("{}", other_uio_err); +/// ``` +//#[macro_use] +#[macro_export] +macro_rules! uio_error( + ($err:expr, $($args:tt)+) => ({ + UIoError::new( + $err.kind(), + format!($($args)+) + ) + }) +); /// A special error type that does not print any message when returned from /// `uumain`. Especially useful for porting utilities to using [`UResult`]. @@ -636,6 +523,13 @@ impl Display for UCommonError { #[derive(Debug)] pub struct ExitCode(pub i32); +impl ExitCode { + #[allow(clippy::new_ret_no_self)] + pub fn new(code: i32) -> Box { + Box::new(Self(code)) + } +} + impl Error for ExitCode {} impl Display for ExitCode { @@ -644,8 +538,14 @@ impl Display for ExitCode { } } -impl UCustomError for ExitCode { +impl UError for ExitCode { fn code(&self) -> i32 { self.0 } } + +impl From for Box { + fn from(i: i32) -> Self { + ExitCode::new(i) + } +} diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index d83b5515b..e52be9506 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -1,5 +1,4 @@ use crate::common::util::*; -#[cfg(unix)] use std::fs::OpenOptions; #[cfg(unix)] use std::io::Read; @@ -274,6 +273,26 @@ fn test_stdin_show_ends() { } } +#[test] +fn squeeze_all_files() { + // empty lines at the end of a file are "squeezed" together with empty lines at the beginning + let (at, mut ucmd) = at_and_ucmd!(); + at.write("input1", "a\n\n"); + at.write("input2", "\n\nb"); + ucmd.args(&["input1", "input2", "-s"]) + .succeeds() + .stdout_only("a\n\nb"); +} + +#[test] +fn test_show_ends_crlf() { + new_ucmd!() + .arg("-E") + .pipe_in("a\nb\r\n\rc\n\r\n\r") + .succeeds() + .stdout_only("a$\nb^M$\n\rc$\n^M$\n\r"); +} + #[test] fn test_stdin_show_all() { for same_param in &["-A", "--show-all"] { @@ -443,3 +462,49 @@ fn test_domain_socket() { thread.join().unwrap(); } + +#[test] +fn test_write_to_self_empty() { + // it's ok if the input file is also the output file if it's empty + let s = TestScenario::new(util_name!()); + let file_path = s.fixtures.plus("file.txt"); + + let file = OpenOptions::new() + .create_new(true) + .write(true) + .append(true) + .open(&file_path) + .unwrap(); + + s.ucmd().set_stdout(file).arg(&file_path).succeeds(); +} + +#[test] +fn test_write_to_self() { + let s = TestScenario::new(util_name!()); + let file_path = s.fixtures.plus("first_file"); + s.fixtures.write("second_file", "second_file_content."); + + let file = OpenOptions::new() + .create_new(true) + .write(true) + .append(true) + .open(&file_path) + .unwrap(); + + s.fixtures.append("first_file", "first_file_content."); + + s.ucmd() + .set_stdout(file) + .arg("first_file") + .arg("first_file") + .arg("second_file") + .fails() + .code_is(2) + .stderr_only("cat: first_file: input file is output file\ncat: first_file: input file is output file"); + + assert_eq!( + s.fixtures.read("first_file"), + "first_file_content.second_file_content." + ); +} diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 8e311116f..1b8057e47 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -244,3 +244,10 @@ fn basic_succeeds() { .no_stderr(); } } + +#[test] +fn test_no_change() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + ucmd.arg("").arg(at.plus("file")).succeeds(); +} diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 19f93e499..541e6b5d9 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -16,8 +16,6 @@ use std::os::windows::fs::symlink_file; use filetime::FileTime; #[cfg(target_os = "linux")] use rlimit::Resource; -#[cfg(not(windows))] -use std::env; #[cfg(target_os = "linux")] use std::fs as std_fs; #[cfg(target_os = "linux")] @@ -743,20 +741,16 @@ fn test_cp_deref_folder_to_folder() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - let cwd = env::current_dir().unwrap(); + let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER); - let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER); - - // Change the cwd to have a correct symlink - assert!(env::set_current_dir(&path_to_new_symlink).is_ok()); - - #[cfg(not(windows))] - let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK); - #[cfg(windows)] - let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK); - - // Back to the initial cwd (breaks the other tests) - assert!(env::set_current_dir(&cwd).is_ok()); + at.symlink_file( + &path_to_new_symlink + .join(TEST_HELLO_WORLD_SOURCE) + .to_string_lossy(), + &path_to_new_symlink + .join(TEST_HELLO_WORLD_SOURCE_SYMLINK) + .to_string_lossy(), + ); //using -P -R option scene @@ -843,20 +837,16 @@ fn test_cp_no_deref_folder_to_folder() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - let cwd = env::current_dir().unwrap(); + let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER); - let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER); - - // Change the cwd to have a correct symlink - assert!(env::set_current_dir(&path_to_new_symlink).is_ok()); - - #[cfg(not(windows))] - let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK); - #[cfg(windows)] - let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK); - - // Back to the initial cwd (breaks the other tests) - assert!(env::set_current_dir(&cwd).is_ok()); + at.symlink_file( + &path_to_new_symlink + .join(TEST_HELLO_WORLD_SOURCE) + .to_string_lossy(), + &path_to_new_symlink + .join(TEST_HELLO_WORLD_SOURCE_SYMLINK) + .to_string_lossy(), + ); //using -P -R option scene @@ -969,10 +959,9 @@ fn test_cp_archive() { } #[test] -#[cfg(target_os = "unix")] +#[cfg(unix)] fn test_cp_archive_recursive() { let (at, mut ucmd) = at_and_ucmd!(); - let cwd = env::current_dir().unwrap(); // creates // dir/1 @@ -988,26 +977,13 @@ fn test_cp_archive_recursive() { at.touch(&file_1.to_string_lossy()); at.touch(&file_2.to_string_lossy()); - // Change the cwd to have a correct symlink - assert!(env::set_current_dir(&at.subdir.join(TEST_COPY_TO_FOLDER)).is_ok()); - - #[cfg(not(windows))] - { - let _r = fs::symlink("1", &file_1_link); - let _r = fs::symlink("2", &file_2_link); - } - #[cfg(windows)] - { - let _r = symlink_file("1", &file_1_link); - let _r = symlink_file("2", &file_2_link); - } - // Back to the initial cwd (breaks the other tests) - assert!(env::set_current_dir(&cwd).is_ok()); + at.symlink_file("1", &file_1_link.to_string_lossy()); + at.symlink_file("2", &file_2_link.to_string_lossy()); ucmd.arg("--archive") .arg(TEST_COPY_TO_FOLDER) .arg(TEST_COPY_TO_FOLDER_NEW) - .fails(); // fails for now + .succeeds(); let scene2 = TestScenario::new("ls"); let result = scene2 @@ -1025,18 +1001,6 @@ fn test_cp_archive_recursive() { .run(); println!("ls dest {}", result.stdout_str()); - assert!(at.file_exists( - &at.subdir - .join(TEST_COPY_TO_FOLDER_NEW) - .join("1.link") - .to_string_lossy() - )); - assert!(at.file_exists( - &at.subdir - .join(TEST_COPY_TO_FOLDER_NEW) - .join("2.link") - .to_string_lossy() - )); assert!(at.file_exists( &at.subdir .join(TEST_COPY_TO_FOLDER_NEW) diff --git a/tests/by-util/test_factor.rs b/tests/by-util/test_factor.rs index a3f06ea8a..bcb1238bf 100644 --- a/tests/by-util/test_factor.rs +++ b/tests/by-util/test_factor.rs @@ -8,7 +8,10 @@ // spell-checker:ignore (methods) hexdigest +use tempfile::TempDir; + use crate::common::util::*; +use std::fs::OpenOptions; use std::time::SystemTime; #[path = "../../src/uu/factor/sieve.rs"] @@ -24,6 +27,43 @@ use self::sieve::Sieve; const NUM_PRIMES: usize = 10000; const NUM_TESTS: usize = 100; +#[test] +fn test_parallel() { + // factor should only flush the buffer at line breaks + let n_integers = 100_000; + let mut input_string = String::new(); + for i in 0..=n_integers { + input_string.push_str(&(format!("{} ", i))[..]); + } + + let tmp_dir = TempDir::new().unwrap(); + let tmp_dir = AtPath::new(tmp_dir.path()); + tmp_dir.touch("output"); + let output = OpenOptions::new() + .append(true) + .open(tmp_dir.plus("output")) + .unwrap(); + + for mut child in (0..10) + .map(|_| { + new_ucmd!() + .set_stdout(output.try_clone().unwrap()) + .pipe_in(input_string.clone()) + .run_no_wait() + }) + .collect::>() + { + assert_eq!(child.wait().unwrap().code().unwrap(), 0); + } + + let result = TestScenario::new(util_name!()) + .ccmd("sort") + .arg(tmp_dir.plus("output")) + .succeeds(); + let hash_check = sha1::Sha1::from(result.stdout()).hexdigest(); + assert_eq!(hash_check, "cc743607c0ff300ff575d92f4ff0c87d5660c393"); +} + #[test] fn test_first_100000_integers() { extern crate sha1; diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 44d14c304..a3372050a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -128,6 +128,7 @@ fn test_ls_width() { scene .ucmd() .args(&option.split(' ').collect::>()) + .arg("-C") .succeeds() .stdout_only("test-width-1 test-width-2 test-width-3 test-width-4\n"); } @@ -136,30 +137,33 @@ fn test_ls_width() { scene .ucmd() .args(&option.split(' ').collect::>()) + .arg("-C") .succeeds() .stdout_only("test-width-1 test-width-3\ntest-width-2 test-width-4\n"); } - for option in &[ - "-w 25", - "-w=25", - "--width=25", - "--width 25", - "-w 0", - "-w=0", - "--width=0", - "--width 0", - ] { + for option in &["-w 25", "-w=25", "--width=25", "--width 25"] { scene .ucmd() .args(&option.split(' ').collect::>()) + .arg("-C") .succeeds() .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n"); } + for option in &["-w 0", "-w=0", "--width=0", "--width 0"] { + scene + .ucmd() + .args(&option.split(' ').collect::>()) + .arg("-C") + .succeeds() + .stdout_only("test-width-1 test-width-2 test-width-3 test-width-4\n"); + } + scene .ucmd() .arg("-w=bad") + .arg("-C") .fails() .stderr_contains("invalid line width"); @@ -167,6 +171,7 @@ fn test_ls_width() { scene .ucmd() .args(&option.split(' ').collect::>()) + .arg("-C") .fails() .stderr_only("ls: invalid line width: '1a'"); } @@ -184,16 +189,10 @@ fn test_ls_columns() { // Columns is the default let result = scene.ucmd().succeeds(); - #[cfg(not(windows))] result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n"); - #[cfg(windows)] - result.stdout_only("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n"); for option in &["-C", "--format=columns"] { let result = scene.ucmd().arg(option).succeeds(); - #[cfg(not(windows))] - result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n"); - #[cfg(windows)] result.stdout_only("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n"); } @@ -205,6 +204,38 @@ fn test_ls_columns() { .succeeds() .stdout_only("test-columns-1 test-columns-3\ntest-columns-2 test-columns-4\n"); } + + // On windows we are always able to get the terminal size, so we can't simulate falling back to the + // environment variable. + #[cfg(not(windows))] + { + for option in &["-C", "--format=columns"] { + scene + .ucmd() + .env("COLUMNS", "40") + .arg(option) + .succeeds() + .stdout_only("test-columns-1 test-columns-3\ntest-columns-2 test-columns-4\n"); + } + + scene + .ucmd() + .env("COLUMNS", "garbage") + .arg("-C") + .succeeds() + .stdout_is("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n") + .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'"); + } + scene + .ucmd() + .arg("-Cw0") + .succeeds() + .stdout_only("test-columns-1 test-columns-2 test-columns-3 test-columns-4\n"); + scene + .ucmd() + .arg("-mw0") + .succeeds() + .stdout_only("test-columns-1, test-columns-2, test-columns-3, test-columns-4\n"); } #[test] @@ -220,11 +251,7 @@ fn test_ls_across() { let result = scene.ucmd().arg(option).succeeds(); // Because the test terminal has width 0, this is the same output as // the columns option. - if cfg!(unix) { - result.stdout_only("test-across-1\ntest-across-2\ntest-across-3\ntest-across-4\n"); - } else { - result.stdout_only("test-across-1 test-across-2 test-across-3 test-across-4\n"); - } + result.stdout_only("test-across-1 test-across-2 test-across-3 test-across-4\n"); } for option in &["-x", "--format=across"] { @@ -250,11 +277,7 @@ fn test_ls_commas() { for option in &["-m", "--format=commas"] { let result = scene.ucmd().arg(option).succeeds(); - if cfg!(unix) { - result.stdout_only("test-commas-1,\ntest-commas-2,\ntest-commas-3,\ntest-commas-4\n"); - } else { - result.stdout_only("test-commas-1, test-commas-2, test-commas-3, test-commas-4\n"); - } + result.stdout_only("test-commas-1, test-commas-2, test-commas-3, test-commas-4\n"); } for option in &["-m", "--format=commas"] { @@ -571,13 +594,11 @@ fn test_ls_sort_name() { at.touch("test-1"); at.touch("test-2"); - let sep = if cfg!(unix) { "\n" } else { " " }; - scene .ucmd() .arg("--sort=name") .succeeds() - .stdout_is(["test-1", "test-2", "test-3\n"].join(sep)); + .stdout_is("test-1\ntest-2\ntest-3\n"); let scene_dot = TestScenario::new(util_name!()); let at = &scene_dot.fixtures; @@ -591,7 +612,7 @@ fn test_ls_sort_name() { .arg("--sort=name") .arg("-A") .succeeds() - .stdout_is([".a", ".b", "a", "b\n"].join(sep)); + .stdout_is(".a\n.b\na\nb\n"); } #[test] @@ -612,28 +633,16 @@ fn test_ls_order_size() { scene.ucmd().arg("-al").succeeds(); let result = scene.ucmd().arg("-S").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - #[cfg(windows)] - result.stdout_only("test-4 test-3 test-2 test-1\n"); let result = scene.ucmd().arg("-S").arg("-r").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n"); - #[cfg(windows)] - result.stdout_only("test-1 test-2 test-3 test-4\n"); let result = scene.ucmd().arg("--sort=size").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - #[cfg(windows)] - result.stdout_only("test-4 test-3 test-2 test-1\n"); let result = scene.ucmd().arg("--sort=size").arg("-r").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n"); - #[cfg(windows)] - result.stdout_only("test-1 test-2 test-3 test-4\n"); } #[test] @@ -755,9 +764,6 @@ fn test_ls_styles() { at.touch("test2"); let result = scene.ucmd().arg("--full-time").arg("-x").succeeds(); - #[cfg(not(windows))] - assert_eq!(result.stdout_str(), "test\ntest2\n"); - #[cfg(windows)] assert_eq!(result.stdout_str(), "test test2\n"); } @@ -794,28 +800,16 @@ fn test_ls_order_time() { // ctime was changed at write, so the order is 4 3 2 1 let result = scene.ucmd().arg("-t").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - #[cfg(windows)] - result.stdout_only("test-4 test-3 test-2 test-1\n"); let result = scene.ucmd().arg("--sort=time").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - #[cfg(windows)] - result.stdout_only("test-4 test-3 test-2 test-1\n"); let result = scene.ucmd().arg("-tr").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n"); - #[cfg(windows)] - result.stdout_only("test-1 test-2 test-3 test-4\n"); let result = scene.ucmd().arg("--sort=time").arg("-r").succeeds(); - #[cfg(not(windows))] result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n"); - #[cfg(windows)] - result.stdout_only("test-1 test-2 test-3 test-4\n"); // 3 was accessed last in the read // So the order should be 2 3 4 1 @@ -826,19 +820,11 @@ fn test_ls_order_time() { // It seems to be dependent on the platform whether the access time is actually set if file3_access > file4_access { - if cfg!(not(windows)) { - result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n"); - } else { - result.stdout_only("test-3 test-4 test-2 test-1\n"); - } + result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n"); } else { // Access time does not seem to be set on Windows and some other // systems so the order is 4 3 2 1 - if cfg!(not(windows)) { - result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); - } else { - result.stdout_only("test-4 test-3 test-2 test-1\n"); - } + result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n"); } } @@ -991,6 +977,7 @@ fn test_ls_color() { .ucmd() .arg("--color") .arg("-w=15") + .arg("-C") .succeeds() .stdout_only(format!( "{} test-color\nb {}\n", @@ -2009,11 +1996,7 @@ fn test_ls_path() { }; scene.ucmd().arg(&abs_path).run().stdout_is(expected_stdout); - let expected_stdout = if cfg!(windows) { - format!("{} {}\n", path, file1) - } else { - format!("{}\n{}\n", path, file1) - }; + let expected_stdout = format!("{}\n{}\n", path, file1); scene .ucmd() .arg(file1) diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 33d7d4dc4..0fc1d5106 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -46,6 +46,17 @@ fn test_file() { .succeeds() .no_stderr() .stdout_is(unindent(ALPHA_OUT)); + + // Ensure that default format matches `-t o2`, and that `-t` does not absorb file argument + new_ucmd!() + .arg("--endian=little") + .arg("-t") + .arg("o2") + .arg(file.as_os_str()) + .succeeds() + .no_stderr() + .stdout_is(unindent(ALPHA_OUT)); + let _ = remove_file(file); } diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 4a79a3eda..5c16e7acc 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -68,41 +68,36 @@ fn test_with_numbering_option_with_number_width() { fn test_with_long_header_option() { let test_file_path = "test_one_page.log"; let expected_test_file_path = "test_one_page_header.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); let header = "new file"; - scenario - .args(&["--header=new file", test_file_path]) - .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - &[("{last_modified_time}", &value), ("{header}", header)], - ); - - new_ucmd!() - .args(&["-h", header, test_file_path]) - .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path, - &[("{last_modified_time}", &value), ("{header}", header)], - ); + for args in &[&["-h", header][..], &["--header=new file"][..]] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(args) + .arg(test_file_path) + .succeeds() + .stdout_is_templated_fixture( + expected_test_file_path, + &[("{last_modified_time}", &value), ("{header}", header)], + ); + } } #[test] fn test_with_double_space_option() { let test_file_path = "test_one_page.log"; let expected_test_file_path = "test_one_page_double_line.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); - scenario - .args(&["-d", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&["--double-space", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); + for &arg in &["-d", "--double-space"] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&[arg, test_file_path]) + .succeeds() + .stdout_is_templated_fixture( + expected_test_file_path, + &[("{last_modified_time}", &value)], + ); + } } #[test] @@ -188,33 +183,28 @@ fn test_with_page_range() { let test_file_path = "test.log"; let expected_test_file_path = "test_page_range_1.log.expected"; let expected_test_file_path1 = "test_page_range_2.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); - scenario - .args(&["--pages=15", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&["+15", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&["--pages=15:17", test_file_path]) - .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path1, - &[("{last_modified_time}", &value)], - ); - - new_ucmd!() - .args(&["+15:17", test_file_path]) - .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path1, - &[("{last_modified_time}", &value)], - ); + for &arg in &["--pages=15", "+15"] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&[arg, test_file_path]) + .succeeds() + .stdout_is_templated_fixture( + expected_test_file_path, + &[("{last_modified_time}", &value)], + ); + } + for &arg in &["--pages=15:17", "+15:17"] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&[arg, test_file_path]) + .succeeds() + .stdout_is_templated_fixture( + expected_test_file_path1, + &[("{last_modified_time}", &value)], + ); + } } #[test] @@ -232,19 +222,17 @@ fn test_with_no_header_trailer_option() { #[test] fn test_with_page_length_option() { let test_file_path = "test.log"; - let expected_test_file_path = "test_page_length.log.expected"; - let expected_test_file_path1 = "test_page_length1.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); - scenario - .args(&["--pages=2:3", "-l", "100", "-n", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&["--pages=2:3", "-l", "5", "-n", test_file_path]) - .succeeds() - .stdout_is_fixture(expected_test_file_path1); + for (arg, expected) in &[ + ("100", "test_page_length.log.expected"), + ("5", "test_page_length1.log.expected"), + ] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&["--pages=2:3", "-l", arg, "-n", test_file_path]) + .succeeds() + .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]); + } } #[test] @@ -277,17 +265,17 @@ fn test_with_stdin() { fn test_with_column() { let test_file_path = "column.log"; let expected_test_file_path = "column.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); - scenario - .args(&["--pages=3:5", "--column=3", "-n", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&["--pages=3:5", "-3", "-n", test_file_path]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); + for arg in &["-3", "--column=3"] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&["--pages=3:5", arg, "-n", test_file_path]) + .succeeds() + .stdout_is_templated_fixture( + expected_test_file_path, + &[("{last_modified_time}", &value)], + ); + } } #[test] @@ -305,36 +293,17 @@ fn test_with_column_across_option() { #[test] fn test_with_column_across_option_and_column_separator() { let test_file_path = "column.log"; - let expected_test_file_path = "column_across_sep.log.expected"; - let expected_test_file_path1 = "column_across_sep1.log.expected"; - let mut scenario = new_ucmd!(); - let value = file_last_modified_time(&scenario, test_file_path); - scenario - .args(&[ - "--pages=3:5", - "--column=3", - "-s|", - "-a", - "-n", - test_file_path, - ]) - .succeeds() - .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]); - - new_ucmd!() - .args(&[ - "--pages=3:5", - "--column=3", - "-Sdivide", - "-a", - "-n", - test_file_path, - ]) - .succeeds() - .stdout_is_templated_fixture( - expected_test_file_path1, - &[("{last_modified_time}", &value)], - ); + for (arg, expected) in &[ + ("-s|", "column_across_sep.log.expected"), + ("-Sdivide", "column_across_sep1.log.expected"), + ] { + let mut scenario = new_ucmd!(); + let value = file_last_modified_time(&scenario, test_file_path); + scenario + .args(&["--pages=3:5", "--column=3", arg, "-a", "-n", test_file_path]) + .succeeds() + .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]); + } } #[test] diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 4d70ceccc..bbb16981e 100644 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -96,7 +96,6 @@ sed -i 's|seq |/usr/bin/seq |' tests/misc/sort-discrim.sh # Add specific timeout to tests that currently hang to limit time spent waiting sed -i 's|seq \$|/usr/bin/timeout 0.1 seq \$|' tests/misc/seq-precision.sh tests/misc/seq-long-double.sh -sed -i 's|cat |/usr/bin/timeout 0.1 cat |' tests/misc/cat-self.sh # Remove dup of /usr/bin/ when executed several times diff --git a/util/compare_gnu_result.py b/util/compare_gnu_result.py new file mode 100644 index 000000000..52aa96abe --- /dev/null +++ b/util/compare_gnu_result.py @@ -0,0 +1,32 @@ +#! /usr/bin/python + +""" +Compare the current results to the last results gathered from the master branch to highlight +if a PR is making the results better/worse +""" + +import json +import sys +from os import environ + +NEW = json.load(open("gnu-result.json")) +OLD = json.load(open("master-gnu-result.json")) + +# Extract the specific results from the dicts +last = OLD[list(OLD.keys())[0]] +current = NEW[list(NEW.keys())[0]] + + +pass_d = int(current["pass"]) - int(last["pass"]) +fail_d = int(current["fail"]) - int(last["fail"]) +error_d = int(current["error"]) - int(last["error"]) +skip_d = int(current["skip"]) - int(last["skip"]) + +# Get an annotation to highlight changes +print( + f"::warning ::Changes from master: PASS {pass_d:+d} / FAIL {fail_d:+d} / ERROR {error_d:+d} / SKIP {skip_d:+d} " +) + +# If results are worse fail the job to draw attention +if pass_d < 0: + sys.exit(1)