RGBCube/uutils-coreutils
1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-27 19:17:43 +00:00
Code Issues Activity

Merge pull request #7675 from Qelxiros/7669-sleep-hex-parsing

Browse source 
fix(sleep): use uucore's from_str to support parsing hex
This commit is contained in:
Dorian Péron 2025-04-15 00:50:15 +02:00 • committed by GitHub
commit 3dcee17572
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 122 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/uu/sleep/Cargo.toml
View file

@ -20,7 +20,7 @@ path = "src/sleep.rs"
[dependencies]
clap = { workspace = true }
fundu = { workspace = true }
uucore = { workspace = true }
uucore = { workspace = true, features = ["parser"] }
[[bin]]
name = "sleep"

31
src/uu/sleep/src/sleep.rs
View file

@ -8,11 +8,12 @@ use std::time::Duration;
use uucore::{
error::{UResult, USimpleError, UUsageError},
format_usage, help_about, help_section, help_usage, show_error,
format_usage, help_about, help_section, help_usage,
parser::parse_time,
show_error,
};
use clap::{Arg, ArgAction, Command};
use fundu::{DurationParser, ParseError, SaturatingInto};
static ABOUT: &str = help_about!("sleep.md");
const USAGE: &str = help_usage!("sleep.md");
@ -61,37 +62,17 @@ pub fn uu_app() -> Command {
fn sleep(args: &[&str]) -> UResult<()> {
let mut arg_error = false;
use fundu::TimeUnit::{Day, Hour, Minute, Second};
let parser = DurationParser::with_time_units(&[Second, Minute, Hour, Day]);
let sleep_dur = args
.iter()
.filter_map(|input| match parser.parse(input.trim()) {
.filter_map(|input| match parse_time::from_str(input) {
Ok(duration) => Some(duration),
Err(error) => {
arg_error = true;
let reason = match error {
ParseError::Empty if input.is_empty() => "Input was empty".to_string(),
ParseError::Empty => "Found only whitespace in input".to_string(),
ParseError::Syntax(pos, description)
| ParseError::TimeUnit(pos, description) => {
format!("{description} at position {}", pos.saturating_add(1))
}
ParseError::NegativeExponentOverflow | ParseError::PositiveExponentOverflow => {
"Exponent was out of bounds".to_string()
}
ParseError::NegativeNumber => "Number was negative".to_string(),
error => error.to_string(),
};
show_error!("invalid time interval '{input}': {reason}");
show_error!("{error}");
None
}
})
.fold(Duration::ZERO, |acc, n| {
acc.saturating_add(SaturatingInto::<Duration>::saturating_into(n))
});
.fold(Duration::ZERO, |acc, n| acc.saturating_add(n));
if arg_error {
return Err(UUsageError::new(1, ""));

81
src/uucore/src/lib/features/parser/num_parser.rs
View file

@ -150,7 +150,7 @@ impl ExtendedParser for i64 {
}
}
match parse(input, true) {
match parse(input, ParseTarget::Integral, &[]) {
Ok(v) => into_i64(v),
Err(e) => Err(e.map(into_i64)),
}
@ -187,7 +187,7 @@ impl ExtendedParser for u64 {
}
}
match parse(input, true) {
match parse(input, ParseTarget::Integral, &[]) {
Ok(v) => into_u64(v),
Err(e) => Err(e.map(into_u64)),
}
@ -219,7 +219,7 @@ impl ExtendedParser for f64 {
Ok(v)
}
match parse(input, false) {
match parse(input, ParseTarget::Decimal, &[]) {
Ok(v) => into_f64(v),
Err(e) => Err(e.map(into_f64)),
}
@ -231,14 +231,15 @@ impl ExtendedParser for ExtendedBigDecimal {
fn extended_parse(
input: &str,
) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
parse(input, false)
parse(input, ParseTarget::Decimal, &[])
}
}
fn parse_special_value(
input: &str,
fn parse_special_value<'a>(
input: &'a str,
negative: bool,
) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
allowed_suffixes: &'a [(char, u32)],
) -> Result<ExtendedBigDecimal, ExtendedParserError<'a, ExtendedBigDecimal>> {
let input_lc = input.to_ascii_lowercase();
// Array of ("String to match", return value when sign positive, when sign negative)
@ -254,7 +255,14 @@ fn parse_special_value(
if negative {
special = -special;
}
let match_len = str.len();
let mut match_len = str.len();
if let Some(ch) = input.chars().nth(str.chars().count()) {
if allowed_suffixes.iter().any(|(c, _)| ch == *c) {
// multiplying is unnecessary for these special values, but we have to note that
// we processed the character to avoid a partial match error
match_len += 1;
}
}
return if input.len() == match_len {
Ok(special)
} else {
@ -381,24 +389,34 @@ fn construct_extended_big_decimal<'a>(
Ok(ExtendedBigDecimal::BigDecimal(bd))
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ParseTarget {
Decimal,
Integral,
Duration,
}
// TODO: As highlighted by clippy, this function _is_ high cognitive complexity, jumps
// around between integer and float parsing, and should be split in multiple parts.
#[allow(clippy::cognitive_complexity)]
fn parse(
input: &str,
integral_only: bool,
) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
pub(crate) fn parse<'a>(
input: &'a str,
target: ParseTarget,
allowed_suffixes: &'a [(char, u32)],
) -> Result<ExtendedBigDecimal, ExtendedParserError<'a, ExtendedBigDecimal>> {
// Parse the " and ' prefixes separately
if let Some(rest) = input.strip_prefix(['\'', '"']) {
let mut chars = rest.char_indices().fuse();
let v = chars
.next()
.map(|(_, c)| ExtendedBigDecimal::BigDecimal(u32::from(c).into()));
return match (v, chars.next()) {
(Some(v), None) => Ok(v),
(Some(v), Some((i, _))) => Err(ExtendedParserError::PartialMatch(v, &rest[i..])),
(None, _) => Err(ExtendedParserError::NotNumeric),
};
if target != ParseTarget::Duration {
if let Some(rest) = input.strip_prefix(['\'', '"']) {
let mut chars = rest.char_indices().fuse();
let v = chars
.next()
.map(|(_, c)| ExtendedBigDecimal::BigDecimal(u32::from(c).into()));
return match (v, chars.next()) {
(Some(v), None) => Ok(v),
(Some(v), Some((i, _))) => Err(ExtendedParserError::PartialMatch(v, &rest[i..])),
(None, _) => Err(ExtendedParserError::NotNumeric),
};
}
}
let trimmed_input = input.trim_ascii_start();
@ -419,7 +437,7 @@ fn parse(
let (base, rest) = if let Some(rest) = unsigned.strip_prefix('0') {
if let Some(rest) = rest.strip_prefix(['x', 'X']) {
(Base::Hexadecimal, rest)
} else if integral_only {
} else if target == ParseTarget::Integral {
// Binary/Octal only allowed for integer parsing.
if let Some(rest) = rest.strip_prefix(['b', 'B']) {
(Base::Binary, rest)
@ -447,7 +465,7 @@ fn parse(
}
// Parse fractional/exponent part of the number for supported bases.
if matches!(base, Base::Decimal | Base::Hexadecimal) && !integral_only {
if matches!(base, Base::Decimal | Base::Hexadecimal) && target != ParseTarget::Integral {
// Parse the fractional part of the number if there can be one and the input contains
// a '.' decimal separator.
if matches!(chars.peek(), Some(&(_, '.'))) {
@ -493,13 +511,24 @@ fn parse(
// If nothing has been parsed, check if this is a special value, or declare the parsing unsuccessful
if let Some((0, _)) = chars.peek() {
return if integral_only {
return if target == ParseTarget::Integral {
Err(ExtendedParserError::NotNumeric)
} else {
parse_special_value(unsigned, negative)
parse_special_value(unsigned, negative, allowed_suffixes)
};
}
if let Some((_, ch)) = chars.peek() {
if let Some(times) = allowed_suffixes
.iter()
.find(|(c, _)| ch == c)
.map(|&(_, t)| t)
{
chars.next();
digits *= times;
}
}
let ebd_result = construct_extended_big_decimal(digits, negative, base, scale, exponent);
// Return what has been parsed so far. If there are extra characters, mark the

32
src/uucore/src/lib/features/parser/parse_time.rs
View file

@ -11,9 +11,8 @@
use crate::{
display::Quotable,
extendedbigdecimal::ExtendedBigDecimal,
parser::num_parser::{ExtendedParser, ExtendedParserError},
parser::num_parser::{self, ExtendedParserError, ParseTarget},
};
use bigdecimal::BigDecimal;
use num_traits::Signed;
use num_traits::ToPrimitive;
use num_traits::Zero;
@ -59,26 +58,18 @@ pub fn from_str(string: &str) -> Result<Duration, String> {
let len = string.len();
if len == 0 {
return Err("empty string".to_owned());
}
let Some(slice) = string.get(..len - 1) else {
return Err(format!("invalid time interval {}", string.quote()));
};
let (numstr, times) = match string.chars().next_back().unwrap() {
's' => (slice, 1),
'm' => (slice, 60),
'h' => (slice, 60 * 60),
'd' => (slice, 60 * 60 * 24),
val if !val.is_alphabetic() => (string, 1),
_ => match string.to_ascii_lowercase().as_str() {
"inf" | "infinity" => ("inf", 1),
_ => return Err(format!("invalid time interval {}", string.quote())),
},
};
let num = match ExtendedBigDecimal::extended_parse(numstr) {
}
let num = match num_parser::parse(
string,
ParseTarget::Duration,
&[('s', 1), ('m', 60), ('h', 60 * 60), ('d', 60 * 60 * 24)],
) {
Ok(ebd) | Err(ExtendedParserError::Overflow(ebd)) => ebd,
Err(ExtendedParserError::Underflow(_)) => return Ok(NANOSECOND_DURATION),
_ => return Err(format!("invalid time interval {}", string.quote())),
_ => {
return Err(format!("invalid time interval {}", string.quote()));
}
};
// Allow non-negative durations (-0 is fine), and infinity.
@ -89,9 +80,6 @@ pub fn from_str(string: &str) -> Result<Duration, String> {
_ => return Err(format!("invalid time interval {}", string.quote())),
};
// Pre-multiply times to avoid precision loss
let num: BigDecimal = num * times;
// Transform to nanoseconds (9 digits after decimal point)
let (nanos_bi, _) = num.with_scale(9).into_bigint_and_scale();

62
tests/by-util/test_sleep.rs
View file

@ -4,7 +4,8 @@
// file that was distributed with this source code.
use rstest::rstest;
// spell-checker:ignore dont SIGBUS SIGSEGV sigsegv sigbus
use uucore::display::Quotable;
// spell-checker:ignore dont SIGBUS SIGSEGV sigsegv sigbus infd
use uutests::new_ucmd;
use uutests::util::TestScenario;
use uutests::util_name;
@ -19,11 +20,11 @@ fn test_invalid_time_interval() {
new_ucmd!()
.arg("xyz")
.fails()
.usage_error("invalid time interval 'xyz': Invalid input: xyz");
.usage_error("invalid time interval 'xyz'");
new_ucmd!()
.args(&["--", "-1"])
.fails()
.usage_error("invalid time interval '-1': Number was negative");
.usage_error("invalid time interval '-1'");
}
#[test]
@ -228,9 +229,7 @@ fn test_sleep_when_multiple_inputs_exceed_max_duration_then_no_error() {
#[rstest]
#[case::whitespace_prefix(" 0.1s")]
#[case::multiple_whitespace_prefix(" 0.1s")]
#[case::whitespace_suffix("0.1s ")]
#[case::mixed_newlines_spaces_tabs("\n\t0.1s \n ")]
fn test_sleep_when_input_has_whitespace_then_no_error(#[case] input: &str) {
fn test_sleep_when_input_has_leading_whitespace_then_no_error(#[case] input: &str) {
new_ucmd!()
.arg(input)
.timeout(Duration::from_secs(10))
@ -238,6 +237,17 @@ fn test_sleep_when_input_has_whitespace_then_no_error(#[case] input: &str) {
.no_output();
}
#[rstest]
#[case::whitespace_suffix("0.1s ")]
#[case::mixed_newlines_spaces_tabs("\n\t0.1s \n ")]
fn test_sleep_when_input_has_trailing_whitespace_then_error(#[case] input: &str) {
new_ucmd!()
.arg(input)
.timeout(Duration::from_secs(10))
.fails()
.usage_error(format!("invalid time interval {}", input.quote()));
}
#[rstest]
#[case::only_space(" ")]
#[case::only_tab("\t")]
@ -247,16 +257,14 @@ fn test_sleep_when_input_has_only_whitespace_then_error(#[case] input: &str) {
.arg(input)
.timeout(Duration::from_secs(10))
.fails()
.usage_error(format!(
"invalid time interval '{input}': Found only whitespace in input"
));
.usage_error(format!("invalid time interval {}", input.quote()));
}
#[test]
fn test_sleep_when_multiple_input_some_with_error_then_shows_all_errors() {
let expected = "invalid time interval 'abc': Invalid input: abc\n\
sleep: invalid time interval '1years': Invalid time unit: 'years' at position 2\n\
sleep: invalid time interval ' ': Found only whitespace in input";
let expected = "invalid time interval 'abc'\n\
sleep: invalid time interval '1years'\n\
sleep: invalid time interval ' '";
// Even if one of the arguments is valid, but the rest isn't, we should still fail and exit early.
// So, the timeout of 10 seconds ensures we haven't executed `thread::sleep` with the only valid
@ -273,7 +281,35 @@ fn test_negative_interval() {
new_ucmd!()
.args(&["--", "-1"])
.fails()
.usage_error("invalid time interval '-1': Number was negative");
.usage_error("invalid time interval '-1'");
}
#[rstest]
#[case::int("0x0")]
#[case::negative_zero("-0x0")]
#[case::int_suffix("0x0s")]
#[case::int_suffix("0x0h")]
#[case::frac("0x0.1")]
#[case::frac_suffix("0x0.1s")]
#[case::frac_suffix("0x0.001h")]
#[case::scientific("0x1.0p-3")]
#[case::scientific_suffix("0x1.0p-4s")]
fn test_valid_hex_duration(#[case] input: &str) {
new_ucmd!().args(&["--", input]).succeeds().no_output();
}
#[rstest]
#[case::negative("-0x1")]
#[case::negative_suffix("-0x1s")]
#[case::negative_frac_suffix("-0x0.1s")]
#[case::wrong_capitalization("infD")]
#[case::wrong_capitalization("INFD")]
#[case::wrong_capitalization("iNfD")]
fn test_invalid_hex_duration(#[case] input: &str) {
new_ucmd!()
.args(&["--", input])
.fails()
.usage_error(format!("invalid time interval {}", input.quote()));
}
#[cfg(unix)]

2
tests/by-util/test_timeout.rs
View file

@ -77,7 +77,7 @@ fn test_command_empty_args() {
new_ucmd!()
.args(&["", ""])
.fails()
.stderr_contains("timeout: empty string");
.stderr_contains("timeout: invalid time interval ''");
}
#[test]