Skip to content

cksum: Add untagged option #4350

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.

Sign up for GitHub

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

Closed
howjmay wants to merge 2 commits into from
Closed

cksum: Add untagged option #4350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
266c70e
cksum: Add untagged option
howjmay Feb 12, 2023
85cb584
Merge branch 'main' into cksum-untagged
sylvestre Apr 28, 2023
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
54 changes: 43 additions & 11 deletions src/uu/cksum/src/cksum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct Options {
algo_name: &'static str,
digest: Box<dyn Digest + 'static>,
output_bits: usize,
reverse: bool,
}

/// Calculate checksum
Expand Down Expand Up @@ -131,36 +132,49 @@ where
File::open(filename).map_err_context(|| filename.to_str().unwrap().to_string())?;
Box::new(file_buf) as Box<dyn Read>
});
let (sum, sz) = digest_read(&mut options.digest, &mut file, options.output_bits)
let (mut sum, sz) = digest_read(&mut options.digest, &mut file, options.output_bits)
.map_err_context(|| "failed to read input".to_string())?;

if options.reverse {
sum = reverse_cksum(&sum);
}
// The BSD checksum output is 5 digit integer
let bsd_width = 5;
match (options.algo_name, not_file) {
(ALGORITHM_OPTIONS_SYSV, true) => println!(
"{} {}",
sum.parse::<u16>().unwrap(),
u16::from_ne_bytes(sum.try_into().unwrap()),
div_ceil(sz, options.output_bits)
),
(ALGORITHM_OPTIONS_SYSV, false) => println!(
"{} {} {}",
sum.parse::<u16>().unwrap(),
u16::from_ne_bytes(sum.try_into().unwrap()),
div_ceil(sz, options.output_bits),
filename.display()
),
(ALGORITHM_OPTIONS_BSD, true) => println!(
"{:0bsd_width$} {:bsd_width$}",
sum.parse::<u16>().unwrap(),
u16::from_ne_bytes(sum.try_into().unwrap()),
div_ceil(sz, options.output_bits)
),
(ALGORITHM_OPTIONS_BSD, false) => println!(
"{:0bsd_width$} {:bsd_width$} {}",
sum.parse::<u16>().unwrap(),
u16::from_ne_bytes(sum.try_into().unwrap()),
div_ceil(sz, options.output_bits),
filename.display()
),
(_, true) => println!("{sum} {sz}"),
(_, false) => println!("{sum} {sz} {}", filename.display()),
(ALGORITHM_OPTIONS_CRC, true) => {
println!("{} {sz}", u32::from_ne_bytes(sum.try_into().unwrap()));
}
(ALGORITHM_OPTIONS_CRC, false) => {
println!(
"{} {sz} {}",
u32::from_ne_bytes(sum.try_into().unwrap()),
filename.display()
);
}
(_, true) => println!("{} {sz}", encode(sum)),
(_, false) => println!("{} {sz} {}", encode(sum), filename.display()),
}
}

