From 27487be267e13a803936efd3c7db82b1293b6806 Mon Sep 17 00:00:00 2001 From: Justin Tracey Date: Mon, 28 Apr 2025 13:21:48 -0400 Subject: [PATCH] printf: use non-zero indexes --- .../src/lib/features/format/argument.rs | 51 ++++++++++--------- src/uucore/src/lib/features/format/spec.rs | 26 ++++++---- tests/by-util/test_printf.rs | 5 ++ 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/uucore/src/lib/features/format/argument.rs b/src/uucore/src/lib/features/format/argument.rs index 4ca5a4a96..f3edbae55 100644 --- a/src/uucore/src/lib/features/format/argument.rs +++ b/src/uucore/src/lib/features/format/argument.rs @@ -12,7 +12,7 @@ use crate::{ show_error, show_warning, }; use os_display::Quotable; -use std::ffi::OsStr; +use std::{ffi::OsStr, num::NonZero}; /// An argument for formatting /// @@ -129,8 +129,9 @@ impl<'a> FormatArguments<'a> { } } - fn get_at_relative_position(&mut self, pos: usize) -> Option<&'a FormatArgument> { - let pos = pos.saturating_sub(1).saturating_add(self.current_offset); + fn get_at_relative_position(&mut self, pos: NonZero) -> Option<&'a FormatArgument> { + let pos: usize = pos.into(); + let pos = (pos - 1).saturating_add(self.current_offset); self.highest_arg_position = Some(self.highest_arg_position.map_or(pos, |x| x.max(pos))); self.args.get(pos) } @@ -281,6 +282,10 @@ mod tests { assert!(args.is_exhausted()); } + fn non_zero_pos(n: usize) -> ArgumentLocation { + ArgumentLocation::Position(NonZero::new(n).unwrap()) + } + #[test] fn test_position_access_pattern() { // Test with consistent positional access patterns @@ -297,23 +302,23 @@ mod tests { ]); // First batch - positional access - assert_eq!(b'b', args.next_char(&ArgumentLocation::Position(2))); // Position 2 - assert_eq!(b'a', args.next_char(&ArgumentLocation::Position(1))); // Position 1 - assert_eq!(b'c', args.next_char(&ArgumentLocation::Position(3))); // Position 3 + assert_eq!(b'b', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'a', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same positional pattern - assert_eq!(b'e', args.next_char(&ArgumentLocation::Position(2))); // Position 2 - assert_eq!(b'd', args.next_char(&ArgumentLocation::Position(1))); // Position 1 - assert_eq!(b'f', args.next_char(&ArgumentLocation::Position(3))); // Position 3 + assert_eq!(b'e', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'd', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'f', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(!args.is_exhausted()); // Third batch - same positional pattern (last batch) - assert_eq!(b'h', args.next_char(&ArgumentLocation::Position(2))); // Position 2 - assert_eq!(b'g', args.next_char(&ArgumentLocation::Position(1))); // Position 1 - assert_eq!(b'i', args.next_char(&ArgumentLocation::Position(3))); // Position 3 + assert_eq!(b'h', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'g', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'i', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(args.is_exhausted()); } @@ -334,19 +339,19 @@ mod tests { // First batch - mix of sequential and positional assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); // Sequential - assert_eq!(b'c', args.next_char(&ArgumentLocation::Position(3))); // Positional + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); // Positional args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same mixed pattern assert_eq!(b'd', args.next_char(&ArgumentLocation::NextArgument)); // Sequential - assert_eq!(b'f', args.next_char(&ArgumentLocation::Position(3))); // Positional + assert_eq!(b'f', args.next_char(&non_zero_pos(3))); // Positional args.start_next_batch(); assert!(!args.is_exhausted()); // Last batch - same mixed pattern assert_eq!(b'g', args.next_char(&ArgumentLocation::NextArgument)); // Sequential - assert_eq!(b'\0', args.next_char(&ArgumentLocation::Position(3))); // Out of bounds + assert_eq!(b'\0', args.next_char(&non_zero_pos(3))); // Out of bounds args.start_next_batch(); assert!(args.is_exhausted()); } @@ -419,16 +424,16 @@ mod tests { let mut args = FormatArguments::new(&args); // First batch - positional access of different types - assert_eq!(b'a', args.next_char(&ArgumentLocation::Position(1))); - assert_eq!("test", args.next_string(&ArgumentLocation::Position(2))); - assert_eq!(42, args.next_u64(&ArgumentLocation::Position(3))); + assert_eq!(b'a', args.next_char(&non_zero_pos(1))); + assert_eq!("test", args.next_string(&non_zero_pos(2))); + assert_eq!(42, args.next_u64(&non_zero_pos(3))); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern - assert_eq!(b'b', args.next_char(&ArgumentLocation::Position(1))); - assert_eq!("more", args.next_string(&ArgumentLocation::Position(2))); - assert_eq!(99, args.next_u64(&ArgumentLocation::Position(3))); + assert_eq!(b'b', args.next_char(&non_zero_pos(1))); + assert_eq!("more", args.next_string(&non_zero_pos(2))); + assert_eq!(99, args.next_u64(&non_zero_pos(3))); args.start_next_batch(); assert!(args.is_exhausted()); } @@ -446,13 +451,13 @@ mod tests { // First batch assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); - assert_eq!(b'c', args.next_char(&ArgumentLocation::Position(3))); + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch (partial) assert_eq!(b'd', args.next_char(&ArgumentLocation::NextArgument)); - assert_eq!(b'\0', args.next_char(&ArgumentLocation::Position(3))); // Out of bounds + assert_eq!(b'\0', args.next_char(&non_zero_pos(3))); // Out of bounds args.start_next_batch(); assert!(args.is_exhausted()); } diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index 1e5d7240f..d22626590 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -16,7 +16,7 @@ use super::{ parse_escape_only, }; use crate::format::FormatArguments; -use std::{io::Write, ops::ControlFlow}; +use std::{io::Write, num::NonZero, ops::ControlFlow}; /// A parsed specification for formatting a value /// @@ -70,7 +70,7 @@ pub enum Spec { #[derive(Clone, Copy, Debug)] pub enum ArgumentLocation { NextArgument, - Position(usize), + Position(NonZero), } /// Precision and width specified might use an asterisk to indicate that they are @@ -156,7 +156,9 @@ impl Spec { let start = *rest; // Check for a positional specifier (%m$) - let position = eat_argument_position(rest, &mut index); + let Some(position) = eat_argument_position(rest, &mut index) else { + return Err(&start[..index]); + }; let flags = Flags::parse(rest, &mut index); @@ -566,19 +568,19 @@ fn write_padded( } // Check for a number ending with a '$' -fn eat_argument_position(rest: &mut &[u8], index: &mut usize) -> ArgumentLocation { +fn eat_argument_position(rest: &mut &[u8], index: &mut usize) -> Option { let original_index = *index; if let Some(pos) = eat_number(rest, index) { if let Some(&b'$') = rest.get(*index) { *index += 1; - ArgumentLocation::Position(pos) + Some(ArgumentLocation::Position(NonZero::new(pos)?)) } else { *index = original_index; - ArgumentLocation::NextArgument + Some(ArgumentLocation::NextArgument) } } else { *index = original_index; - ArgumentLocation::NextArgument + Some(ArgumentLocation::NextArgument) } } @@ -586,7 +588,7 @@ fn eat_asterisk_or_number(rest: &mut &[u8], index: &mut usize) -> Option