Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

factor: add -h/--exponents option #4710

Merged
merged 2 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ unindent = "0.2"
uucore = { workspace=true, features=["entries", "process", "signals"] }
walkdir = { workspace=true }
is-terminal = { workspace=true }
hex-literal = "0.4.0"
hex-literal = "0.4.1"
rstest = "0.17.0"

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion src/uu/factor/factor.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# factor

```
factor [NUMBER]...
factor [OPTION]... [NUMBER]...
```

Print the prime factors of the given NUMBER(s).
Expand Down
36 changes: 33 additions & 3 deletions src/uu/factor/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,30 @@ const ABOUT: &str = help_about!("factor.md");
const USAGE: &str = help_usage!("factor.md");

mod options {
pub static EXPONENTS: &str = "exponents";
pub static HELP: &str = "help";
pub static NUMBER: &str = "NUMBER";
}

fn print_factors_str(
num_str: &str,
w: &mut io::BufWriter<impl io::Write>,
factors_buffer: &mut String,
print_exponents: bool,
) -> Result<(), Box<dyn Error>> {
num_str
.trim()
.parse::<u64>()
.map_err(|e| e.into())
.and_then(|x| {
factors_buffer.clear();
writeln!(factors_buffer, "{}:{}", x, factor(x))?;
// If print_exponents is true, use the alternate format specifier {:#} from fmt to print the factors
// of x in the form of p^e.
if print_exponents {
writeln!(factors_buffer, "{}:{:#}", x, factor(x))?;
} else {
writeln!(factors_buffer, "{}:{}", x, factor(x))?;
}
w.write_all(factors_buffer.as_bytes())?;
Ok(())
})
Expand All @@ -50,14 +59,19 @@ fn print_factors_str(
#[uucore::main]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let matches = uu_app().try_get_matches_from(args)?;

// If matches find --exponents flag than variable print_exponents is true and p^e output format will be used.
let print_exponents = matches.get_flag(options::EXPONENTS);

let stdout = stdout();
// We use a smaller buffer here to pass a gnu test. 4KiB appears to be the default pipe size for bash.
let mut w = io::BufWriter::with_capacity(4 * 1024, stdout.lock());
let mut factors_buffer = String::new();

if let Some(values) = matches.get_many::<String>(options::NUMBER) {
for number in values {
if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer, print_exponents)
{
show_warning!("{}: {}", number.maybe_quote(), e);
}
}
Expand All @@ -66,7 +80,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let lines = stdin.lock().lines();
for line in lines {
for number in line.unwrap().split_whitespace() {
if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
if let Err(e) =
print_factors_str(number, &mut w, &mut factors_buffer, print_exponents)
{
show_warning!("{}: {}", number.maybe_quote(), e);
}
}
Expand All @@ -86,5 +102,19 @@ pub fn uu_app() -> Command {
.about(ABOUT)
.override_usage(format_usage(USAGE))
.infer_long_args(true)
.disable_help_flag(true)
.arg(Arg::new(options::NUMBER).action(ArgAction::Append))
.arg(
Arg::new(options::EXPONENTS)
.short('h')
.long(options::EXPONENTS)
.help("Print factors in the form p^e")
.action(ArgAction::SetTrue),
)
.arg(
Arg::new(options::HELP)
.long(options::HELP)
.help("Print help information.")
.action(ArgAction::Help),
)
}
9 changes: 7 additions & 2 deletions src/uu/factor/src/factor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,14 @@ impl fmt::Display for Factors {
let v = &mut (self.0).borrow_mut().0;
v.sort_unstable();

let include_exponents = f.alternate();
for (p, exp) in v.iter() {
for _ in 0..*exp {
write!(f, " {p}")?;
if include_exponents && *exp > 1 {
write!(f, " {p}^{exp}")?;
} else {
for _ in 0..*exp {
write!(f, " {p}")?;
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion tests/benches/factor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@ description = "Benchmarks for the uu_factor integer factorization tool"
homepage = "https:/uutils/coreutils"
edition = "2021"

[workspace]

[dependencies]
uu_factor = { path = "../../../src/uu/factor" }

[dev-dependencies]
array-init = "2.0.0"
criterion = "0.3"
rand = "0.8"
rand_chacha = "0.2.2"
rand_chacha = "0.3.1"

[[bench]]
name = "gcd"
Expand Down
4 changes: 2 additions & 2 deletions tests/benches/factor/benches/gcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fn gcd(c: &mut Criterion) {
// Deterministic RNG; use an explicitly-named RNG to guarantee stability
use rand::{RngCore, SeedableRng};
use rand_chacha::ChaCha8Rng;
const SEED: u64 = 0xa_b4d_1dea_dead_cafe;
const SEED: u64 = 0xab4d_1dea_dead_cafe;
let mut rng = ChaCha8Rng::seed_from_u64(SEED);

std::iter::repeat_with(move || (rng.next_u64(), rng.next_u64()))
Expand All @@ -22,7 +22,7 @@ fn gcd(c: &mut Criterion) {
},
);
}
group.finish()
group.finish();
}

criterion_group!(benches, gcd);
Expand Down
33 changes: 13 additions & 20 deletions tests/benches/factor/benches/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ fn table(c: &mut Criterion) {
check_personality();

const INPUT_SIZE: usize = 128;
assert!(
INPUT_SIZE % CHUNK_SIZE == 0,
"INPUT_SIZE ({}) is not divisible by CHUNK_SIZE ({})",
INPUT_SIZE,
CHUNK_SIZE
);

let inputs = {
// Deterministic RNG; use an explicitly-named RNG to guarantee stability
use rand::{RngCore, SeedableRng};
Expand All @@ -29,42 +24,40 @@ fn table(c: &mut Criterion) {
let a_str = format!("{:?}", a);
group.bench_with_input(BenchmarkId::new("factor_chunk", &a_str), &a, |b, &a| {
b.iter(|| {
let mut n_s = a.clone();
let mut n_s = a;
let mut f_s: [_; INPUT_SIZE] = array_init(|_| Factors::one());
for (n_s, f_s) in n_s.chunks_mut(CHUNK_SIZE).zip(f_s.chunks_mut(CHUNK_SIZE)) {
factor_chunk(n_s.try_into().unwrap(), f_s.try_into().unwrap())
factor_chunk(n_s.try_into().unwrap(), f_s.try_into().unwrap());
}
})
});
});
group.bench_with_input(BenchmarkId::new("factor", &a_str), &a, |b, &a| {
b.iter(|| {
let mut n_s = a.clone();
let mut n_s = a;
let mut f_s: [_; INPUT_SIZE] = array_init(|_| Factors::one());
for (n, f) in n_s.iter_mut().zip(f_s.iter_mut()) {
factor(n, f)
factor(n, f);
}
})
});
});
}
group.finish()
group.finish();
}

#[cfg(target_os = "linux")]
fn check_personality() {
use std::fs;
const ADDR_NO_RANDOMIZE: u64 = 0x0040000;
const PERSONALITY_PATH: &'static str = "/proc/self/personality";
const PERSONALITY_PATH: &str = "/proc/self/personality";

let p_string = fs::read_to_string(PERSONALITY_PATH)
.expect(&format!("Couldn't read '{}'", PERSONALITY_PATH))
.strip_suffix("\n")
.unwrap_or_else(|_| panic!("Couldn't read '{}'", PERSONALITY_PATH))
.strip_suffix('\n')
.unwrap()
.to_owned();

let personality = u64::from_str_radix(&p_string, 16).expect(&format!(
"Expected a hex value for personality, got '{:?}'",
p_string
));
let personality = u64::from_str_radix(&p_string, 16)
.unwrap_or_else(|_| panic!("Expected a hex value for personality, got '{:?}'", p_string));
if personality & ADDR_NO_RANDOMIZE == 0 {
eprintln!(
"WARNING: Benchmarking with ASLR enabled (personality is {:x}), results might not be reproducible.",
Expand Down
75 changes: 67 additions & 8 deletions tests/by-util/test_factor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,16 @@

// spell-checker:ignore (methods) hexdigest

use crate::common::util::{AtPath, TestScenario};
use crate::common::util::TestScenario;

use std::time::{Duration, SystemTime};

#[path = "../../src/uu/factor/sieve.rs"]
mod sieve;

extern crate conv;
extern crate rand;

use self::rand::distributions::{Distribution, Uniform};
use self::rand::{rngs::SmallRng, Rng, SeedableRng};
use self::sieve::Sieve;
use rand::distributions::{Distribution, Uniform};
use rand::{rngs::SmallRng, Rng, SeedableRng};

const NUM_PRIMES: usize = 10000;
const NUM_TESTS: usize = 100;
Expand All @@ -30,9 +27,16 @@ fn test_invalid_arg() {
new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
}

#[test]
fn test_valid_arg_exponents() {
new_ucmd!().arg("-h").succeeds().code_is(0);
new_ucmd!().arg("--exponents").succeeds().code_is(0);
}

#[test]
#[cfg(feature = "sort")]
fn test_parallel() {
use crate::common::util::AtPath;
use hex_literal::hex;
use sha1::{Digest, Sha1};
use std::{fs::OpenOptions, time::Duration};
Expand Down Expand Up @@ -80,7 +84,6 @@ fn test_parallel() {

#[test]
fn test_first_1000_integers() {
extern crate sha1;
use hex_literal::hex;
use sha1::{Digest, Sha1};

Expand All @@ -103,6 +106,33 @@ fn test_first_1000_integers() {
);
}

#[test]
fn test_first_1000_integers_with_exponents() {
use hex_literal::hex;
use sha1::{Digest, Sha1};

let n_integers = 1000;
let mut input_string = String::new();
for i in 0..=n_integers {
input_string.push_str(&(format!("{i} "))[..]);
}

println!("STDIN='{input_string}'");
let result = new_ucmd!()
.arg("-h")
.pipe_in(input_string.as_bytes())
.succeeds();

// Using factor from GNU Coreutils 9.2
// `seq 0 1000 | factor -h | sha1sum` => "45f5f758a9319870770bd1fec2de23d54331944d"
let mut hasher = Sha1::new();
hasher.update(result.stdout());
let hash_check = hasher.finalize();
assert_eq!(
hash_check[..],
hex!("45f5f758a9319870770bd1fec2de23d54331944d")
);
}
#[test]
fn test_cli_args() {
// Make sure that factor works with CLI arguments as well.
Expand All @@ -117,7 +147,7 @@ fn test_cli_args() {

#[test]
fn test_random() {
use conv::prelude::*;
use conv::prelude::ValueFrom;

let log_num_primes = f64::value_from(NUM_PRIMES).unwrap().log2().ceil();
let primes = Sieve::primes().take(NUM_PRIMES).collect::<Vec<u64>>();
Expand Down Expand Up @@ -282,6 +312,35 @@ fn run(input_string: &[u8], output_string: &[u8]) {
.stdout_is(String::from_utf8(output_string.to_owned()).unwrap());
}

#[test]
fn test_primes_with_exponents() {
let mut input_string = String::new();
let mut output_string = String::new();
for primes in PRIMES_BY_BITS.iter() {
for &prime in *primes {
input_string.push_str(&(format!("{prime} "))[..]);
output_string.push_str(&(format!("{prime}: {prime}\n"))[..]);
}
}

println!(
"STDIN='{}'",
String::from_utf8_lossy(input_string.as_bytes())
);
println!(
"STDOUT(expected)='{}'",
String::from_utf8_lossy(output_string.as_bytes())
);

// run factor with --exponents
new_ucmd!()
.timeout(Duration::from_secs(240))
.arg("--exponents")
.pipe_in(input_string)
.run()
.stdout_is(String::from_utf8(output_string.as_bytes().to_owned()).unwrap());
}

const PRIMES_BY_BITS: &[&[u64]] = &[
PRIMES14, PRIMES15, PRIMES16, PRIMES17, PRIMES18, PRIMES19, PRIMES20, PRIMES21, PRIMES22,
PRIMES23, PRIMES24, PRIMES25, PRIMES26, PRIMES27, PRIMES28, PRIMES29, PRIMES30, PRIMES31,
Expand Down