Expand All @@ -171,7 +185,7 @@ fn digest_read<T: Read>(
digest: &mut Box<dyn Digest>,
reader: &mut BufReader<T>,
output_bits: usize,
) -> io::Result<(String, usize)> {
) -> io::Result<(Vec<u8>, usize)> {
digest.reset();

// Read bytes from `reader` and write those bytes to `digest`.
Expand All @@ -189,21 +203,30 @@ fn digest_read<T: Read>(
let mut digest_writer = DigestWriter::new(digest, true);
let output_size = std::io::copy(reader, &mut digest_writer)? as usize;
digest_writer.finalize();

if digest.output_bits() > 0 {
Ok((digest.result_str(), output_size))
Ok((digest.result_bytes(), output_size))
} else {
// Assume it's SHAKE. result_str() doesn't work with shake (as of 8/30/2016)
let mut bytes = Vec::new();
bytes.resize((output_bits + 7) / 8, 0);
digest.hash_finalize(&mut bytes);
Ok((encode(bytes), output_size))
Ok((bytes, output_size))
}
}

fn reverse_cksum(checksum: &[u8]) -> Vec<u8> {
let sz = checksum.len();
let mut ret: Vec<u8> = std::iter::repeat(0).take(sz).collect::<Vec<u8>>();
for i in 0..sz {
ret[i] = checksum[sz - i - 1].reverse_bits();
}
ret
}
Comment on lines +217 to 224
Copy link
Copy Markdown
Contributor

@anastygnome anastygnome Mar 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'd write this without a for loop.

fn reverse_cksum(checksum: &[u8]) -> Vec<u8> {
    checksum.iter().rev().map(|&b| b.reverse_bits()).collect()
}

But anyway, ret should be declared this way :

     let mut ret: Vec<u8> = Vec::with_capacity(sz);

to avoid a useless initialisation

Copy link
Copy Markdown
Contributor

@anastygnome anastygnome Mar 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tertsdiepraam I forgot whether we try to avoid for loops for uutils, is this a rule here?

Copy link
Copy Markdown
Collaborator

@tertsdiepraam tertsdiepraam Mar 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use whatever you think is most clear in each situation!

Copy link
Copy Markdown
Contributor

@sylvestre sylvestre Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@howjmay what do you think here?


mod options {
pub static FILE: &str = "file";
pub static ALGORITHM: &str = "algorithm";
pub static UNTAGGED: &str = "untagged";
}

#[uucore::main]
Expand All @@ -217,11 +240,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
None => ALGORITHM_OPTIONS_CRC,
};

let untagged: bool = matches.get_flag(options::UNTAGGED);

let (name, algo, bits) = detect_algo(algo_name);
let opts = Options {
algo_name: name,
digest: algo,
output_bits: bits,
reverse: untagged,
};

match matches.get_many::<String>(options::FILE) {
Expand Down Expand Up @@ -264,5 +290,11 @@ pub fn uu_app() -> Command {
ALGORITHM_OPTIONS_SM3,
]),
)
.arg(
Arg::new(options::UNTAGGED)
.long(options::UNTAGGED)
.help("create a reversed style checksum, without digest type")
.action(clap::ArgAction::SetTrue),
)
.after_help(AFTER_HELP)
}
25 changes: 24 additions & 1 deletion src/uucore/src/lib/features/sum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ pub trait Digest {
self.hash_finalize(&mut buf);
encode(buf)
}
fn result_bytes(&mut self) -> Vec<u8> {
let mut buf: Vec<u8> = vec![0; self.output_bytes()];
self.hash_finalize(&mut buf);
buf
}
}

/// first element of the tuple is the blake2b state
Expand Down Expand Up @@ -200,12 +205,18 @@ impl Digest for CRC {
format!("{}", self.state)
}

fn result_bytes(&mut self) -> Vec<u8> {
let mut out: Vec<u8> = vec![0; 4];
self.hash_finalize(&mut out);
out.to_vec()
}

fn reset(&mut self) {
*self = Self::new();
}

fn output_bits(&self) -> usize {
256
32
}
}

Expand Down Expand Up @@ -241,6 +252,12 @@ impl Digest for BSD {
format!("{}", self.state)
}

fn result_bytes(&mut self) -> Vec<u8> {
let mut out: Vec<u8> = vec![0; 2];
self.hash_finalize(&mut out);
out.to_vec()
}

fn reset(&mut self) {
*self = Self::new();
}
Expand Down Expand Up @@ -276,6 +293,12 @@ impl Digest for SYSV {
format!("{}", self.state)
}

fn result_bytes(&mut self) -> Vec<u8> {
let mut out: Vec<u8> = vec![0; 2];
self.hash_finalize(&mut out);
out.to_vec()
}

fn reset(&mut self) {
*self = Self::new();
}
Expand Down
9 changes: 9 additions & 0 deletions tests/by-util/test_cksum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ fn test_single_file() {
.stdout_is_fixture("single_file.expected");
}

#[test]
fn test_single_file_untagged() {
new_ucmd!()
.arg("--untagged")
.arg("lorem_ipsum.txt")
.succeeds()
.stdout_is_fixture("single_file_untagged.expected");
}

#[test]
fn test_multiple_files() {
new_ucmd!()
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/cksum/single_file_untagged.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
369766760 772 lorem_ipsum.txt