From 6c2789308bc2849876a0bba2f99c1af496392a14 Mon Sep 17 00:00:00 2001 From: "Guillem L. Jara" <4lon3ly0@tutanota.com> Date: Fri, 6 Mar 2026 10:37:42 +0100 Subject: [PATCH 1/3] chore(factor): deduplicate printing code Now there is a single polymorphic function that just takes displayable data, not concrete datatypes. It's two opaque Display types since one of the cases has mismatched number types. --- src/uu/factor/src/factor.rs | 62 +++++-------------------------------- 1 file changed, 8 insertions(+), 54 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 602359ceee2..aa86330dc91 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -48,7 +48,7 @@ fn write_factors_str( // use num_prime's factorize64 algorithm for u64 integers if x <= BigUint::from_u64(u64::MAX).unwrap() { let prime_factors = num_prime::nt_funcs::factorize64(x.clone().to_u64_digits()[0]); - write_result_u64(w, &x, prime_factors, print_exponents) + write_result(w, &x, prime_factors, print_exponents) .map_err_context(|| translate!("factor-error-write-error"))?; } // use num_prime's factorize128 algorithm for u128 integers @@ -59,7 +59,7 @@ fn write_factors_str( return Ok(()); }; let prime_factors = num_prime::nt_funcs::factorize128(x); - write_result_u128(w, &x, prime_factors, print_exponents) + write_result(w, &x, prime_factors, print_exponents) .map_err_context(|| translate!("factor-error-write-error"))?; } // use num_prime's fallible factorization for anything greater than u128::MAX @@ -71,12 +71,12 @@ fn write_factors_str( translate!("factor-error-factorization-incomplete"), )); } - write_result_big_uint(w, &x, prime_factors, print_exponents) + write_result(w, &x, prime_factors, print_exponents) .map_err_context(|| translate!("factor-error-write-error"))?; } } else { let empty_primes: BTreeMap = BTreeMap::new(); - write_result_big_uint(w, &x, empty_primes, print_exponents) + write_result(w, &x, empty_primes, print_exponents) .map_err_context(|| translate!("factor-error-write-error"))?; } @@ -132,57 +132,11 @@ impl Display for NumError<'_> { } } -/// Writing out the prime factors for u64 integers -fn write_result_u64( +/// Writing out the prime factors for integers +fn write_result( w: &mut io::BufWriter, - x: &BigUint, - factorization: BTreeMap, - print_exponents: bool, -) -> io::Result<()> { - write!(w, "{x}:")?; - for (factor, n) in factorization { - if print_exponents { - if n > 1 { - write!(w, " {factor}^{n}")?; - } else { - write!(w, " {factor}")?; - } - } else { - w.write_all(format!(" {factor}").repeat(n).as_bytes())?; - } - } - writeln!(w)?; - w.flush() -} - -/// Writing out the prime factors for u128 integers -fn write_result_u128( - w: &mut io::BufWriter, - x: &u128, - factorization: BTreeMap, - print_exponents: bool, -) -> io::Result<()> { - write!(w, "{x}:")?; - for (factor, n) in factorization { - if print_exponents { - if n > 1 { - write!(w, " {factor}^{n}")?; - } else { - write!(w, " {factor}")?; - } - } else { - w.write_all(format!(" {factor}").repeat(n).as_bytes())?; - } - } - writeln!(w)?; - w.flush() -} - -/// Writing out the prime factors for BigUint integers -fn write_result_big_uint( - w: &mut io::BufWriter, - x: &BigUint, - factorization: BTreeMap, + x: &impl Display, + factorization: BTreeMap, print_exponents: bool, ) -> io::Result<()> { write!(w, "{x}:")?; From ab0a59dab81f54e913dcd5564048a316232b7500 Mon Sep 17 00:00:00 2001 From: "Guillem L. Jara" <4lon3ly0@tutanota.com> Date: Wed, 11 Mar 2026 13:44:42 +0100 Subject: [PATCH 2/3] chore(factor): refactor & optimize parsing Now UTF-8 checks are performed strictly once, the worst-case of BigUints needs not be parsed & allocated every time, and error handling has been tightened. --- src/uu/factor/src/factor.rs | 128 ++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 63 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index aa86330dc91..5d381d9d393 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -10,12 +10,12 @@ use std::fmt::Display; use std::io::BufRead; use std::io::{self, Write, stdin, stdout}; use std::iter::once; -use std::str::FromStr; +use std::num::IntErrorKind; use clap::{Arg, ArgAction, Command}; use memchr::memchr3_iter; use num_bigint::BigUint; -use num_traits::FromPrimitive; +use num_prime::nt_funcs::{factorize64, factorize128, factors}; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, set_exit_code}; use uucore::translate; @@ -33,72 +33,77 @@ const DELIM_SPACE: u8 = b' '; const DELIM_TAB: u8 = b'\t'; const DELIM_NULL: u8 = b'\0'; +#[derive(Debug, PartialEq, Eq)] +enum Number { + U64(u64), + U128(u128), + BigUint(BigUint), +} + fn write_factors_str( num_str: &[u8], w: &mut io::BufWriter, print_exponents: bool, ) -> UResult<()> { - let parsed = parse_num::(num_str); + let parsed = parse_num(num_str); show_if_err!(&parsed); let Ok(x) = parsed else { + // return Ok(). it's non-fatal and we should try the next number. return Ok(()); }; - if x > BigUint::from_u32(1).unwrap() { + match x { // use num_prime's factorize64 algorithm for u64 integers - if x <= BigUint::from_u64(u64::MAX).unwrap() { - let prime_factors = num_prime::nt_funcs::factorize64(x.clone().to_u64_digits()[0]); - write_result(w, &x, prime_factors, print_exponents) - .map_err_context(|| translate!("factor-error-write-error"))?; - } + Number::U64(x) if x > 1 => write_result(w, &x, factorize64(x), print_exponents), + Number::U64(x) => write_result(w, &x, BTreeMap::::new(), print_exponents), // use num_prime's factorize128 algorithm for u128 integers - else if x <= BigUint::from_u128(u128::MAX).unwrap() { - let parsed = parse_num::(num_str); - show_if_err!(&parsed); - let Ok(x) = parsed else { - return Ok(()); - }; - let prime_factors = num_prime::nt_funcs::factorize128(x); - write_result(w, &x, prime_factors, print_exponents) - .map_err_context(|| translate!("factor-error-write-error"))?; - } + Number::U128(x) => write_result(w, &x, factorize128(x), print_exponents), // use num_prime's fallible factorization for anything greater than u128::MAX - else { - let (prime_factors, remaining) = num_prime::nt_funcs::factors(x.clone(), None); - if let Some(_remaining) = remaining { + Number::BigUint(x) => { + let (prime_factors, remaining) = factors(x.clone(), None); + if remaining.is_some() { return Err(USimpleError::new( 1, translate!("factor-error-factorization-incomplete"), )); } write_result(w, &x, prime_factors, print_exponents) - .map_err_context(|| translate!("factor-error-write-error"))?; } - } else { - let empty_primes: BTreeMap = BTreeMap::new(); - write_result(w, &x, empty_primes, print_exponents) - .map_err_context(|| translate!("factor-error-write-error"))?; } - - Ok(()) + .map_err_context(|| translate!("factor-error-write-error")) } -fn parse_num(slice: &[u8]) -> UResult { - str::from_utf8(slice) - .ok() - .and_then(|str| str.trim_matches(DELIM_SPACE as char).parse().ok()) - .ok_or_else(|| { - let num = NumError(slice).to_string(); - let num = if num.len() > slice.len() { - num.quote() // Force quoting if there are invalid characters. - } else { - num.maybe_quote() - }; - USimpleError::new( - 1, - format!("warning: {num}: {}", translate!("factor-error-invalid-int")), - ) - }) +fn parse_num(slice: &[u8]) -> UResult { + let err_invalid = |s: &str, force_quoting| { + let num = if force_quoting { + s.quote() // Force quoting if there are invalid characters. + } else { + s.maybe_quote() + }; + USimpleError::new( + 1, + format!("warning: {num}: {}", translate!("factor-error-invalid-int")), + ) + }; + let num = str::from_utf8(slice).map_err(|_| err_invalid(&NumError(slice).to_string(), true))?; + + match num.parse::() { + Ok(x) => return Ok(Number::U64(x)), + // If overflown, attempt a greater width + Err(e) if matches!(e.kind(), IntErrorKind::PosOverflow) => {} + Err(_) => return Err(err_invalid(num, false)), + } + + match num.parse::() { + Ok(x) => return Ok(Number::U128(x)), + // If overflown, attempt a greater width + Err(e) if matches!(e.kind(), IntErrorKind::PosOverflow) => {} + Err(_) => return Err(err_invalid(num, false)), + } + + num.parse::() + .map(Number::BigUint) + .map_err(|_| err_invalid(num, false)) } /// This is a newtype wrapper over a potentially malformed UTF-8 @@ -168,7 +173,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if let Some(values) = matches.get_many::(options::NUMBER) { for number in values { - write_factors_str(number.as_bytes(), &mut w, print_exponents)?; + write_factors_str(number.trim().as_bytes(), &mut w, print_exponents)?; } } else { let stdin = stdin(); @@ -239,23 +244,24 @@ pub fn uu_app() -> Command { #[cfg(test)] mod tests { - use crate::BigUint; + use crate::Number; use crate::parse_num; #[test] fn test_correct_parsing() { - let zero = parse_num::(b"0").unwrap(); - let u64_max = parse_num::(u64::MAX.to_string().as_bytes()).unwrap(); - let u128_one = parse_num::((u64::MAX as u128 + 1).to_string().as_bytes()).unwrap(); - let u128_max = parse_num::(u128::MAX.to_string().as_bytes()).unwrap(); - let bigint = parse_num::(u128::MAX.to_string().as_bytes()).unwrap() + 1u64; + const U128_MAX_ONE: &str = "340282366920938463463374607431768211456"; + let zero = parse_num(b"0").unwrap(); + let u64_max = parse_num(u64::MAX.to_string().as_bytes()).unwrap(); + let u128_one = parse_num((u64::MAX as u128 + 1).to_string().as_bytes()).unwrap(); + let u128_max = parse_num(u128::MAX.to_string().as_bytes()).unwrap(); + let bigint = parse_num(U128_MAX_ONE.as_bytes()).unwrap(); assert_eq!( ( - 0, - u64::MAX, - (u64::MAX as u128 + 1), - u128::MAX, - BigUint::from(u128::MAX) + 1u64 + Number::U64(0), + Number::U64(u64::MAX), + Number::U128(u64::MAX as u128 + 1), + Number::U128(u128::MAX), + Number::BigUint(U128_MAX_ONE.parse().unwrap()) ), (zero, u64_max, u128_one, u128_max, bigint) ); @@ -264,11 +270,7 @@ mod tests { #[test] #[should_panic] fn test_incorrect_parsing() { - parse_num::(b"abcd").unwrap(); - parse_num::(b"12\x00\xFF\x1D").unwrap(); - parse_num::(b"abcd").unwrap(); - parse_num::(b"12\x00\xFF\x1D").unwrap(); - parse_num::(b"abcd").unwrap(); - parse_num::(b"12\x00\xFF\x1D").unwrap(); + parse_num(b"abcd").unwrap(); + parse_num(b"12\x00\xFF\x1D").unwrap(); } } From fdf46e4f2953ea3a9eecf1f9361ec3aafcb2570a Mon Sep 17 00:00:00 2001 From: "Guillem L. Jara" <4lon3ly0@tutanota.com> Date: Wed, 11 Mar 2026 14:53:00 +0100 Subject: [PATCH 3/3] chore(factor): remove unused dependencies --- Cargo.lock | 1 - src/uu/factor/Cargo.toml | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1bcf9f5083a..9cf89e6cfb3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3519,7 +3519,6 @@ dependencies = [ "memchr", "num-bigint", "num-prime", - "num-traits", "uucore", ] diff --git a/src/uu/factor/Cargo.toml b/src/uu/factor/Cargo.toml index bb5762bfe15..01dc46621d7 100644 --- a/src/uu/factor/Cargo.toml +++ b/src/uu/factor/Cargo.toml @@ -15,13 +15,9 @@ readme.workspace = true [lints] workspace = true -[build-dependencies] -num-traits = { workspace = true } # used in src/numerics.rs, which is included by build.rs - [dependencies] clap = { workspace = true } -num-traits = { workspace = true } -uucore = { workspace = true, features = ["quoting-style"] } +uucore = { workspace = true } memchr = { workspace = true } num-bigint = { workspace = true } num-prime = { workspace = true }