From 707e346b84a0b54a6154014100d965dfbe9934db Mon Sep 17 00:00:00 2001 From: Dorian Peron Date: Fri, 31 Jan 2025 16:14:24 +0100 Subject: [PATCH 1/2] printf: negative asterisk param changes alignement --- src/uucore/src/lib/features/format/spec.rs | 33 +++++++++++++++++++--- tests/by-util/test_printf.rs | 26 +++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index 81dbc1ebc..295aa1f98 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -314,15 +314,17 @@ impl Spec { ) -> Result<(), FormatError> { match self { Self::Char { width, align_left } => { - let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0); - write_padded(writer, &[args.get_char()], width, *align_left) + let (width, neg_width) = + resolve_asterisk_maybe_negative(*width, &mut args)?.unwrap_or_default(); + write_padded(writer, &[args.get_char()], width, *align_left || neg_width) } Self::String { width, align_left, precision, } => { - let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0); + let (width, neg_width) = + resolve_asterisk_maybe_negative(*width, &mut args)?.unwrap_or_default(); // GNU does do this truncation on a byte level, see for instance: // printf "%.1s" 🙃 @@ -336,7 +338,12 @@ impl Spec { Some(p) if p < s.len() => &s[..p], _ => s, }; - write_padded(writer, truncated.as_bytes(), width, *align_left) + write_padded( + writer, + truncated.as_bytes(), + width, + *align_left || neg_width, + ) } Self::EscapedString => { let s = args.get_str(); @@ -458,6 +465,24 @@ fn resolve_asterisk<'a>( }) } +fn resolve_asterisk_maybe_negative<'a>( + option: Option>, + mut args: impl ArgumentIter<'a>, +) -> Result, FormatError> { + Ok(match option { + None => None, + Some(CanAsterisk::Asterisk) => { + let nb = args.get_i64(); + if nb < 0 { + Some((usize::try_from(-(nb as isize)).ok().unwrap_or(0), true)) + } else { + Some((usize::try_from(nb).ok().unwrap_or(0), false)) + } + } + Some(CanAsterisk::Fixed(w)) => Some((w, false)), + }) +} + fn write_padded( mut writer: impl Write, text: &[u8], diff --git a/tests/by-util/test_printf.rs b/tests/by-util/test_printf.rs index 044817214..9eb41b44b 100644 --- a/tests/by-util/test_printf.rs +++ b/tests/by-util/test_printf.rs @@ -495,6 +495,32 @@ fn sub_any_asterisk_hex_arg() { .stdout_only("0123456789"); } +#[test] +fn sub_any_asterisk_negative_first_param() { + new_ucmd!() + .args(&["a(%*s)b", "-5", "xyz"]) + .succeeds() + .stdout_only("a(xyz )b"); // Would be 'a( xyz)b' if -5 was 5 + + // Negative octal + new_ucmd!() + .args(&["a(%*s)b", "-010", "xyz"]) + .succeeds() + .stdout_only("a(xyz )b"); + + // Negative hexadecimal + new_ucmd!() + .args(&["a(%*s)b", "-0x10", "xyz"]) + .succeeds() + .stdout_only("a(xyz )b"); + + // Should also work on %c + new_ucmd!() + .args(&["a(%*c)b", "-5", "x"]) + .succeeds() + .stdout_only("a(x )b"); // Would be 'a( x)b' if -5 was 5 +} + #[test] fn sub_any_specifiers_no_params() { new_ucmd!() From dcc2f1b72cc3880a44809932328e9b00761b7b65 Mon Sep 17 00:00:00 2001 From: Dorian Peron Date: Fri, 31 Jan 2025 16:54:01 +0100 Subject: [PATCH 2/2] printf: remove unneeded Result<> from resolve_asterisk* functions --- src/uucore/src/lib/features/format/spec.rs | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index 295aa1f98..d061a2e0d 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -315,7 +315,7 @@ impl Spec { match self { Self::Char { width, align_left } => { let (width, neg_width) = - resolve_asterisk_maybe_negative(*width, &mut args)?.unwrap_or_default(); + resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default(); write_padded(writer, &[args.get_char()], width, *align_left || neg_width) } Self::String { @@ -324,7 +324,7 @@ impl Spec { precision, } => { let (width, neg_width) = - resolve_asterisk_maybe_negative(*width, &mut args)?.unwrap_or_default(); + resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default(); // GNU does do this truncation on a byte level, see for instance: // printf "%.1s" 🙃 @@ -332,7 +332,7 @@ impl Spec { // For now, we let printf panic when we truncate within a code point. // TODO: We need to not use Rust's formatting for aligning the output, // so that we can just write bytes to stdout without panicking. - let precision = resolve_asterisk(*precision, &mut args)?; + let precision = resolve_asterisk(*precision, &mut args); let s = args.get_str(); let truncated = match precision { Some(p) if p < s.len() => &s[..p], @@ -381,8 +381,8 @@ impl Spec { positive_sign, alignment, } => { - let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0); - let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0); + let width = resolve_asterisk(*width, &mut args).unwrap_or(0); + let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0); let i = args.get_i64(); if precision as u64 > i32::MAX as u64 { @@ -404,8 +404,8 @@ impl Spec { precision, alignment, } => { - let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0); - let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0); + let width = resolve_asterisk(*width, &mut args).unwrap_or(0); + let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0); let i = args.get_u64(); if precision as u64 > i32::MAX as u64 { @@ -430,8 +430,8 @@ impl Spec { alignment, precision, } => { - let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0); - let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(6); + let width = resolve_asterisk(*width, &mut args).unwrap_or(0); + let precision = resolve_asterisk(*precision, &mut args).unwrap_or(6); let f = args.get_f64(); if precision as u64 > i32::MAX as u64 { @@ -457,19 +457,19 @@ impl Spec { fn resolve_asterisk<'a>( option: Option>, mut args: impl ArgumentIter<'a>, -) -> Result, FormatError> { - Ok(match option { +) -> Option { + match option { None => None, Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)), Some(CanAsterisk::Fixed(w)) => Some(w), - }) + } } fn resolve_asterisk_maybe_negative<'a>( option: Option>, mut args: impl ArgumentIter<'a>, -) -> Result, FormatError> { - Ok(match option { +) -> Option<(usize, bool)> { + match option { None => None, Some(CanAsterisk::Asterisk) => { let nb = args.get_i64(); @@ -480,7 +480,7 @@ fn resolve_asterisk_maybe_negative<'a>( } } Some(CanAsterisk::Fixed(w)) => Some((w, false)), - }) + } } fn write_padded(