From a1ad751aa9738e0285491dd36ce92d6ce1718456 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 3 Mar 2024 20:14:05 +0100 Subject: [PATCH] truncate: deduplicate fifo check, fix handling of missing files The fifo check used to include 'metadata(filename)?', which would error if the file does not exist. In our case however, this is not an error. --- src/uu/truncate/src/truncate.rs | 46 +++++++++++++-------------------- tests/by-util/test_truncate.rs | 32 +++++++++++++++++++++-- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index c0e38f59b..7af25085f 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/src/uu/truncate/src/truncate.rs @@ -178,13 +178,26 @@ pub fn uu_app() -> Command { /// /// 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<()> { +fn file_truncate(filename: &str, create: bool, size: u64) -> UResult<()> { + #[cfg(unix)] + if let Ok(metadata) = std::fs::metadata(filename) { + if metadata.file_type().is_fifo() { + return Err(USimpleError::new( + 1, + format!( + "cannot open {} for writing: No such device or address", + filename.quote() + ), + )); + } + } let path = Path::new(filename); match OpenOptions::new().write(true).create(create).open(path) { Ok(file) => file.set_len(size), Err(e) if e.kind() == ErrorKind::NotFound && !create => Ok(()), Err(e) => Err(e), } + .map_err_context(|| format!("cannot open {} for writing", filename.quote())) } /// Truncate files to a size relative to a given file. @@ -236,19 +249,7 @@ fn truncate_reference_and_size( let fsize = metadata.len(); let tsize = mode.to_size(fsize); for filename in filenames { - #[cfg(unix)] - if std::fs::metadata(filename)?.file_type().is_fifo() { - return Err(USimpleError::new( - 1, - format!( - "cannot open {} for writing: No such device or address", - filename.quote() - ), - )); - } - - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + file_truncate(filename, create, tsize)?; } Ok(()) } @@ -283,18 +284,7 @@ fn truncate_reference_file_only( })?; let tsize = metadata.len(); for filename in filenames { - #[cfg(unix)] - if std::fs::metadata(filename)?.file_type().is_fifo() { - return Err(USimpleError::new( - 1, - format!( - "cannot open {} for writing: No such device or address", - filename.quote() - ), - )); - } - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + file_truncate(filename, create, tsize)?; } Ok(()) } @@ -340,8 +330,8 @@ fn truncate_size_only(size_string: &str, filenames: &[String], create: bool) -> Err(_) => 0, }; let tsize = mode.to_size(fsize); - file_truncate(filename, create, tsize) - .map_err_context(|| format!("cannot open {} for writing", filename.quote()))?; + // TODO: Fix duplicate call to stat + file_truncate(filename, create, tsize)?; } Ok(()) } diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index 41a28736d..e6a128186 100644 --- a/tests/by-util/test_truncate.rs +++ b/tests/by-util/test_truncate.rs @@ -266,6 +266,36 @@ fn test_new_file() { assert_eq!(at.read_bytes(filename), vec![b'\0'; 8]); } +/// Test that truncating a non-existent file creates that file, even in reference-mode. +#[test] +fn test_new_file_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-r", FILE1, filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(at.file_exists(filename)); + assert_eq!(at.read_bytes(filename), vec![b'\0'; 10]); +} + +/// Test that truncating a non-existent file creates that file, even in size-and-reference-mode. +#[test] +fn test_new_file_size_and_reference() { + let (at, mut ucmd) = at_and_ucmd!(); + let mut old_file = at.make_file(FILE1); + old_file.write_all(b"1234567890").unwrap(); + let filename = "new_file_that_does_not_exist_yet"; + ucmd.args(&["-s", "+3", "-r", FILE1, filename]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(at.file_exists(filename)); + assert_eq!(at.read_bytes(filename), vec![b'\0'; 13]); +} + /// Test for not creating a non-existent file. #[test] fn test_new_file_no_create_size_only() { @@ -280,7 +310,6 @@ fn test_new_file_no_create_size_only() { /// Test for not creating a non-existent file. #[test] -#[ignore = "other bug"] fn test_new_file_no_create_reference_only() { let (at, mut ucmd) = at_and_ucmd!(); let mut old_file = at.make_file(FILE1); @@ -295,7 +324,6 @@ fn test_new_file_no_create_reference_only() { /// Test for not creating a non-existent file. #[test] -#[ignore = "other bug"] fn test_new_file_no_create_size_and_reference() { let (at, mut ucmd) = at_and_ucmd!(); let mut old_file = at.make_file(FILE1);