From 2b3594a5f536496035215bf2bf7fee46d06c9452 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 19:13:33 -0700 Subject: [PATCH 01/17] core: add octal and hex size parse support --- src/uucore/src/lib/parser/parse_size.rs | 126 ++++++++++++++++++++---- 1 file changed, 106 insertions(+), 20 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 60209d849..70b94fbcc 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -25,6 +25,12 @@ pub struct Parser<'parser> { pub default_unit: Option<&'parser str>, } +enum NumberSystem { + Decimal, + Octal, + Hexadecimal, +} + impl<'parser> Parser<'parser> { pub fn with_allow_list(&mut self, allow_list: &'parser [&str]) -> &mut Self { self.allow_list = Some(allow_list); @@ -62,32 +68,42 @@ impl<'parser> Parser<'parser> { /// assert_eq!(Ok(123), parse_size("123")); /// assert_eq!(Ok(9 * 1000), parse_size("9kB")); // kB is 1000 /// assert_eq!(Ok(2 * 1024), parse_size("2K")); // K is 1024 + /// assert_eq!(Ok(44251 * 1024), parse_size("0xACDBK")); /// ``` pub fn parse(&self, size: &str) -> Result { if size.is_empty() { return Err(ParseSizeError::parse_failure(size)); } - // Get the numeric part of the size argument. For example, if the - // argument is "123K", then the numeric part is "123". - let numeric_string: String = size.chars().take_while(|c| c.is_ascii_digit()).collect(); - let number: u64 = if numeric_string.is_empty() { - 1 - } else { - match numeric_string.parse() { - Ok(n) => n, - Err(_) => return Err(ParseSizeError::parse_failure(size)), + + let number_system: NumberSystem = self.determine_number_system(size); + + // Split the size argument into numeric and unit parts + // For example, if the argument is "123K", the numeric part is "123", and + // the unit is "K" + let (numeric_string, unit) = match number_system { + NumberSystem::Hexadecimal => { + let numeric_string: String = size + .chars() + .take(2) + .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) + .collect(); + let unit: String = size.chars().skip(numeric_string.len()).collect(); + + (numeric_string, unit) + } + _ => { + let mut unit: String = size + .chars() + .rev() + .take_while(|c| c.is_alphabetic()) + .collect(); + unit = unit.chars().rev().collect(); + let numeric_string = size.chars().take(size.len() - unit.len()).collect(); + + (numeric_string, unit) } }; - - // Get the alphabetic units part of the size argument and compute - // the factor it represents. For example, if the argument is "123K", - // then the unit part is "K" and the factor is 1024. This may be the - // empty string, in which case, the factor is 1. - // - // The lowercase "b" (used by `od`, `head`, `tail`, etc.) means - // "block" and the Posix block size is 512. The uppercase "B" - // means "byte". - let mut unit: &str = &size[numeric_string.len()..]; + let mut unit: &str = unit.as_str(); if let Some(default_unit) = self.default_unit { // Check if `unit` is empty then assigns `default_unit` to `unit` @@ -115,6 +131,12 @@ impl<'parser> Parser<'parser> { } } + // Compute the factor the unit represents. + // empty string means the factor is 1. + // + // The lowercase "b" (used by `od`, `head`, `tail`, etc.) means + // "block" and the Posix block size is 512. The uppercase "B" + // means "byte". let (base, exponent): (u128, u32) = match unit { "" => (1, 0), "B" if self.capital_b_bytes => (1, 0), @@ -142,10 +164,60 @@ impl<'parser> Parser<'parser> { Ok(n) => n, Err(_) => return Err(ParseSizeError::size_too_big(size)), }; + + // parse string into u64 + let number: u64 = match number_system { + NumberSystem::Decimal => { + if numeric_string.is_empty() { + 1 + } else { + match numeric_string.parse() { + Ok(n) => n, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + } + NumberSystem::Octal => { + let trimmed_string = numeric_string.trim_start_matches("0"); + match u64::from_str_radix(trimmed_string, 8) { + Ok(res) => res, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + NumberSystem::Hexadecimal => { + let trimmed_string = numeric_string.trim_start_matches("0x"); + match u64::from_str_radix(trimmed_string, 16) { + Ok(res) => res, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + }; + number .checked_mul(factor) .ok_or_else(|| ParseSizeError::size_too_big(size)) } + + fn determine_number_system(&self, size: &str) -> NumberSystem { + if size.len() <= 1 { + return NumberSystem::Decimal; + } + + if size.starts_with("0x") { + return NumberSystem::Hexadecimal; + } + + let num_digits: usize = size + .chars() + .take_while(|c| c.is_ascii_digit()) + .collect::() + .len(); + if size.starts_with("0") && num_digits > 1 { + return NumberSystem::Octal; + } + + NumberSystem::Decimal + } } /// Parse a size string into a number of bytes. @@ -336,7 +408,7 @@ mod tests { #[test] fn invalid_suffix() { - let test_strings = ["328hdsf3290", "5mib", "1e2", "1H", "1.2"]; + let test_strings = ["5mib", "1eb", "1H"]; for &test_string in &test_strings { assert_eq!( parse_size(test_string).unwrap_err(), @@ -450,4 +522,18 @@ mod tests { assert!(parser.parse("1B").is_err()); assert!(parser.parse("B").is_err()); } + + #[test] + fn parse_octal_size() { + assert_eq!(Ok(63), parse_size("077")); + assert_eq!(Ok(528), parse_size("01020")); + assert_eq!(Ok(668 * 1024), parse_size("01234K")); + } + + #[test] + fn parse_hex_size() { + assert_eq!(Ok(10), parse_size("0xA")); + assert_eq!(Ok(94722), parse_size("0x17202")); + assert_eq!(Ok(44251 * 1024), parse_size("0xACDBK")); + } } From f8a46196ef42bc404040173ad9bc93ae25d15ac4 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 20:10:16 -0700 Subject: [PATCH 02/17] shred: add support for hex and octal size --- src/uu/shred/src/shred.rs | 42 ++++++++++--------------------------- tests/by-util/test_shred.rs | 11 ++++++++++ 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 89f857b19..1ceffe995 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -19,6 +19,7 @@ use std::os::unix::prelude::PermissionsExt; use std::path::{Path, PathBuf}; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; +use uucore::parse_size::parse_size; use uucore::{format_usage, help_about, help_section, help_usage, show, show_error, show_if_err}; const ABOUT: &str = help_about!("shred.md"); @@ -318,38 +319,17 @@ pub fn uu_app() -> Command { ) } -// TODO: Add support for all postfixes here up to and including EiB -// http://www.gnu.org/software/coreutils/manual/coreutils.html#Block-size fn get_size(size_str_opt: Option) -> Option { - size_str_opt.as_ref()?; - - let mut size_str = size_str_opt.as_ref().unwrap().clone(); - // Immutably look at last character of size string - let unit = match size_str.chars().last().unwrap() { - 'K' => { - size_str.pop(); - 1024u64 - } - 'M' => { - size_str.pop(); - (1024 * 1024) as u64 - } - 'G' => { - size_str.pop(); - (1024 * 1024 * 1024) as u64 - } - _ => 1u64, - }; - - let coefficient = match size_str.parse::() { - Ok(u) => u, - Err(_) => { - show_error!("{}: Invalid file size", size_str_opt.unwrap().maybe_quote()); - std::process::exit(1); - } - }; - - Some(coefficient * unit) + match size_str_opt { + Some(size) => match parse_size(size.as_str()) { + Ok(res) => Some(res), + Err(_) => { + show_error!("invalid file size: {}", size.quote()); + std::process::exit(1) + } + }, + None => None, + } } fn pass_name(pass_type: &PassType) -> String { diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 58db09cbd..a34345aee 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -51,3 +51,14 @@ fn test_shred_force() { // file_a was deleted. assert!(!at.file_exists(file)); } + +#[test] +fn test_hex() { + let (at, mut ucmd) = at_and_ucmd!(); + + let file = "test_hex"; + + at.touch(file); + + ucmd.arg("--size=0x10").arg(file).succeeds(); +} From 6233ad6dd7e9a0fa4552cec788265a75dac5fecf Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 20:59:21 -0700 Subject: [PATCH 03/17] core: fix clippy warning for size parser --- src/uucore/src/lib/parser/parse_size.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 70b94fbcc..be642ef8b 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -178,7 +178,7 @@ impl<'parser> Parser<'parser> { } } NumberSystem::Octal => { - let trimmed_string = numeric_string.trim_start_matches("0"); + let trimmed_string = numeric_string.trim_start_matches('0'); match u64::from_str_radix(trimmed_string, 8) { Ok(res) => res, Err(_) => return Err(ParseSizeError::parse_failure(size)), @@ -212,7 +212,7 @@ impl<'parser> Parser<'parser> { .take_while(|c| c.is_ascii_digit()) .collect::() .len(); - if size.starts_with("0") && num_digits > 1 { + if size.starts_with('0') && num_digits > 1 { return NumberSystem::Octal; } From af528ba84e91a36005e2d6f4c82e92107a4937b5 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:26:17 -0700 Subject: [PATCH 04/17] core: refactor num and unit split --- src/uucore/src/lib/parser/parse_size.rs | 31 +++++++------------------ 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index be642ef8b..716cf8d79 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -80,30 +80,15 @@ impl<'parser> Parser<'parser> { // Split the size argument into numeric and unit parts // For example, if the argument is "123K", the numeric part is "123", and // the unit is "K" - let (numeric_string, unit) = match number_system { - NumberSystem::Hexadecimal => { - let numeric_string: String = size - .chars() - .take(2) - .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) - .collect(); - let unit: String = size.chars().skip(numeric_string.len()).collect(); - - (numeric_string, unit) - } - _ => { - let mut unit: String = size - .chars() - .rev() - .take_while(|c| c.is_alphabetic()) - .collect(); - unit = unit.chars().rev().collect(); - let numeric_string = size.chars().take(size.len() - unit.len()).collect(); - - (numeric_string, unit) - } + let numeric_string: String = match number_system { + NumberSystem::Hexadecimal => size + .chars() + .take(2) + .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) + .collect(), + _ => size.chars().take_while(|c| c.is_ascii_digit()).collect(), }; - let mut unit: &str = unit.as_str(); + let mut unit: &str = &size[numeric_string.len()..]; if let Some(default_unit) = self.default_unit { // Check if `unit` is empty then assigns `default_unit` to `unit` From aed8a5759a42ffe6b7f94dc8d6750e18c623df65 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:27:09 -0700 Subject: [PATCH 05/17] core: size parser treat 000 as decimal --- src/uucore/src/lib/parser/parse_size.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 716cf8d79..af4cf990d 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -197,7 +197,8 @@ impl<'parser> Parser<'parser> { .take_while(|c| c.is_ascii_digit()) .collect::() .len(); - if size.starts_with('0') && num_digits > 1 { + let all_zeros = size.chars().all(|c| c == '0'); + if size.starts_with('0') && num_digits > 1 && !all_zeros { return NumberSystem::Octal; } From 81871d54a2bc3b765d5c3bee219b3d7725e2d362 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:33:14 -0700 Subject: [PATCH 06/17] core: refactor parse_number in size parser --- src/uucore/src/lib/parser/parse_size.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index af4cf990d..c6a02dfae 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -156,25 +156,16 @@ impl<'parser> Parser<'parser> { if numeric_string.is_empty() { 1 } else { - match numeric_string.parse() { - Ok(n) => n, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(&numeric_string, 10, size)? } } NumberSystem::Octal => { let trimmed_string = numeric_string.trim_start_matches('0'); - match u64::from_str_radix(trimmed_string, 8) { - Ok(res) => res, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(trimmed_string, 8, size)? } NumberSystem::Hexadecimal => { let trimmed_string = numeric_string.trim_start_matches("0x"); - match u64::from_str_radix(trimmed_string, 16) { - Ok(res) => res, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(trimmed_string, 16, size)? } }; @@ -204,6 +195,16 @@ impl<'parser> Parser<'parser> { NumberSystem::Decimal } + + fn parse_number( + &self, + numeric_string: &str, + radix: u32, + original_size: &str, + ) -> Result { + u64::from_str_radix(numeric_string, radix) + .map_err(|_| ParseSizeError::ParseFailure(original_size.to_string())) + } } /// Parse a size string into a number of bytes. From 6278c6f2d6bcd923c2ce294b13b822853a529030 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 19:13:33 -0700 Subject: [PATCH 07/17] core: add octal and hex size parse support --- src/uucore/src/lib/parser/parse_size.rs | 126 ++++++++++++++++++++---- 1 file changed, 106 insertions(+), 20 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 60209d849..70b94fbcc 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -25,6 +25,12 @@ pub struct Parser<'parser> { pub default_unit: Option<&'parser str>, } +enum NumberSystem { + Decimal, + Octal, + Hexadecimal, +} + impl<'parser> Parser<'parser> { pub fn with_allow_list(&mut self, allow_list: &'parser [&str]) -> &mut Self { self.allow_list = Some(allow_list); @@ -62,32 +68,42 @@ impl<'parser> Parser<'parser> { /// assert_eq!(Ok(123), parse_size("123")); /// assert_eq!(Ok(9 * 1000), parse_size("9kB")); // kB is 1000 /// assert_eq!(Ok(2 * 1024), parse_size("2K")); // K is 1024 + /// assert_eq!(Ok(44251 * 1024), parse_size("0xACDBK")); /// ``` pub fn parse(&self, size: &str) -> Result { if size.is_empty() { return Err(ParseSizeError::parse_failure(size)); } - // Get the numeric part of the size argument. For example, if the - // argument is "123K", then the numeric part is "123". - let numeric_string: String = size.chars().take_while(|c| c.is_ascii_digit()).collect(); - let number: u64 = if numeric_string.is_empty() { - 1 - } else { - match numeric_string.parse() { - Ok(n) => n, - Err(_) => return Err(ParseSizeError::parse_failure(size)), + + let number_system: NumberSystem = self.determine_number_system(size); + + // Split the size argument into numeric and unit parts + // For example, if the argument is "123K", the numeric part is "123", and + // the unit is "K" + let (numeric_string, unit) = match number_system { + NumberSystem::Hexadecimal => { + let numeric_string: String = size + .chars() + .take(2) + .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) + .collect(); + let unit: String = size.chars().skip(numeric_string.len()).collect(); + + (numeric_string, unit) + } + _ => { + let mut unit: String = size + .chars() + .rev() + .take_while(|c| c.is_alphabetic()) + .collect(); + unit = unit.chars().rev().collect(); + let numeric_string = size.chars().take(size.len() - unit.len()).collect(); + + (numeric_string, unit) } }; - - // Get the alphabetic units part of the size argument and compute - // the factor it represents. For example, if the argument is "123K", - // then the unit part is "K" and the factor is 1024. This may be the - // empty string, in which case, the factor is 1. - // - // The lowercase "b" (used by `od`, `head`, `tail`, etc.) means - // "block" and the Posix block size is 512. The uppercase "B" - // means "byte". - let mut unit: &str = &size[numeric_string.len()..]; + let mut unit: &str = unit.as_str(); if let Some(default_unit) = self.default_unit { // Check if `unit` is empty then assigns `default_unit` to `unit` @@ -115,6 +131,12 @@ impl<'parser> Parser<'parser> { } } + // Compute the factor the unit represents. + // empty string means the factor is 1. + // + // The lowercase "b" (used by `od`, `head`, `tail`, etc.) means + // "block" and the Posix block size is 512. The uppercase "B" + // means "byte". let (base, exponent): (u128, u32) = match unit { "" => (1, 0), "B" if self.capital_b_bytes => (1, 0), @@ -142,10 +164,60 @@ impl<'parser> Parser<'parser> { Ok(n) => n, Err(_) => return Err(ParseSizeError::size_too_big(size)), }; + + // parse string into u64 + let number: u64 = match number_system { + NumberSystem::Decimal => { + if numeric_string.is_empty() { + 1 + } else { + match numeric_string.parse() { + Ok(n) => n, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + } + NumberSystem::Octal => { + let trimmed_string = numeric_string.trim_start_matches("0"); + match u64::from_str_radix(trimmed_string, 8) { + Ok(res) => res, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + NumberSystem::Hexadecimal => { + let trimmed_string = numeric_string.trim_start_matches("0x"); + match u64::from_str_radix(trimmed_string, 16) { + Ok(res) => res, + Err(_) => return Err(ParseSizeError::parse_failure(size)), + } + } + }; + number .checked_mul(factor) .ok_or_else(|| ParseSizeError::size_too_big(size)) } + + fn determine_number_system(&self, size: &str) -> NumberSystem { + if size.len() <= 1 { + return NumberSystem::Decimal; + } + + if size.starts_with("0x") { + return NumberSystem::Hexadecimal; + } + + let num_digits: usize = size + .chars() + .take_while(|c| c.is_ascii_digit()) + .collect::() + .len(); + if size.starts_with("0") && num_digits > 1 { + return NumberSystem::Octal; + } + + NumberSystem::Decimal + } } /// Parse a size string into a number of bytes. @@ -336,7 +408,7 @@ mod tests { #[test] fn invalid_suffix() { - let test_strings = ["328hdsf3290", "5mib", "1e2", "1H", "1.2"]; + let test_strings = ["5mib", "1eb", "1H"]; for &test_string in &test_strings { assert_eq!( parse_size(test_string).unwrap_err(), @@ -450,4 +522,18 @@ mod tests { assert!(parser.parse("1B").is_err()); assert!(parser.parse("B").is_err()); } + + #[test] + fn parse_octal_size() { + assert_eq!(Ok(63), parse_size("077")); + assert_eq!(Ok(528), parse_size("01020")); + assert_eq!(Ok(668 * 1024), parse_size("01234K")); + } + + #[test] + fn parse_hex_size() { + assert_eq!(Ok(10), parse_size("0xA")); + assert_eq!(Ok(94722), parse_size("0x17202")); + assert_eq!(Ok(44251 * 1024), parse_size("0xACDBK")); + } } From 3ca003846dc2d272d56a27043cf73c9b5dc6b427 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 20:10:16 -0700 Subject: [PATCH 08/17] shred: add support for hex and octal size --- src/uu/shred/src/shred.rs | 42 ++++++++++--------------------------- tests/by-util/test_shred.rs | 11 ++++++++++ 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 89f857b19..1ceffe995 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -19,6 +19,7 @@ use std::os::unix::prelude::PermissionsExt; use std::path::{Path, PathBuf}; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; +use uucore::parse_size::parse_size; use uucore::{format_usage, help_about, help_section, help_usage, show, show_error, show_if_err}; const ABOUT: &str = help_about!("shred.md"); @@ -318,38 +319,17 @@ pub fn uu_app() -> Command { ) } -// TODO: Add support for all postfixes here up to and including EiB -// http://www.gnu.org/software/coreutils/manual/coreutils.html#Block-size fn get_size(size_str_opt: Option) -> Option { - size_str_opt.as_ref()?; - - let mut size_str = size_str_opt.as_ref().unwrap().clone(); - // Immutably look at last character of size string - let unit = match size_str.chars().last().unwrap() { - 'K' => { - size_str.pop(); - 1024u64 - } - 'M' => { - size_str.pop(); - (1024 * 1024) as u64 - } - 'G' => { - size_str.pop(); - (1024 * 1024 * 1024) as u64 - } - _ => 1u64, - }; - - let coefficient = match size_str.parse::() { - Ok(u) => u, - Err(_) => { - show_error!("{}: Invalid file size", size_str_opt.unwrap().maybe_quote()); - std::process::exit(1); - } - }; - - Some(coefficient * unit) + match size_str_opt { + Some(size) => match parse_size(size.as_str()) { + Ok(res) => Some(res), + Err(_) => { + show_error!("invalid file size: {}", size.quote()); + std::process::exit(1) + } + }, + None => None, + } } fn pass_name(pass_type: &PassType) -> String { diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index 58db09cbd..a34345aee 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -51,3 +51,14 @@ fn test_shred_force() { // file_a was deleted. assert!(!at.file_exists(file)); } + +#[test] +fn test_hex() { + let (at, mut ucmd) = at_and_ucmd!(); + + let file = "test_hex"; + + at.touch(file); + + ucmd.arg("--size=0x10").arg(file).succeeds(); +} From 6cadffc8f1879ba1174d30d10cadc9ac273498a3 Mon Sep 17 00:00:00 2001 From: John Shin Date: Mon, 29 May 2023 20:59:21 -0700 Subject: [PATCH 09/17] core: fix clippy warning for size parser --- src/uucore/src/lib/parser/parse_size.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 70b94fbcc..be642ef8b 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -178,7 +178,7 @@ impl<'parser> Parser<'parser> { } } NumberSystem::Octal => { - let trimmed_string = numeric_string.trim_start_matches("0"); + let trimmed_string = numeric_string.trim_start_matches('0'); match u64::from_str_radix(trimmed_string, 8) { Ok(res) => res, Err(_) => return Err(ParseSizeError::parse_failure(size)), @@ -212,7 +212,7 @@ impl<'parser> Parser<'parser> { .take_while(|c| c.is_ascii_digit()) .collect::() .len(); - if size.starts_with("0") && num_digits > 1 { + if size.starts_with('0') && num_digits > 1 { return NumberSystem::Octal; } From 8ef926c6e863c0b274df9c33b326d45e7e24b10e Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:26:17 -0700 Subject: [PATCH 10/17] core: refactor num and unit split --- src/uucore/src/lib/parser/parse_size.rs | 31 +++++++------------------ 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index be642ef8b..716cf8d79 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -80,30 +80,15 @@ impl<'parser> Parser<'parser> { // Split the size argument into numeric and unit parts // For example, if the argument is "123K", the numeric part is "123", and // the unit is "K" - let (numeric_string, unit) = match number_system { - NumberSystem::Hexadecimal => { - let numeric_string: String = size - .chars() - .take(2) - .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) - .collect(); - let unit: String = size.chars().skip(numeric_string.len()).collect(); - - (numeric_string, unit) - } - _ => { - let mut unit: String = size - .chars() - .rev() - .take_while(|c| c.is_alphabetic()) - .collect(); - unit = unit.chars().rev().collect(); - let numeric_string = size.chars().take(size.len() - unit.len()).collect(); - - (numeric_string, unit) - } + let numeric_string: String = match number_system { + NumberSystem::Hexadecimal => size + .chars() + .take(2) + .chain(size.chars().skip(2).take_while(|c| c.is_ascii_hexdigit())) + .collect(), + _ => size.chars().take_while(|c| c.is_ascii_digit()).collect(), }; - let mut unit: &str = unit.as_str(); + let mut unit: &str = &size[numeric_string.len()..]; if let Some(default_unit) = self.default_unit { // Check if `unit` is empty then assigns `default_unit` to `unit` From 0465553f6ebd36cebf4d0d71a476b957cb2b29fc Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:27:09 -0700 Subject: [PATCH 11/17] core: size parser treat 000 as decimal --- src/uucore/src/lib/parser/parse_size.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index 716cf8d79..af4cf990d 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -197,7 +197,8 @@ impl<'parser> Parser<'parser> { .take_while(|c| c.is_ascii_digit()) .collect::() .len(); - if size.starts_with('0') && num_digits > 1 { + let all_zeros = size.chars().all(|c| c == '0'); + if size.starts_with('0') && num_digits > 1 && !all_zeros { return NumberSystem::Octal; } From a3979201836e97cea203eee0f328d89ff75d8a7b Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 00:33:14 -0700 Subject: [PATCH 12/17] core: refactor parse_number in size parser --- src/uucore/src/lib/parser/parse_size.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index af4cf990d..c6a02dfae 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -156,25 +156,16 @@ impl<'parser> Parser<'parser> { if numeric_string.is_empty() { 1 } else { - match numeric_string.parse() { - Ok(n) => n, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(&numeric_string, 10, size)? } } NumberSystem::Octal => { let trimmed_string = numeric_string.trim_start_matches('0'); - match u64::from_str_radix(trimmed_string, 8) { - Ok(res) => res, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(trimmed_string, 8, size)? } NumberSystem::Hexadecimal => { let trimmed_string = numeric_string.trim_start_matches("0x"); - match u64::from_str_radix(trimmed_string, 16) { - Ok(res) => res, - Err(_) => return Err(ParseSizeError::parse_failure(size)), - } + self.parse_number(trimmed_string, 16, size)? } }; @@ -204,6 +195,16 @@ impl<'parser> Parser<'parser> { NumberSystem::Decimal } + + fn parse_number( + &self, + numeric_string: &str, + radix: u32, + original_size: &str, + ) -> Result { + u64::from_str_radix(numeric_string, radix) + .map_err(|_| ParseSizeError::ParseFailure(original_size.to_string())) + } } /// Parse a size string into a number of bytes. From 58bf9989199c0713ed6d4e385a594f0424e100cc Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 30 May 2023 15:49:53 +0200 Subject: [PATCH 13/17] Ignore some words --- src/uucore/src/lib/parser/parse_size.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index c6a02dfae..2ea84e389 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -3,7 +3,7 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) hdsf ghead gtail +// spell-checker:ignore (ToDO) hdsf ghead gtail ACDBK hexdigit use std::error::Error; use std::fmt; From f10faf21bc328fb5704cdb0361c5084e20e84d7a Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 11:26:39 -0700 Subject: [PATCH 14/17] head: interpret size as decimal --- src/uu/head/src/parse.rs | 8 +++++++- tests/by-util/test_head.rs | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/uu/head/src/parse.rs b/src/uu/head/src/parse.rs index b24b6591e..56c359a0c 100644 --- a/src/uu/head/src/parse.rs +++ b/src/uu/head/src/parse.rs @@ -114,7 +114,13 @@ pub fn parse_num(src: &str) -> Result<(u64, bool), ParseSizeError> { return Err(ParseSizeError::ParseFailure(src.to_string())); } - parse_size(size_string).map(|n| (n, all_but_last)) + // remove leading zeros so that size is interpreted as decimal, not octal + let trimmed_string = size_string.trim_start_matches('0'); + if trimmed_string.is_empty() { + Ok((0, all_but_last)) + } else { + parse_size(trimmed_string).map(|n| (n, all_but_last)) + } } #[cfg(test)] diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 571bfb3a8..0e1eafc86 100644 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -189,6 +189,15 @@ fn test_no_such_file_or_directory() { .stderr_contains("cannot open 'no_such_file.toml' for reading: No such file or directory"); } +#[test] +fn test_lines_leading_zeros() { + new_ucmd!() + .arg("--lines=010") + .pipe_in("\n\n\n\n\n\n\n\n\n\n\n\n") + .succeeds() + .stdout_is("\n\n\n\n\n\n\n\n\n\n"); +} + /// Test that each non-existent files gets its own error message printed. #[test] fn test_multiple_nonexistent_files() { From b93bae2964ce52ad24dbd16ee110593096efb944 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 13:34:00 -0700 Subject: [PATCH 15/17] tail: fix parsing logic and add quote --- src/uu/tail/src/args.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/uu/tail/src/args.rs b/src/uu/tail/src/args.rs index 5fb5ef2b3..955a9d8f6 100644 --- a/src/uu/tail/src/args.rs +++ b/src/uu/tail/src/args.rs @@ -80,7 +80,7 @@ impl FilterMode { Err(e) => { return Err(USimpleError::new( 1, - format!("invalid number of bytes: {e}"), + format!("invalid number of bytes: '{e}'"), )) } } @@ -415,16 +415,17 @@ fn parse_num(src: &str) -> Result { starting_with = true; } } - } else { - return Err(ParseSizeError::ParseFailure(src.to_string())); } - parse_size(size_string).map(|n| match (n, starting_with) { - (0, true) => Signum::PlusZero, - (0, false) => Signum::MinusZero, - (n, true) => Signum::Positive(n), - (n, false) => Signum::Negative(n), - }) + match parse_size(size_string) { + Ok(n) => match (n, starting_with) { + (0, true) => Ok(Signum::PlusZero), + (0, false) => Ok(Signum::MinusZero), + (n, true) => Ok(Signum::Positive(n)), + (n, false) => Ok(Signum::Negative(n)), + }, + Err(_) => Err(ParseSizeError::ParseFailure(size_string.to_string())), + } } pub fn parse_args(args: impl uucore::Args) -> UResult { From f10059db026cdda3def757ee6a430b122e59bd83 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 13:40:41 -0700 Subject: [PATCH 16/17] shred: refactor get_size --- src/uu/shred/src/shred.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 1ceffe995..47c3ff6ec 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -320,16 +320,16 @@ pub fn uu_app() -> Command { } fn get_size(size_str_opt: Option) -> Option { - match size_str_opt { - Some(size) => match parse_size(size.as_str()) { - Ok(res) => Some(res), - Err(_) => { + size_str_opt + .as_ref() + .and_then(|size| parse_size(size.as_str()).ok()) + .or_else(|| { + if let Some(size) = size_str_opt { show_error!("invalid file size: {}", size.quote()); - std::process::exit(1) + std::process::exit(1); } - }, - None => None, - } + None + }) } fn pass_name(pass_type: &PassType) -> String { From 4b09b917cd3e4687da28fb91b6d3d3eef0ffea54 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 30 May 2023 15:08:22 -0700 Subject: [PATCH 17/17] shred: use exact if size is given --- src/uu/shred/src/shred.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 47c3ff6ec..5ec1d1213 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -239,7 +239,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .get_one::(options::SIZE) .map(|s| s.to_string()); let size = get_size(size_arg); - let exact = matches.get_flag(options::EXACT) && size.is_none(); // if -s is given, ignore -x + let exact = matches.get_flag(options::EXACT) || size.is_some(); let zero = matches.get_flag(options::ZERO); let verbose = matches.get_flag(options::VERBOSE);