From 59c0f51208da8442ecec2a8a21a9f7f13c0b0916 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 14 Apr 2023 22:58:22 +0200 Subject: [PATCH 1/4] hashsum: split/doc detect_algo into smaller functions --- src/uu/hashsum/src/hashsum.rs | 365 +++++++++++++++++++--------------- 1 file changed, 207 insertions(+), 158 deletions(-) diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index 5678a58b350..44e0cbcd80e 100644 --- a/src/uu/hashsum/src/hashsum.rs +++ b/src/uu/hashsum/src/hashsum.rs @@ -50,71 +50,140 @@ struct Options { zero: bool, } -#[allow(clippy::cognitive_complexity)] +/// Creates a Blake2b hasher instance based on the specified length argument. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. +/// +/// # Panics +/// +/// Panics if the length is not a multiple of 8 or if it is greater than 512. +fn create_blake2b(matches: &ArgMatches) -> (&'static str, Box, usize) { + match matches.get_one::("length") { + Some(0) | None => ("BLAKE2", Box::new(Blake2b::new()) as Box, 512), + Some(length_in_bits) => { + if *length_in_bits > 512 { + crash!(1, "Invalid length (maximum digest length is 512 bits)") + } + + if length_in_bits % 8 == 0 { + let length_in_bytes = length_in_bits / 8; + ( + "BLAKE2", + Box::new(Blake2b::with_output_bytes(length_in_bytes)), + *length_in_bits, + ) + } else { + crash!(1, "Invalid length (expected a multiple of 8)") + } + } + } +} + +/// Creates a SHA3 hasher instance based on the specified bits argument. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. +/// +/// # Panics +/// +/// Panics if an unsupported output size is provided, or if the `--bits` flag is missing. +fn create_sha3(matches: &ArgMatches) -> (&'static str, Box, usize) { + match matches.get_one::("bits") { + Some(224) => ( + "SHA3-224", + Box::new(Sha3_224::new()) as Box, + 224, + ), + Some(256) => ( + "SHA3-256", + Box::new(Sha3_256::new()) as Box, + 256, + ), + Some(384) => ( + "SHA3-384", + Box::new(Sha3_384::new()) as Box, + 384, + ), + Some(512) => ( + "SHA3-512", + Box::new(Sha3_512::new()) as Box, + 512, + ), + Some(_) => crash!( + 1, + "Invalid output size for SHA3 (expected 224, 256, 384, or 512)" + ), + None => crash!(1, "--bits required for SHA3"), + } +} + +/// Creates a SHAKE-128 hasher instance based on the specified bits argument. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. +/// +/// # Panics +/// +/// Panics if the `--bits` flag is missing. +fn create_shake128(matches: &ArgMatches) -> (&'static str, Box, usize) { + match matches.get_one::("bits") { + Some(bits) => ( + "SHAKE128", + Box::new(Shake128::new()) as Box, + *bits, + ), + None => crash!(1, "--bits required for SHAKE-128"), + } +} + +/// Creates a SHAKE-256 hasher instance based on the specified bits argument. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. +/// +/// # Panics +/// +/// Panics if the `--bits` flag is missing. +fn create_shake256(matches: &ArgMatches) -> (&'static str, Box, usize) { + match matches.get_one::("bits") { + Some(bits) => ( + "SHAKE256", + Box::new(Shake256::new()) as Box, + *bits, + ), + None => crash!(1, "--bits required for SHAKE-256"), + } +} + +/// Detects the hash algorithm from the program name or command-line arguments. +/// +/// # Arguments +/// +/// * `program` - A string slice containing the program name. +/// * `matches` - A reference to the `ArgMatches` object containing the command-line arguments. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. fn detect_algo( program: &str, matches: &ArgMatches, ) -> (&'static str, Box, usize) { - let mut alg: Option> = None; - let mut name: &'static str = ""; - let mut output_bits = 0; - match program { + let (name, alg, output_bits) = match program { "md5sum" => ("MD5", Box::new(Md5::new()) as Box, 128), "sha1sum" => ("SHA1", Box::new(Sha1::new()) as Box, 160), "sha224sum" => ("SHA224", Box::new(Sha224::new()) as Box, 224), "sha256sum" => ("SHA256", Box::new(Sha256::new()) as Box, 256), "sha384sum" => ("SHA384", Box::new(Sha384::new()) as Box, 384), "sha512sum" => ("SHA512", Box::new(Sha512::new()) as Box, 512), - "b2sum" => match matches.get_one::("length") { - // by default, blake2 uses 64 bytes (512 bits) - // --length=0 falls back to default behavior - Some(0) | None => ("BLAKE2", Box::new(Blake2b::new()) as Box, 512), - Some(length_in_bits) => { - if *length_in_bits > 512 { - crash!(1, "Invalid length (maximum digest length is 512 bits)") - } - - // blake2 output size must be a multiple of 8 bits - if length_in_bits % 8 == 0 { - let length_in_bytes = length_in_bits / 8; - ( - "BLAKE2", - Box::new(Blake2b::with_output_bytes(length_in_bytes)), - *length_in_bits, - ) - } else { - crash!(1, "Invalid length (expected a multiple of 8)") - } - } - }, + "b2sum" => create_blake2b(matches), "b3sum" => ("BLAKE3", Box::new(Blake3::new()) as Box, 256), - "sha3sum" => match matches.get_one::("bits") { - Some(224) => ( - "SHA3-224", - Box::new(Sha3_224::new()) as Box, - 224, - ), - Some(256) => ( - "SHA3-256", - Box::new(Sha3_256::new()) as Box, - 256, - ), - Some(384) => ( - "SHA3-384", - Box::new(Sha3_384::new()) as Box, - 384, - ), - Some(512) => ( - "SHA3-512", - Box::new(Sha3_512::new()) as Box, - 512, - ), - Some(_) => crash!( - 1, - "Invalid output size for SHA3 (expected 224, 256, 384, or 512)" - ), - None => crash!(1, "--bits required for SHA3"), - }, + "sha3sum" => create_sha3(matches), "sha3-224sum" => ( "SHA3-224", Box::new(Sha3_224::new()) as Box, @@ -135,114 +204,94 @@ fn detect_algo( Box::new(Sha3_512::new()) as Box, 512, ), - "shake128sum" => match matches.get_one::("bits") { - Some(bits) => ( - "SHAKE128", - Box::new(Shake128::new()) as Box, - *bits, - ), + "shake128sum" => create_shake128(matches), + "shake256sum" => create_shake256(matches), + _ => create_algorithm_from_flags(matches), + }; + (name, alg, output_bits) +} + +/// Creates a hasher instance based on the command-line flags. +/// +/// # Arguments +/// +/// * `matches` - A reference to the `ArgMatches` object containing the command-line arguments. +/// +/// # Returns +/// +/// Returns a tuple containing the algorithm name, the hasher instance, and the output length in bits. +/// +/// # Panics +/// +/// Panics if multiple hash algorithms are specified or if a required flag is missing. +fn create_algorithm_from_flags(matches: &ArgMatches) -> (&'static str, Box, usize) { + let mut alg: Option> = None; + let mut name: &'static str = ""; + let mut output_bits = 0; + let mut set_or_crash = |n, val, bits| { + if alg.is_some() { + crash!(1, "You cannot combine multiple hash algorithms!"); + }; + name = n; + alg = Some(val); + output_bits = bits; + }; + + if matches.get_flag("md5") { + set_or_crash("MD5", Box::new(Md5::new()), 128); + } + if matches.get_flag("sha1") { + set_or_crash("SHA1", Box::new(Sha1::new()), 160); + } + if matches.get_flag("sha224") { + set_or_crash("SHA224", Box::new(Sha224::new()), 224); + } + if matches.get_flag("sha256") { + set_or_crash("SHA256", Box::new(Sha256::new()), 256); + } + if matches.get_flag("sha384") { + set_or_crash("SHA384", Box::new(Sha384::new()), 384); + } + if matches.get_flag("sha512") { + set_or_crash("SHA512", Box::new(Sha512::new()), 512); + } + if matches.get_flag("b2sum") { + set_or_crash("BLAKE2", Box::new(Blake2b::new()), 512); + } + if matches.get_flag("b3sum") { + set_or_crash("BLAKE3", Box::new(Blake3::new()), 256); + } + if matches.get_flag("sha3") { + let (n, val, bits) = create_sha3(matches); + set_or_crash(n, val, bits); + } + if matches.get_flag("sha3-224") { + set_or_crash("SHA3-224", Box::new(Sha3_224::new()), 224); + } + if matches.get_flag("sha3-256") { + set_or_crash("SHA3-256", Box::new(Sha3_256::new()), 256); + } + if matches.get_flag("sha3-384") { + set_or_crash("SHA3-384", Box::new(Sha3_384::new()), 384); + } + if matches.get_flag("sha3-512") { + set_or_crash("SHA3-512", Box::new(Sha3_512::new()), 512); + } + if matches.get_flag("shake128") { + match matches.get_one::("bits") { + Some(bits) => set_or_crash("SHAKE128", Box::new(Shake128::new()), *bits), None => crash!(1, "--bits required for SHAKE-128"), - }, - "shake256sum" => match matches.get_one::("bits") { - Some(bits) => ( - "SHAKE256", - Box::new(Shake256::new()) as Box, - *bits, - ), + } + } + if matches.get_flag("shake256") { + match matches.get_one::("bits") { + Some(bits) => set_or_crash("SHAKE256", Box::new(Shake256::new()), *bits), None => crash!(1, "--bits required for SHAKE-256"), - }, - _ => { - { - let mut set_or_crash = |n, val, bits| { - if alg.is_some() { - crash!(1, "You cannot combine multiple hash algorithms!") - }; - name = n; - alg = Some(val); - output_bits = bits; - }; - if matches.get_flag("md5") { - set_or_crash("MD5", Box::new(Md5::new()), 128); - } - if matches.get_flag("sha1") { - set_or_crash("SHA1", Box::new(Sha1::new()), 160); - } - if matches.get_flag("sha224") { - set_or_crash("SHA224", Box::new(Sha224::new()), 224); - } - if matches.get_flag("sha256") { - set_or_crash("SHA256", Box::new(Sha256::new()), 256); - } - if matches.get_flag("sha384") { - set_or_crash("SHA384", Box::new(Sha384::new()), 384); - } - if matches.get_flag("sha512") { - set_or_crash("SHA512", Box::new(Sha512::new()), 512); - } - if matches.get_flag("b2sum") { - set_or_crash("BLAKE2", Box::new(Blake2b::new()), 512); - } - if matches.get_flag("b3sum") { - set_or_crash("BLAKE3", Box::new(Blake3::new()), 256); - } - if matches.get_flag("sha3") { - match matches.get_one::("bits") { - Some(224) => set_or_crash( - "SHA3-224", - Box::new(Sha3_224::new()) as Box, - 224, - ), - Some(256) => set_or_crash( - "SHA3-256", - Box::new(Sha3_256::new()) as Box, - 256, - ), - Some(384) => set_or_crash( - "SHA3-384", - Box::new(Sha3_384::new()) as Box, - 384, - ), - Some(512) => set_or_crash( - "SHA3-512", - Box::new(Sha3_512::new()) as Box, - 512, - ), - Some(_) => crash!( - 1, - "Invalid output size for SHA3 (expected 224, 256, 384, or 512)" - ), - None => crash!(1, "--bits required for SHA3"), - } - } - if matches.get_flag("sha3-224") { - set_or_crash("SHA3-224", Box::new(Sha3_224::new()), 224); - } - if matches.get_flag("sha3-256") { - set_or_crash("SHA3-256", Box::new(Sha3_256::new()), 256); - } - if matches.get_flag("sha3-384") { - set_or_crash("SHA3-384", Box::new(Sha3_384::new()), 384); - } - if matches.get_flag("sha3-512") { - set_or_crash("SHA3-512", Box::new(Sha3_512::new()), 512); - } - if matches.get_flag("shake128") { - match matches.get_one::("bits") { - Some(bits) => set_or_crash("SHAKE128", Box::new(Shake128::new()), *bits), - None => crash!(1, "--bits required for SHAKE-128"), - } - } - if matches.get_flag("shake256") { - match matches.get_one::("bits") { - Some(bits) => set_or_crash("SHAKE256", Box::new(Shake256::new()), *bits), - None => crash!(1, "--bits required for SHAKE-256"), - } - } - } - let alg = alg.unwrap_or_else(|| crash!(1, "You must specify hash algorithm!")); - (name, alg, output_bits) } } + + let alg = alg.unwrap_or_else(|| crash!(1, "You must specify hash algorithm!")); + (name, alg, output_bits) } // TODO: return custom error type From fb72738db4c06162648993cd5c2a5cde1f4b8f66 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 14 Apr 2023 22:58:40 +0200 Subject: [PATCH 2/4] stat: split/doc print_it into smaller functions --- src/uu/stat/src/stat.rs | 216 +++++++++++++++++++++++++++++----------- 1 file changed, 160 insertions(+), 56 deletions(-) diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index 586803db4a0..69f3c276084 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -205,7 +205,16 @@ struct Stater { default_dev_tokens: Vec, } -#[allow(clippy::cognitive_complexity)] +/// Prints a formatted output based on the provided output type, flags, width, and precision. +/// +/// # Arguments +/// +/// * `output` - A reference to the OutputType enum containing the value to be printed. +/// * `flags` - A Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed output. +/// * `precision` - An Option containing the precision value. +/// +/// This function delegates the printing process to more specialized functions depending on the output type. fn print_it(output: &OutputType, flags: Flags, width: usize, precision: Option) { // If the precision is given as just '.', the precision is taken to be zero. // A negative precision is taken as if the precision were omitted. @@ -236,71 +245,166 @@ fn print_it(output: &OutputType, flags: Flags, width: usize, precision: Option { - let s = match precision { - Some(p) if p < s.len() => &s[..p], - _ => s, - }; - pad_and_print(s, flags.left, width, Padding::Space); - } - OutputType::Integer(num) => { - let num = num.to_string(); - let arg = if flags.group { - group_num(&num) - } else { - Cow::Borrowed(num.as_str()) - }; - let prefix = if flags.sign { - "+" - } else if flags.space { - " " - } else { - "" - }; - let extended = format!( - "{prefix}{arg:0>precision$}", - precision = precision.unwrap_or(0) - ); - pad_and_print(&extended, flags.left, width, padding_char); - } - OutputType::Unsigned(num) => { - let num = num.to_string(); - let s = if flags.group { - group_num(&num) - } else { - Cow::Borrowed(num.as_str()) - }; - let s = format!("{s:0>precision$}", precision = precision.unwrap_or(0)); - pad_and_print(&s, flags.left, width, padding_char); - } + OutputType::Str(s) => print_str(s, &flags, width, precision), + OutputType::Integer(num) => print_integer(*num, &flags, width, precision, padding_char), + OutputType::Unsigned(num) => print_unsigned(*num, &flags, width, precision, padding_char), OutputType::UnsignedOct(num) => { - let prefix = if flags.alter { "0" } else { "" }; - let s = format!( - "{prefix}{num:0>precision$o}", - precision = precision.unwrap_or(0) - ); - pad_and_print(&s, flags.left, width, padding_char); + print_unsigned_oct(*num, &flags, width, precision, padding_char); } OutputType::UnsignedHex(num) => { - let prefix = if flags.alter { "0x" } else { "" }; - let s = format!( - "{prefix}{num:0>precision$x}", - precision = precision.unwrap_or(0) - ); - pad_and_print(&s, flags.left, width, padding_char); + print_unsigned_hex(*num, &flags, width, precision, padding_char); } OutputType::Unknown => print!("?"), } } +/// Determines the padding character based on the provided flags and precision. +/// +/// # Arguments +/// +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `precision` - An Option containing the precision value. +/// +/// # Returns +/// +/// * Padding - An instance of the Padding enum representing the padding character. +fn determine_padding_char(flags: &Flags, precision: &Option) -> Padding { + if flags.zero && !flags.left && precision.is_none() { + Padding::Zero + } else { + Padding::Space + } +} + +/// Prints a string value based on the provided flags, width, and precision. +/// +/// # Arguments +/// +/// * `s` - The string to be printed. +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed string. +/// * `precision` - An Option containing the precision value. +fn print_str(s: &str, flags: &Flags, width: usize, precision: Option) { + let s = match precision { + Some(p) if p < s.len() => &s[..p], + _ => s, + }; + pad_and_print(s, flags.left, width, Padding::Space); +} + +/// Prints an integer value based on the provided flags, width, and precision. +/// +/// # Arguments +/// +/// * `num` - The integer value to be printed. +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed integer. +/// * `precision` - An Option containing the precision value. +/// * `padding_char` - The padding character as determined by `determine_padding_char`. +fn print_integer( + num: i64, + flags: &Flags, + width: usize, + precision: Option, + padding_char: Padding, +) { + let num = num.to_string(); + let arg = if flags.group { + group_num(&num) + } else { + Cow::Borrowed(num.as_str()) + }; + let prefix = if flags.sign { + "+" + } else if flags.space { + " " + } else { + "" + }; + let extended = format!( + "{prefix}{arg:0>precision$}", + precision = precision.unwrap_or(0) + ); + pad_and_print(&extended, flags.left, width, padding_char); +} + +/// Prints an unsigned integer value based on the provided flags, width, and precision. +/// +/// # Arguments +/// +/// * `num` - The unsigned integer value to be printed. +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed unsigned integer. +/// * `precision` - An Option containing the precision value. +/// * `padding_char` - The padding character as determined by `determine_padding_char`. +fn print_unsigned( + num: u64, + flags: &Flags, + width: usize, + precision: Option, + padding_char: Padding, +) { + let num = num.to_string(); + let s = if flags.group { + group_num(&num) + } else { + Cow::Borrowed(num.as_str()) + }; + let s = format!("{s:0>precision$}", precision = precision.unwrap_or(0)); + pad_and_print(&s, flags.left, width, padding_char); +} + +/// Prints an unsigned octal integer value based on the provided flags, width, and precision. +/// +/// # Arguments +/// +/// * `num` - The unsigned octal integer value to be printed. +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed unsigned octal integer. +/// * `precision` - An Option containing the precision value. +/// * `padding_char` - The padding character as determined by `determine_padding_char`. +fn print_unsigned_oct( + num: u32, + flags: &Flags, + width: usize, + precision: Option, + padding_char: Padding, +) { + let prefix = if flags.alter { "0" } else { "" }; + let s = format!( + "{prefix}{num:0>precision$o}", + precision = precision.unwrap_or(0) + ); + pad_and_print(&s, flags.left, width, padding_char); +} + +/// Prints an unsigned hexadecimal integer value based on the provided flags, width, and precision. +/// +/// # Arguments +/// +/// * `num` - The unsigned hexadecimal integer value to be printed. +/// * `flags` - A reference to the Flags struct containing formatting flags. +/// * `width` - The width of the field for the printed unsigned hexadecimal integer. +/// * `precision` - An Option containing the precision value. +/// * `padding_char` - The padding character as determined by `determine_padding_char`. +fn print_unsigned_hex( + num: u64, + flags: &Flags, + width: usize, + precision: Option, + padding_char: Padding, +) { + let prefix = if flags.alter { "0x" } else { "" }; + let s = format!( + "{prefix}{num:0>precision$x}", + precision = precision.unwrap_or(0) + ); + pad_and_print(&s, flags.left, width, padding_char); +} + impl Stater { fn generate_tokens(format_str: &str, use_printf: bool) -> UResult> { let mut tokens = Vec::new(); From 2c1aa229a0ffbe99a43fb93e1de23934341e03e2 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 14 Apr 2023 23:20:23 +0200 Subject: [PATCH 3/4] install: split/doc copy into smaller functions --- src/uu/install/src/install.rs | 167 +++++++++++++++++++++++++--------- 1 file changed, 123 insertions(+), 44 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 039aa1b1538..e0307fe34b7 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -682,36 +682,23 @@ fn chown_optional_user_group(path: &Path, b: &Behavior) -> UResult<()> { Ok(()) } -/// Copy one file to a new location, changing metadata. -/// -/// Returns a Result type with the Err variant containing the error message. +/// Perform backup before overwriting. /// /// # Parameters /// -/// _from_ must exist as a non-directory. -/// _to_ must be a non-existent file, whose parent directory exists. +/// * `to` - The destination file path. +/// * `b` - The behavior configuration. /// -/// # Errors +/// # Returns /// -/// If the copy system call fails, we print a verbose error and return an empty error value. +/// Returns an Option containing the backup path, or None if backup is not needed. /// -#[allow(clippy::cognitive_complexity)] -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. - let mut backup_path = None; - - // Perform backup, if any, before overwriting 'to' - // - // The codes actually making use of the backup process don't seem to agree - // on how best to approach the issue. (mv and ln, for example) +fn perform_backup(to: &Path, b: &Behavior) -> UResult> { if to.exists() { if b.verbose { println!("removed {}", to.quote()); } - backup_path = backup_control::get_backup_path(b.backup_mode, to, &b.suffix); + let backup_path = backup_control::get_backup_path(b.backup_mode, to, &b.suffix); if let Some(ref backup_path) = backup_path { // TODO!! if let Err(err) = fs::rename(to, backup_path) { @@ -723,8 +710,24 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { .into()); } } + Ok(backup_path) + } else { + Ok(None) } +} +/// Copy a file from one path to another. +/// +/// # Parameters +/// +/// * `from` - The source file path. +/// * `to` - The destination file path. +/// +/// # Returns +/// +/// Returns an empty Result or an error in case of failure. +/// +fn copy_file(from: &Path, to: &Path) -> UResult<()> { if from.as_os_str() == "/dev/null" { /* workaround a limitation of fs::copy * https://github.com/rust-lang/rust/issues/79390 @@ -737,27 +740,53 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { } else if let Err(err) = fs::copy(from, to) { return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into()); } + Ok(()) +} - if b.strip && cfg!(not(windows)) { - match process::Command::new(&b.strip_program).arg(to).output() { - Ok(o) => { - if !o.status.success() { - // Follow GNU's behavior: if strip fails, removes the target - let _ = fs::remove_file(to); - return Err(InstallError::StripProgramFailed( - String::from_utf8(o.stderr).unwrap_or_default(), - ) - .into()); - } - } - Err(e) => { +/// Strip a file using an external program. +/// +/// # Parameters +/// +/// * `to` - The destination file path. +/// * `b` - The behavior configuration. +/// +/// # Returns +/// +/// Returns an empty Result or an error in case of failure. +/// +fn strip_file(to: &Path, b: &Behavior) -> UResult<()> { + match process::Command::new(&b.strip_program).arg(to).output() { + Ok(o) => { + if !o.status.success() { // Follow GNU's behavior: if strip fails, removes the target let _ = fs::remove_file(to); - return Err(InstallError::StripProgramFailed(e.to_string()).into()); + return Err(InstallError::StripProgramFailed( + String::from_utf8(o.stderr).unwrap_or_default(), + ) + .into()); } } + Err(e) => { + // Follow GNU's behavior: if strip fails, removes the target + let _ = fs::remove_file(to); + return Err(InstallError::StripProgramFailed(e.to_string()).into()); + } } + Ok(()) +} +/// Set ownership and permissions on the destination file. +/// +/// # Parameters +/// +/// * `to` - The destination file path. +/// * `b` - The behavior configuration. +/// +/// # Returns +/// +/// Returns an empty Result or an error in case of failure. +/// +fn set_ownership_and_permissions(to: &Path, b: &Behavior) -> UResult<()> { // Silent the warning as we want to the error message #[allow(clippy::question_mark)] if mode::chmod(to, b.mode()).is_err() { @@ -766,20 +795,70 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> { chown_optional_user_group(to, b)?; - if b.preserve_timestamps { - let meta = match fs::metadata(from) { - Ok(meta) => meta, - Err(e) => return Err(InstallError::MetadataFailed(e).into()), - }; + Ok(()) +} - let modified_time = FileTime::from_last_modification_time(&meta); - let accessed_time = FileTime::from_last_access_time(&meta); +/// Preserve timestamps on the destination file. +/// +/// # Parameters +/// +/// * `from` - The source file path. +/// * `to` - The destination file path. +/// +/// # Returns +/// +/// Returns an empty Result or an error in case of failure. +/// +fn preserve_timestamps(from: &Path, to: &Path) -> UResult<()> { + let meta = match fs::metadata(from) { + Ok(meta) => meta, + Err(e) => return Err(InstallError::MetadataFailed(e).into()), + }; - match set_file_times(to, accessed_time, modified_time) { - Ok(_) => {} - Err(e) => show_error!("{}", e), + let modified_time = FileTime::from_last_modification_time(&meta); + let accessed_time = FileTime::from_last_access_time(&meta); + + match set_file_times(to, accessed_time, modified_time) { + Ok(_) => Ok(()), + Err(e) => { + show_error!("{}", e); + 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. +/// _to_ must be a non-existent file, whose parent directory exists. +/// +/// # Errors +/// +/// If the copy system call fails, we print a verbose error and return an empty error value. +/// +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. + let backup_path = perform_backup(to, b)?; + + copy_file(from, to)?; + + #[cfg(not(windows))] + if b.strip { + strip_file(to, b)?; + } + + set_ownership_and_permissions(to, b)?; + + if b.preserve_timestamps { + preserve_timestamps(from, to)?; + } if b.verbose { print!("{} -> {}", from.quote(), to.quote()); From 25fbcd89a5fab08cc4dc5aac45477b3f2b262317 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 14 Apr 2023 23:36:51 +0200 Subject: [PATCH 4/4] fmt:uumain: split/doc copy into smaller functions --- src/uu/fmt/src/fmt.rs | 103 ++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 34 deletions(-) diff --git a/src/uu/fmt/src/fmt.rs b/src/uu/fmt/src/fmt.rs index bd03b5497ad..0fd863730fa 100644 --- a/src/uu/fmt/src/fmt.rs +++ b/src/uu/fmt/src/fmt.rs @@ -11,7 +11,7 @@ use clap::{crate_version, Arg, ArgAction, Command}; use std::cmp; use std::fs::File; use std::io::{stdin, stdout, Write}; -use std::io::{BufReader, BufWriter, Read}; +use std::io::{BufReader, BufWriter, Read, Stdout}; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError}; use uucore::{format_usage, help_about, help_usage, show_warning}; @@ -60,10 +60,16 @@ pub struct FmtOptions { goal: usize, tabwidth: usize, } - -#[uucore::main] -#[allow(clippy::cognitive_complexity)] -pub fn uumain(args: impl uucore::Args) -> UResult<()> { +/// Parse the command line arguments and return the list of files and formatting options. +/// +/// # Arguments +/// +/// * `args` - Command line arguments. +/// +/// # Returns +/// +/// A tuple containing a vector of file names and a `FmtOptions` struct. +fn parse_arguments(args: impl uucore::Args) -> UResult<(Vec, FmtOptions)> { let matches = uu_app().try_get_matches_from(args)?; let mut files: Vec = matches @@ -177,39 +183,68 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { files.push("-".to_owned()); } - let mut ostream = BufWriter::new(stdout()); + Ok((files, fmt_opts)) +} - for i in files.iter().map(|x| &x[..]) { - let mut fp = match i { - "-" => BufReader::new(Box::new(stdin()) as Box), - _ => match File::open(i) { - Ok(f) => BufReader::new(Box::new(f) as Box), - Err(e) => { - show_warning!("{}: {}", i.maybe_quote(), e); - continue; - } - }, - }; - let p_stream = ParagraphStream::new(&fmt_opts, &mut fp); - for para_result in p_stream { - match para_result { - Err(s) => { - ostream - .write_all(s.as_bytes()) - .map_err_context(|| "failed to write output".to_string())?; - ostream - .write_all(b"\n") - .map_err_context(|| "failed to write output".to_string())?; - } - Ok(para) => break_lines(¶, &fmt_opts, &mut ostream) - .map_err_context(|| "failed to write output".to_string())?, +/// Process the content of a file and format it according to the provided options. +/// +/// # Arguments +/// +/// * `file_name` - The name of the file to process. A value of "-" represents the standard input. +/// * `fmt_opts` - A reference to a `FmtOptions` struct containing the formatting options. +/// * `ostream` - A mutable reference to a `BufWriter` wrapping the standard output. +/// +/// # Returns +/// +/// A `UResult<()>` indicating success or failure. +fn process_file( + file_name: &str, + fmt_opts: &FmtOptions, + ostream: &mut BufWriter, +) -> UResult<()> { + let mut fp = match file_name { + "-" => BufReader::new(Box::new(stdin()) as Box), + _ => match File::open(file_name) { + Ok(f) => BufReader::new(Box::new(f) as Box), + Err(e) => { + show_warning!("{}: {}", file_name.maybe_quote(), e); + return Ok(()); + } + }, + }; + + let p_stream = ParagraphStream::new(fmt_opts, &mut fp); + for para_result in p_stream { + match para_result { + Err(s) => { + ostream + .write_all(s.as_bytes()) + .map_err_context(|| "failed to write output".to_string())?; + ostream + .write_all(b"\n") + .map_err_context(|| "failed to write output".to_string())?; } + Ok(para) => break_lines(¶, fmt_opts, ostream) + .map_err_context(|| "failed to write output".to_string())?, } + } + + // flush the output after each file + ostream + .flush() + .map_err_context(|| "failed to write output".to_string())?; + + Ok(()) +} + +#[uucore::main] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let (files, fmt_opts) = parse_arguments(args)?; + + let mut ostream = BufWriter::new(stdout()); - // flush the output after each file - ostream - .flush() - .map_err_context(|| "failed to write output".to_string())?; + for file_name in &files { + process_file(file_name, &fmt_opts, &mut ostream)?; } Ok(())