From a746e37dc7e087159ede20c9c887015d77ff6221 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 21 May 2021 18:20:05 -0400 Subject: [PATCH 1/5] truncate: add test for -r and -s options together Add a test for when the reference file is not found and both `-r` and `-s` options are given on the command-line. --- tests/by-util/test_truncate.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index 120982e3c..6323b058f 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -262,3 +262,11 @@ fn test_reference_file_not_found() { .fails() .stderr_contains("cannot stat 'a': No such file or directory"); } + +#[test] +fn test_reference_with_size_file_not_found() { + new_ucmd!() + .args(&["-r", "a", "-s", "+1", "b"]) + .fails() + .stderr_contains("cannot stat 'a': No such file or directory"); +} From 5eb2a5c3e1e72e7d01ce87eed55928f37b14d5ca Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 21 May 2021 18:23:50 -0400 Subject: [PATCH 2/5] truncate: remove read permissions from OpenOptions Remove "read" permissions from the `OpenOptions` when opening a new file just to truncate it. We will never read from the file, only write to it. (Specifically, we will only call `File::set_len()`.) --- src/uu/truncate/src/truncate.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 03b18723c..086e14858 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -189,12 +189,7 @@ fn truncate( }; for filename in &filenames { let path = Path::new(filename); - match OpenOptions::new() - .read(true) - .write(true) - .create(!no_create) - .open(path) - { + match OpenOptions::new().write(true).create(!no_create).open(path) { Ok(file) => { let fsize = match reference { Some(_) => refsize, From 544ae875753b050a5073278e8bcb8af893e31de0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 21 May 2021 21:06:45 -0400 Subject: [PATCH 3/5] truncate: add parse_mode_and_size() helper func Add a helper function to contain the code for parsing the size and the modifier symbol, if any. This commit also changes the `TruncateMode` enum so that the parameter for each "mode" is stored along with the enumeration value. This is because the parameter has a different meaning in each mode. --- src/uu/truncate/src/truncate.rs | 136 ++++++++++++++++++++------------ 1 file changed, 85 insertions(+), 51 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 086e14858..9df775300 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -15,16 +15,16 @@ use std::fs::{metadata, OpenOptions}; use std::io::ErrorKind; use std::path::Path; -#[derive(Eq, PartialEq)] +#[derive(Debug, Eq, PartialEq)] enum TruncateMode { - Absolute, - Reference, - Extend, - Reduce, - AtMost, - AtLeast, - RoundDown, - RoundUp, + Reference(u64), + Absolute(u64), + Extend(u64), + Reduce(u64), + AtMost(u64), + AtLeast(u64), + RoundDown(u64), + RoundUp(u64), } static ABOUT: &str = "Shrink or extend the size of each file to the specified size."; @@ -133,46 +133,21 @@ fn truncate( size: Option, filenames: Vec, ) { - let (modsize, mode) = match size { - Some(size_string) => { - // Trim any whitespace. - let size_string = size_string.trim(); - - // Get the modifier character from the size string, if any. For - // example, if the argument is "+123", then the modifier is '+'. - let c = size_string.chars().next().unwrap(); - - let mode = match c { - '+' => TruncateMode::Extend, - '-' => TruncateMode::Reduce, - '<' => TruncateMode::AtMost, - '>' => TruncateMode::AtLeast, - '/' => TruncateMode::RoundDown, - '%' => TruncateMode::RoundUp, - _ => TruncateMode::Absolute, /* assume that the size is just a number */ - }; - - // If there was a modifier character, strip it. - let size_string = match mode { - TruncateMode::Absolute => size_string, - _ => &size_string[1..], - }; - let num_bytes = match parse_size(size_string) { - Ok(b) => b, - Err(_) => crash!(1, "Invalid number: ‘{}’", size_string), - }; - (num_bytes, mode) - } - None => (0, TruncateMode::Reference), + let mode = match size { + Some(size_string) => match parse_mode_and_size(&size_string) { + Ok(m) => m, + Err(_) => crash!(1, "Invalid number: ‘{}’", size_string), + }, + None => TruncateMode::Reference(0), }; let refsize = match reference { Some(ref rfilename) => { match mode { // Only Some modes work with a reference - TruncateMode::Reference => (), //No --size was given - TruncateMode::Extend => (), - TruncateMode::Reduce => (), + TruncateMode::Reference(_) => (), //No --size was given + TruncateMode::Extend(_) => (), + TruncateMode::Reduce(_) => (), _ => crash!(1, "you must specify a relative ‘--size’ with ‘--reference’"), }; match metadata(rfilename) { @@ -202,14 +177,14 @@ fn truncate( }, }; let tsize: u64 = match mode { - TruncateMode::Absolute => modsize, - TruncateMode::Reference => fsize, - TruncateMode::Extend => fsize + modsize, - TruncateMode::Reduce => fsize - modsize, - TruncateMode::AtMost => fsize.min(modsize), - TruncateMode::AtLeast => fsize.max(modsize), - TruncateMode::RoundDown => fsize - fsize % modsize, - TruncateMode::RoundUp => fsize + fsize % modsize, + TruncateMode::Absolute(modsize) => modsize, + TruncateMode::Reference(_) => fsize, + TruncateMode::Extend(modsize) => fsize + modsize, + TruncateMode::Reduce(modsize) => fsize - modsize, + TruncateMode::AtMost(modsize) => fsize.min(modsize), + TruncateMode::AtLeast(modsize) => fsize.max(modsize), + TruncateMode::RoundDown(modsize) => fsize - fsize % modsize, + TruncateMode::RoundUp(modsize) => fsize + fsize % modsize, }; match file.set_len(tsize) { Ok(_) => {} @@ -221,6 +196,52 @@ fn truncate( } } +/// Decide whether a character is one of the size modifiers, like '+' or '<'. +fn is_modifier(c: char) -> bool { + c == '+' || c == '-' || c == '<' || c == '>' || c == '/' || c == '%' +} + +/// Parse a size string with optional modifier symbol as its first character. +/// +/// A size string is as described in [`parse_size`]. The first character +/// of `size_string` might be a modifier symbol, like `'+'` or +/// `'<'`. The first element of the pair returned by this function +/// indicates which modifier symbol was present, or +/// [`TruncateMode::Absolute`] if none. +/// +/// # Panics +/// +/// If `size_string` is empty, or if no number could be parsed from the +/// given string (for example, if the string were `"abc"`). +/// +/// # Examples +/// +/// ```rust,ignore +/// assert_eq!(parse_mode_and_size("+123"), (TruncateMode::Extend, 123)); +/// ``` +fn parse_mode_and_size(size_string: &str) -> Result { + // Trim any whitespace. + let size_string = size_string.trim(); + + // Get the modifier character from the size string, if any. For + // example, if the argument is "+123", then the modifier is '+'. + let c = size_string.chars().next().unwrap(); + let size_string = if is_modifier(c) { + &size_string[1..] + } else { + size_string + }; + parse_size(size_string).map(match c { + '+' => TruncateMode::Extend, + '-' => TruncateMode::Reduce, + '<' => TruncateMode::AtMost, + '>' => TruncateMode::AtLeast, + '/' => TruncateMode::RoundDown, + '%' => TruncateMode::RoundUp, + _ => TruncateMode::Absolute, + }) +} + /// Parse a size string into a number of bytes. /// /// A size string comprises an integer and an optional unit. The unit @@ -280,7 +301,9 @@ fn parse_size(size: &str) -> Result { #[cfg(test)] mod tests { + use crate::parse_mode_and_size; use crate::parse_size; + use crate::TruncateMode; #[test] fn test_parse_size_zero() { @@ -306,4 +329,15 @@ mod tests { assert_eq!(parse_size("123M").unwrap(), 123 * 1024 * 1024); assert_eq!(parse_size("123MB").unwrap(), 123 * 1000 * 1000); } + + #[test] + fn test_parse_mode_and_size() { + assert_eq!(parse_mode_and_size("10"), Ok(TruncateMode::Absolute(10))); + assert_eq!(parse_mode_and_size("+10"), Ok(TruncateMode::Extend(10))); + assert_eq!(parse_mode_and_size("-10"), Ok(TruncateMode::Reduce(10))); + assert_eq!(parse_mode_and_size("<10"), Ok(TruncateMode::AtMost(10))); + assert_eq!(parse_mode_and_size(">10"), Ok(TruncateMode::AtLeast(10))); + assert_eq!(parse_mode_and_size("/10"), Ok(TruncateMode::RoundDown(10))); + assert_eq!(parse_mode_and_size("%10"), Ok(TruncateMode::RoundUp(10))); + } } From c6d4d0c07d1f1ed5d6dbe58ca0eda5e9c939b2c2 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 21 May 2021 21:09:33 -0400 Subject: [PATCH 4/5] truncate: create TruncateMode::to_size() method Create a method that computes the final target size in bytes for the file to truncate, given the reference file size and the parameter to the `TruncateMode`. --- src/uu/truncate/src/truncate.rs | 37 ++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 9df775300..c0f078458 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -27,6 +27,32 @@ enum TruncateMode { RoundUp(u64), } +impl TruncateMode { + /// Compute a target size in bytes for this truncate mode. + /// + /// `fsize` is the size of the reference file, in bytes. + /// + /// # Examples + /// + /// ```rust,ignore + /// let mode = TruncateMode::Extend(5); + /// let fsize = 10; + /// assert_eq!(mode.to_size(fsize), 15); + /// ``` + fn to_size(&self, fsize: u64) -> u64 { + match self { + TruncateMode::Absolute(modsize) => *modsize, + TruncateMode::Reference(_) => fsize, + TruncateMode::Extend(modsize) => fsize + modsize, + TruncateMode::Reduce(modsize) => fsize - modsize, + TruncateMode::AtMost(modsize) => fsize.min(*modsize), + TruncateMode::AtLeast(modsize) => fsize.max(*modsize), + TruncateMode::RoundDown(modsize) => fsize - fsize % modsize, + TruncateMode::RoundUp(modsize) => fsize + fsize % modsize, + } + } +} + static ABOUT: &str = "Shrink or extend the size of each file to the specified size."; static VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -176,16 +202,7 @@ fn truncate( } }, }; - let tsize: u64 = match mode { - TruncateMode::Absolute(modsize) => modsize, - TruncateMode::Reference(_) => fsize, - TruncateMode::Extend(modsize) => fsize + modsize, - TruncateMode::Reduce(modsize) => fsize - modsize, - TruncateMode::AtMost(modsize) => fsize.min(modsize), - TruncateMode::AtLeast(modsize) => fsize.max(modsize), - TruncateMode::RoundDown(modsize) => fsize - fsize % modsize, - TruncateMode::RoundUp(modsize) => fsize + fsize % modsize, - }; + let tsize = mode.to_size(fsize); match file.set_len(tsize) { Ok(_) => {} Err(f) => crash!(1, "{}", f.to_string()), From 1f1cd3d966cd4317c2eec8e8dfdcfb350f79fcf4 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 21 May 2021 21:12:13 -0400 Subject: [PATCH 5/5] truncate: re-organize into one func for each mode Reorganize the code in `truncate.rs` into three distinct functions representing the three modes of operation of the `truncate` program. The three modes are - `truncate -r RFILE FILE`, which sets the length of `FILE` to match the length of `RFILE`, - `truncate -r RFILE -s NUM FILE`, which sets the length of `FILE` relative to the given `RFILE`, - `truncate -s NUM FILE`, which sets the length of `FILE` either absolutely or relative to its curent length. This organization of the code makes it more concise and easier to follow. --- src/uu/truncate/src/truncate.rs | 196 ++++++++++++++++++++++---------- 1 file changed, 139 insertions(+), 57 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index c0f078458..3a6077b3c 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -17,7 +17,6 @@ use std::path::Path; #[derive(Debug, Eq, PartialEq)] enum TruncateMode { - Reference(u64), Absolute(u64), Extend(u64), Reduce(u64), @@ -42,7 +41,6 @@ impl TruncateMode { fn to_size(&self, fsize: u64) -> u64 { match self { TruncateMode::Absolute(modsize) => *modsize, - TruncateMode::Reference(_) => fsize, TruncateMode::Extend(modsize) => fsize + modsize, TruncateMode::Reduce(modsize) => fsize - modsize, TruncateMode::AtMost(modsize) => fsize.min(*modsize), @@ -142,74 +140,158 @@ pub fn uumain(args: impl uucore::Args) -> i32 { 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); - if reference.is_none() && size.is_none() { - crash!(1, "you must specify either --reference or --size"); - } else { - truncate(no_create, io_blocks, reference, size, files); + if let Err(e) = truncate(no_create, io_blocks, reference, size, files) { + 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); + crash!( + 1, + "cannot stat '{}': No such file or directory", + reference.unwrap() + ); + } + _ => crash!(1, "{}", e.to_string()), + } } } 0 } +/// Truncate the named file to the specified size. +/// +/// If `create` is true, then the file will be created if it does not +/// already exist. If `size` is larger than the number of bytes in the +/// file, then the file will be padded with zeros. If `size` is smaller +/// than the number of bytes in the file, then the file will be +/// truncated and any bytes beyond `size` will be lost. +/// +/// # Errors +/// +/// If the file could not be opened, or there was a problem setting the +/// size of the file. +fn file_truncate(filename: &str, create: bool, size: u64) -> std::io::Result<()> { + let path = Path::new(filename); + let f = OpenOptions::new().write(true).create(create).open(path)?; + f.set_len(size) +} + +/// Truncate files to a size relative to a given file. +/// +/// `rfilename` is the name of the reference file. +/// +/// `size_string` gives the size relative to the reference file to which +/// to set the target files. For example, "+3K" means "set each file to +/// be three kilobytes larger than the size of the reference file". +/// +/// If `create` is true, then each file will be created if it does not +/// already exist. +/// +/// # Errors +/// +/// If the any file could not be opened, or there was a problem setting +/// the size of at least one file. +fn truncate_reference_and_size( + rfilename: &str, + size_string: &str, + filenames: Vec, + create: bool, +) -> std::io::Result<()> { + 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(_) => crash!(1, "Invalid number: ‘{}’", size_string), + }; + let fsize = metadata(rfilename)?.len(); + let tsize = mode.to_size(fsize); + for filename in &filenames { + file_truncate(filename, create, tsize)?; + } + Ok(()) +} + +/// Truncate files to match the size of a given reference file. +/// +/// `rfilename` is the name of the reference file. +/// +/// If `create` is true, then each file will be created if it does not +/// already exist. +/// +/// # Errors +/// +/// If the any file could not be opened, or there was a problem setting +/// the size of at least one file. +fn truncate_reference_file_only( + rfilename: &str, + filenames: Vec, + create: bool, +) -> std::io::Result<()> { + let tsize = metadata(rfilename)?.len(); + for filename in &filenames { + file_truncate(filename, create, tsize)?; + } + Ok(()) +} + +/// Truncate files to a specified size. +/// +/// `size_string` gives either an absolute size or a relative size. A +/// relative size adjusts the size of each file relative to its current +/// size. For example, "3K" means "set each file to be three kilobytes" +/// whereas "+3K" means "set each file to be three kilobytes larger than +/// its current size". +/// +/// If `create` is true, then each file will be created if it does not +/// already exist. +/// +/// # Errors +/// +/// 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(_) => crash!(1, "Invalid number: ‘{}’", size_string), + }; + for filename in &filenames { + let fsize = metadata(filename).map(|m| m.len()).unwrap_or(0); + let tsize = mode.to_size(fsize); + file_truncate(filename, create, tsize)?; + } + Ok(()) +} + fn truncate( no_create: bool, _: bool, reference: Option, size: Option, filenames: Vec, -) { - let mode = match size { - Some(size_string) => match parse_mode_and_size(&size_string) { - Ok(m) => m, - Err(_) => crash!(1, "Invalid number: ‘{}’", size_string), - }, - None => TruncateMode::Reference(0), - }; - - let refsize = match reference { - Some(ref rfilename) => { - match mode { - // Only Some modes work with a reference - TruncateMode::Reference(_) => (), //No --size was given - TruncateMode::Extend(_) => (), - TruncateMode::Reduce(_) => (), - _ => crash!(1, "you must specify a relative ‘--size’ with ‘--reference’"), - }; - match metadata(rfilename) { - Ok(meta) => meta.len(), - Err(f) => match f.kind() { - ErrorKind::NotFound => { - crash!(1, "cannot stat '{}': No such file or directory", rfilename) - } - _ => crash!(1, "{}", f.to_string()), - }, - } - } - None => 0, - }; - for filename in &filenames { - let path = Path::new(filename); - match OpenOptions::new().write(true).create(!no_create).open(path) { - Ok(file) => { - let fsize = match reference { - Some(_) => refsize, - None => match metadata(filename) { - Ok(meta) => meta.len(), - Err(f) => { - show_warning!("{}", f.to_string()); - continue; - } - }, - }; - let tsize = mode.to_size(fsize); - match file.set_len(tsize) { - Ok(_) => {} - Err(f) => crash!(1, "{}", f.to_string()), - }; - } - Err(f) => crash!(1, "{}", f.to_string()), +) -> std::io::Result<()> { + let create = !no_create; + // There are four possibilities + // - reference file given and size given, + // - reference file given but no size given, + // - no reference file given but size given, + // - no reference file given and no size given, + match (reference, size) { + (Some(rfilename), Some(size_string)) => { + truncate_reference_and_size(&rfilename, &size_string, filenames, create) } + (Some(rfilename), None) => truncate_reference_file_only(&rfilename, filenames, create), + (None, Some(size_string)) => truncate_size_only(&size_string, filenames, create), + (None, None) => crash!(1, "you must specify either --reference or --size"), } }