Skip to content

Commit 3971bb3

Browse files
sylvestresylvestre
authored
Merge pull request #7620 from sargas/pass-printf-test
printf: Consistently handle negative widths/precision
2 parents f6cadac + 64ce76b commit 3971bb3

4 files changed

Lines changed: 295 additions & 32 deletions

File tree

‎src/uucore/src/lib/features/format/argument.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &s
119119
}
120120
ExtendedParserError::PartialMatch(v, rest) => {
121121
let bytes = input.as_encoded_bytes();
122-
if !bytes.is_empty() && bytes[0] == b'\'' {
122+
if !bytes.is_empty() && (bytes[0] == b'\'' || bytes[0] == b'"') {
123123
show_warning!(
124124
"{}: character(s) following character constant have been ignored",
125125
&rest,

‎src/uucore/src/lib/features/format/spec.rs‎

Lines changed: 157 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ impl Spec {
316316
match self {
317317
Self::Char { width, align_left } => {
318318
let (width, neg_width) =
319-
resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default();
319+
resolve_asterisk_width(*width, &mut args).unwrap_or_default();
320320
write_padded(writer, &[args.get_char()], width, *align_left || neg_width)
321321
}
322322
Self::String {
@@ -325,15 +325,15 @@ impl Spec {
325325
precision,
326326
} => {
327327
let (width, neg_width) =
328-
resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default();
328+
resolve_asterisk_width(*width, &mut args).unwrap_or_default();
329329

330330
// GNU does do this truncation on a byte level, see for instance:
331331
// printf "%.1s" 🙃
332332
// > �
333333
// For now, we let printf panic when we truncate within a code point.
334334
// TODO: We need to not use Rust's formatting for aligning the output,
335335
// so that we can just write bytes to stdout without panicking.
336-
let precision = resolve_asterisk(*precision, &mut args);
336+
let precision = resolve_asterisk_precision(*precision, &mut args);
337337
let s = args.get_str();
338338
let truncated = match precision {
339339
Some(p) if p < s.len() => &s[..p],
@@ -349,7 +349,7 @@ impl Spec {
349349
Self::EscapedString => {
350350
let s = args.get_str();
351351
let mut parsed = Vec::new();
352-
for c in parse_escape_only(s.as_bytes(), OctalParsing::default()) {
352+
for c in parse_escape_only(s.as_bytes(), OctalParsing::ThreeDigits) {
353353
match c.write(&mut parsed)? {
354354
ControlFlow::Continue(()) => {}
355355
ControlFlow::Break(()) => {
@@ -382,8 +382,10 @@ impl Spec {
382382
positive_sign,
383383
alignment,
384384
} => {
385-
let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
386-
let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0);
385+
let (width, neg_width) =
386+
resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
387+
let precision =
388+
resolve_asterisk_precision(*precision, &mut args).unwrap_or_default();
387389
let i = args.get_i64();
388390

389391
if precision as u64 > i32::MAX as u64 {
@@ -394,7 +396,11 @@ impl Spec {
394396
width,
395397
precision,
396398
positive_sign: *positive_sign,
397-
alignment: *alignment,
399+
alignment: if neg_width {
400+
NumberAlignment::Left
401+
} else {
402+
*alignment
403+
},
398404
}
399405
.fmt(writer, i)
400406
.map_err(FormatError::IoError)
@@ -405,8 +411,10 @@ impl Spec {
405411
precision,
406412
alignment,
407413
} => {
408-
let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
409-
let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0);
414+
let (width, neg_width) =
415+
resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
416+
let precision =
417+
resolve_asterisk_precision(*precision, &mut args).unwrap_or_default();
410418
let i = args.get_u64();
411419

412420
if precision as u64 > i32::MAX as u64 {
@@ -417,7 +425,11 @@ impl Spec {
417425
variant: *variant,
418426
precision,
419427
width,
420-
alignment: *alignment,
428+
alignment: if neg_width {
429+
NumberAlignment::Left
430+
} else {
431+
*alignment
432+
},
421433
}
422434
.fmt(writer, i)
423435
.map_err(FormatError::IoError)
@@ -431,8 +443,9 @@ impl Spec {
431443
alignment,
432444
precision,
433445
} => {
434-
let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
435-
let precision = resolve_asterisk(*precision, &mut args);
446+
let (width, neg_width) =
447+
resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
448+
let precision = resolve_asterisk_precision(*precision, &mut args);
436449
let f: ExtendedBigDecimal = args.get_extended_big_decimal();
437450

438451
if precision.is_some_and(|p| p as u64 > i32::MAX as u64) {
@@ -448,7 +461,11 @@ impl Spec {
448461
case: *case,
449462
force_decimal: *force_decimal,
450463
positive_sign: *positive_sign,
451-
alignment: *alignment,
464+
alignment: if neg_width {
465+
NumberAlignment::Left
466+
} else {
467+
*alignment
468+
},
452469
}
453470
.fmt(writer, &f)
454471
.map_err(FormatError::IoError)
@@ -457,18 +474,9 @@ impl Spec {
457474
}
458475
}
459476

460-
fn resolve_asterisk<'a>(
461-
option: Option<CanAsterisk<usize>>,
462-
mut args: impl ArgumentIter<'a>,
463-
) -> Option<usize> {
464-
match option {
465-
None => None,
466-
Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)),
467-
Some(CanAsterisk::Fixed(w)) => Some(w),
468-
}
469-
}
470-
471-
fn resolve_asterisk_maybe_negative<'a>(
477+
/// Determine the width, potentially getting a value from args
478+
/// Returns the non-negative width and whether the value should be left-aligned.
479+
fn resolve_asterisk_width<'a>(
472480
option: Option<CanAsterisk<usize>>,
473481
mut args: impl ArgumentIter<'a>,
474482
) -> Option<(usize, bool)> {
@@ -486,6 +494,23 @@ fn resolve_asterisk_maybe_negative<'a>(
486494
}
487495
}
488496

497+
/// Determines the precision, which should (if defined)
498+
/// be a non-negative number.
499+
fn resolve_asterisk_precision<'a>(
500+
option: Option<CanAsterisk<usize>>,
501+
mut args: impl ArgumentIter<'a>,
502+
) -> Option<usize> {
503+
match option {
504+
None => None,
505+
Some(CanAsterisk::Asterisk) => match args.get_i64() {
506+
v if v >= 0 => usize::try_from(v).ok(),
507+
v if v < 0 => Some(0usize),
508+
_ => None,
509+
},
510+
Some(CanAsterisk::Fixed(w)) => Some(w),
511+
}
512+
}
513+
489514
fn write_padded(
490515
mut writer: impl Write,
491516
text: &[u8],
@@ -527,3 +552,110 @@ fn eat_number(rest: &mut &[u8], index: &mut usize) -> Option<usize> {
527552
}
528553
}
529554
}
555+
556+
#[cfg(test)]
557+
mod tests {
558+
use super::*;
559+
560+
mod resolve_asterisk_width {
561+
use super::*;
562+
use crate::format::FormatArgument;
563+
564+
#[test]
565+
fn no_width() {
566+
assert_eq!(None, resolve_asterisk_width(None, Vec::new().iter()));
567+
}
568+
569+
#[test]
570+
fn fixed_width() {
571+
assert_eq!(
572+
Some((42, false)),
573+
resolve_asterisk_width(Some(CanAsterisk::Fixed(42)), Vec::new().iter())
574+
);
575+
}
576+
577+
#[test]
578+
fn asterisks_with_numbers() {
579+
assert_eq!(
580+
Some((42, false)),
581+
resolve_asterisk_width(
582+
Some(CanAsterisk::Asterisk),
583+
vec![FormatArgument::SignedInt(42)].iter()
584+
)
585+
);
586+
assert_eq!(
587+
Some((42, false)),
588+
resolve_asterisk_width(
589+
Some(CanAsterisk::Asterisk),
590+
vec![FormatArgument::Unparsed("42".to_string())].iter()
591+
)
592+
);
593+
594+
assert_eq!(
595+
Some((42, true)),
596+
resolve_asterisk_width(
597+
Some(CanAsterisk::Asterisk),
598+
vec![FormatArgument::SignedInt(-42)].iter()
599+
)
600+
);
601+
assert_eq!(
602+
Some((42, true)),
603+
resolve_asterisk_width(
604+
Some(CanAsterisk::Asterisk),
605+
vec![FormatArgument::Unparsed("-42".to_string())].iter()
606+
)
607+
);
608+
}
609+
}
610+
611+
mod resolve_asterisk_precision {
612+
use super::*;
613+
use crate::format::FormatArgument;
614+
615+
#[test]
616+
fn no_width() {
617+
assert_eq!(None, resolve_asterisk_precision(None, Vec::new().iter()));
618+
}
619+
620+
#[test]
621+
fn fixed_width() {
622+
assert_eq!(
623+
Some(42),
624+
resolve_asterisk_precision(Some(CanAsterisk::Fixed(42)), Vec::new().iter())
625+
);
626+
}
627+
628+
#[test]
629+
fn asterisks_with_numbers() {
630+
assert_eq!(
631+
Some(42),
632+
resolve_asterisk_precision(
633+
Some(CanAsterisk::Asterisk),
634+
vec![FormatArgument::SignedInt(42)].iter()
635+
)
636+
);
637+
assert_eq!(
638+
Some(42),
639+
resolve_asterisk_precision(
640+
Some(CanAsterisk::Asterisk),
641+
vec![FormatArgument::Unparsed("42".to_string())].iter()
642+
)
643+
);
644+
645+
assert_eq!(
646+
Some(0),
647+
resolve_asterisk_precision(
648+
Some(CanAsterisk::Asterisk),
649+
vec![FormatArgument::SignedInt(-42)].iter()
650+
)
651+
);
652+
assert_eq!(
653+
Some(0),
654+
resolve_asterisk_precision(
655+
Some(CanAsterisk::Asterisk),
656+
vec![FormatArgument::Unparsed("-42".to_string())].iter()
657+
)
658+
);
659+
}
660+
}
661+
}

‎src/uucore/src/lib/features/parser/num_parser.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,8 @@ fn parse(
360360
input: &str,
361361
integral_only: bool,
362362
) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
363-
// Parse the "'" prefix separately
364-
if let Some(rest) = input.strip_prefix('\'') {
363+
// Parse the " and ' prefixes separately
364+
if let Some(rest) = input.strip_prefix(['\'', '"']) {
365365
let mut chars = rest.char_indices().fuse();
366366
let v = chars
367367
.next()
@@ -465,11 +465,11 @@ fn parse(
465465

466466
// If nothing has been parsed, check if this is a special value, or declare the parsing unsuccessful
467467
if let Some((0, _)) = chars.peek() {
468-
if integral_only {
469-
return Err(ExtendedParserError::NotNumeric);
468+
return if integral_only {
469+
Err(ExtendedParserError::NotNumeric)
470470
} else {
471-
return parse_special_value(unsigned, negative);
472-
}
471+
parse_special_value(unsigned, negative)
472+
};
473473
}
474474

475475
let ebd_result = construct_extended_big_decimal(digits, negative, base, scale, exponent);

0 commit comments

Comments
 (0)