From 791ae269571095ea553c7c4230687f92e9dad42d Mon Sep 17 00:00:00 2001 From: Jeff Bailey Date: Mon, 15 Jun 2026 07:08:54 +0100 Subject: [PATCH] refactor: reduce cognitive complexity of create and list operations --- src/uu/tar/src/operations/create.rs | 74 +++++++++++--------- src/uu/tar/src/operations/list.rs | 104 +++++++++++++--------------- 2 files changed, 90 insertions(+), 88 deletions(-) diff --git a/src/uu/tar/src/operations/create.rs b/src/uu/tar/src/operations/create.rs index 41923c9..600cdc7 100644 --- a/src/uu/tar/src/operations/create.rs +++ b/src/uu/tar/src/operations/create.rs @@ -56,42 +56,10 @@ pub fn create_archive( } if verbose { - let to_print = get_tree(path)? - .iter() - .map(|p| (p.is_dir(), p.display().to_string())) - .map(|(is_dir, path)| { - if is_dir { - format!("{}{}", path, path::MAIN_SEPARATOR) - } else { - path - } - }) - .collect::>() - .join("\n"); - writeln!(status_output, "{to_print}").map_err(TarError::Io)?; + print_verbose_tree(&mut status_output, path)?; } - // Normalize path if needed (so far, handles only absolute paths) - let normalized_name = if let Some(normalized) = normalize_path(path, allow_absolute) { - let original_components: Vec = path.components().collect(); - let normalized_components: Vec = normalized.components().collect(); - if original_components.len() > normalized_components.len() { - let removed: PathBuf = original_components - [..original_components.len() - normalized_components.len()] - .iter() - .collect(); - writeln!( - std::io::stderr(), - "tar: Removing leading `{}' from member names", - removed.display() - ) - .map_err(TarError::Io)?; - } - - normalized - } else { - path.to_path_buf() - }; + let normalized_name = get_normalized_path(path, allow_absolute)?; // If it's a directory, recursively add all contents if path.is_dir() { @@ -125,6 +93,44 @@ pub fn create_archive( Ok(()) } +fn print_verbose_tree(status_output: &mut impl Write, path: &Path) -> UResult<()> { + let to_print = get_tree(path)? + .iter() + .map(|p| (p.is_dir(), p.display().to_string())) + .map(|(is_dir, path)| { + if is_dir { + format!("{}{}", path, path::MAIN_SEPARATOR) + } else { + path + } + }) + .collect::>() + .join("\n"); + writeln!(status_output, "{to_print}").map_err(TarError::Io)?; + Ok(()) +} + +fn get_normalized_path(path: &Path, allow_absolute: bool) -> Result { + if let Some(normalized) = normalize_path(path, allow_absolute) { + let original_components: Vec = path.components().collect(); + let normalized_components: Vec = normalized.components().collect(); + if original_components.len() > normalized_components.len() { + let removed: PathBuf = original_components + [..original_components.len() - normalized_components.len()] + .iter() + .collect(); + writeln!( + std::io::stderr(), + "tar: Removing leading `{}' from member names", + removed.display() + )?; + } + Ok(normalized) + } else { + Ok(path.to_path_buf()) + } +} + fn get_tree(path: &Path) -> Result, std::io::Error> { let mut paths = Vec::new(); let mut stack = VecDeque::new(); diff --git a/src/uu/tar/src/operations/list.rs b/src/uu/tar/src/operations/list.rs index 162711c..9dd8c54 100644 --- a/src/uu/tar/src/operations/list.rs +++ b/src/uu/tar/src/operations/list.rs @@ -28,62 +28,10 @@ pub fn list_archive( let entry = entry_result.map_err(TarError::CannotReadEntry)?; if verbose { - // Collect all header fields into owned values before borrowing entry for the path, - // since both header() and path() require a borrow of entry. - let (mode, entry_type, owner, group, size, mtime) = { - let header = entry.header(); - ( - header.mode().unwrap_or(0), - header.entry_type(), - header - .username() - .ok() - .flatten() - .unwrap_or_default() - .to_owned(), - header - .groupname() - .ok() - .flatten() - .unwrap_or_default() - .to_owned(), - header.size().unwrap_or(0), - header.mtime().unwrap_or(0), - ) - }; - - let path = entry.path().map_err(TarError::CannotReadEntryPath)?; - - let type_char = match entry_type { - tar::EntryType::Directory => 'd', - tar::EntryType::Symlink => 'l', - tar::EntryType::Char => 'c', - tar::EntryType::Block => 'b', - tar::EntryType::Fifo => 'p', - _ => '-', - }; - // Tar headers store the type separately from the mode bits, so we get the - // 9-character rwx string from uucore and prepend our own type character. - let perm_str = display_permissions_unix(mode, false); - let permissions = format!("{type_char}{perm_str}"); - - // TODO: GNU tar displays mtime in the user's local timezone; we - // currently format in UTC. Convert to local time for compatibility. - let dt: chrono::DateTime = Utc - .timestamp_opt(mtime as i64, 0) - .single() - .unwrap_or_else(Utc::now); - let date_str = dt.format("%Y-%m-%d %H:%M"); - - writeln!( - out, - "{permissions} {owner}/{group} {size:>8} {date_str} {}", - path.display() - ) - .map_err(TarError::Io)?; + let formatted = format_verbose_entry(&entry)?; + writeln!(out, "{formatted}").map_err(TarError::Io)?; } else { let path = entry.path().map_err(TarError::CannotReadEntryPath)?; - writeln!(out, "{}", path.display()).map_err(TarError::Io)?; } } @@ -92,6 +40,54 @@ pub fn list_archive( Ok(()) } +fn format_verbose_entry(entry: &tar::Entry<'_, R>) -> Result { + let (mode, entry_type, owner, group, size, mtime) = { + let header = entry.header(); + ( + header.mode().unwrap_or(0), + header.entry_type(), + header + .username() + .ok() + .flatten() + .unwrap_or_default() + .to_owned(), + header + .groupname() + .ok() + .flatten() + .unwrap_or_default() + .to_owned(), + header.size().unwrap_or(0), + header.mtime().unwrap_or(0), + ) + }; + + let path = entry.path().map_err(TarError::CannotReadEntryPath)?; + + let type_char = match entry_type { + tar::EntryType::Directory => 'd', + tar::EntryType::Symlink => 'l', + tar::EntryType::Char => 'c', + tar::EntryType::Block => 'b', + tar::EntryType::Fifo => 'p', + _ => '-', + }; + let perm_str = display_permissions_unix(mode, false); + let permissions = format!("{type_char}{perm_str}"); + + let dt: chrono::DateTime = Utc + .timestamp_opt(mtime as i64, 0) + .single() + .unwrap_or_else(Utc::now); + let date_str = dt.format("%Y-%m-%d %H:%M"); + + Ok(format!( + "{permissions} {owner}/{group} {size:>8} {date_str} {}", + path.display() + )) +} + #[cfg(test)] mod tests { use super::*;