From 1074deeb0340f5a2f365f52fa5bffc4213048587 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 27 Jan 2022 20:56:09 -0500 Subject: [PATCH 1/2] truncate: make better use of UResult Replace some uses of `crash!()` and move `UError` handling down into the `truncate()` function. This does not change the behavior of the program, just organizes the code to facilitate introducing code to handle other types of errors in the future. --- src/uu/truncate/src/truncate.rs | 88 +++++++++++++++------------------ 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index b615cf495..4b81900c0 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -6,17 +6,13 @@ // * file that was distributed with this source code. // spell-checker:ignore (ToDO) RFILE refsize rfilename fsize tsize - -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use std::convert::TryFrom; use std::fs::{metadata, OpenOptions}; use std::io::ErrorKind; use std::path::Path; use uucore::display::Quotable; -use uucore::error::{UIoError, UResult, USimpleError, UUsageError}; +use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::parse_size::{parse_size, ParseSizeError}; #[derive(Debug, Eq, PartialEq)] @@ -113,24 +109,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let no_create = matches.is_present(options::NO_CREATE); let reference = matches.value_of(options::REFERENCE).map(String::from); let size = matches.value_of(options::SIZE).map(String::from); - truncate(no_create, io_blocks, reference, size, files).map_err(|e| { - match e.kind() { - ErrorKind::NotFound => { - // TODO Improve error-handling so that the error - // returned by `truncate()` provides the necessary - // parameter for formatting the error message. - let reference = matches.value_of(options::REFERENCE).map(String::from); - USimpleError::new( - 1, - format!( - "cannot stat {}: No such file or directory", - reference.as_deref().unwrap_or("").quote() - ), - ) // TODO: fix '--no-create' see test_reference and test_truncate_bytes_size - } - _ => uio_error!(e, ""), - } - }) + truncate(no_create, io_blocks, reference, size, files) } } @@ -212,20 +191,31 @@ fn truncate_reference_and_size( size_string: &str, filenames: Vec, create: bool, -) -> std::io::Result<()> { +) -> UResult<()> { let mode = match parse_mode_and_size(size_string) { - Ok(m) => match m { - TruncateMode::Absolute(_) => { - crash!(1, "you must specify a relative '--size' with '--reference'") - } - _ => m, - }, - Err(e) => crash!(1, "Invalid number: {}", e.to_string()), + Err(e) => return Err(USimpleError::new(1, format!("Invalid number: {}", e))), + Ok(TruncateMode::Absolute(_)) => { + return Err(USimpleError::new( + 1, + String::from("you must specify a relative '--size' with '--reference'"), + )) + } + Ok(m) => m, }; - let fsize = usize::try_from(metadata(rfilename)?.len()).unwrap(); + let metadata = metadata(rfilename).map_err(|e| match e.kind() { + ErrorKind::NotFound => USimpleError::new( + 1, + format!( + "cannot stat {}: No such file or directory", + rfilename.quote() + ), + ), + _ => e.map_err_context(String::new), + })?; + let fsize = metadata.len() as usize; let tsize = mode.to_size(fsize); for filename in &filenames { - file_truncate(filename, create, tsize)?; + file_truncate(filename, create, tsize).map_err_context(String::new)?; } Ok(()) } @@ -245,10 +235,20 @@ fn truncate_reference_file_only( rfilename: &str, filenames: Vec, create: bool, -) -> std::io::Result<()> { - let tsize = usize::try_from(metadata(rfilename)?.len()).unwrap(); +) -> UResult<()> { + let metadata = metadata(rfilename).map_err(|e| match e.kind() { + ErrorKind::NotFound => USimpleError::new( + 1, + format!( + "cannot stat {}: No such file or directory", + rfilename.quote() + ), + ), + _ => e.map_err_context(String::new), + })?; + let tsize = metadata.len() as usize; for filename in &filenames { - file_truncate(filename, create, tsize)?; + file_truncate(filename, create, tsize).map_err_context(String::new)?; } Ok(()) } @@ -268,15 +268,9 @@ fn truncate_reference_file_only( /// /// If the any file could not be opened, or there was a problem setting /// the size of at least one file. -fn truncate_size_only( - size_string: &str, - filenames: Vec, - create: bool, -) -> std::io::Result<()> { - let mode = match parse_mode_and_size(size_string) { - Ok(m) => m, - Err(e) => crash!(1, "Invalid number: {}", e.to_string()), - }; +fn truncate_size_only(size_string: &str, filenames: Vec, create: bool) -> UResult<()> { + let mode = parse_mode_and_size(size_string) + .map_err(|e| USimpleError::new(1, format!("Invalid number: {}", e)))?; for filename in &filenames { let fsize = match metadata(filename) { Ok(m) => m.len(), @@ -286,7 +280,7 @@ fn truncate_size_only( match file_truncate(filename, create, tsize) { Ok(_) => continue, Err(e) if e.kind() == ErrorKind::NotFound && !create => continue, - Err(e) => return Err(e), + Err(e) => return Err(e.map_err_context(String::new)), } } Ok(()) @@ -298,7 +292,7 @@ fn truncate( reference: Option, size: Option, filenames: Vec, -) -> std::io::Result<()> { +) -> UResult<()> { let create = !no_create; // There are four possibilities // - reference file given and size given, From 1e5e637990bcc8c8c0691d7a57313c3803285be6 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 27 Jan 2022 21:03:38 -0500 Subject: [PATCH 2/2] truncate: add a division by zero error Add an error for division by zero. Previously, running `truncate -s /0 file` or `-s %0` would panic due to division by zero. After this change, it writes an error message "division by zero" to stderr and terminates with an error code. --- src/uu/truncate/src/truncate.rs | 6 ++++++ tests/by-util/test_truncate.rs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 4b81900c0..df42d7f66 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -202,6 +202,9 @@ fn truncate_reference_and_size( } Ok(m) => m, }; + if let TruncateMode::RoundDown(0) | TruncateMode::RoundUp(0) = mode { + return Err(USimpleError::new(1, "division by zero")); + } let metadata = metadata(rfilename).map_err(|e| match e.kind() { ErrorKind::NotFound => USimpleError::new( 1, @@ -271,6 +274,9 @@ fn truncate_reference_file_only( fn truncate_size_only(size_string: &str, filenames: Vec, create: bool) -> UResult<()> { let mode = parse_mode_and_size(size_string) .map_err(|e| USimpleError::new(1, format!("Invalid number: {}", e)))?; + if let TruncateMode::RoundDown(0) | TruncateMode::RoundUp(0) = mode { + return Err(USimpleError::new(1, "division by zero")); + } for filename in &filenames { let fsize = match metadata(filename) { Ok(m) => m.len(), diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index bb76e8b94..135c55456 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -346,3 +346,34 @@ fn test_new_file_no_create() { .no_stderr(); assert!(!at.file_exists(filename)); } + +#[test] +fn test_division_by_zero_size_only() { + new_ucmd!() + .args(&["-s", "/0", "file"]) + .fails() + .no_stdout() + .stderr_contains("division by zero"); + new_ucmd!() + .args(&["-s", "%0", "file"]) + .fails() + .no_stdout() + .stderr_contains("division by zero"); +} + +#[test] +fn test_division_by_zero_reference_and_size() { + let (at, mut ucmd) = at_and_ucmd!(); + at.make_file(FILE1); + ucmd.args(&["-r", FILE1, "-s", "/0", "file"]) + .fails() + .no_stdout() + .stderr_contains("division by zero"); + + let (at, mut ucmd) = at_and_ucmd!(); + at.make_file(FILE1); + ucmd.args(&["-r", FILE1, "-s", "%0", "file"]) + .fails() + .no_stdout() + .stderr_contains("division by zero"); +}