Skip to content

Commit 5e0c64c

Browse files
sargassargas
committed
printf: Consistently handle negative widths/precision
Also allows character constants with " instead of ', and for interpolated values with %b to use \0XXX notation for octal bytes
1 parent 48cbcc4 commit 5e0c64c

File tree

4 files changed

+154
-32
lines changed

4 files changed

+154
-32
lines changed

‎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/num_parser.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,8 @@ fn parse(
354354
input: &str,
355355
integral_only: bool,
356356
) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
357-
// Parse the "'" prefix separately
358-
if let Some(rest) = input.strip_prefix('\'') {
357+
// Parse the " and ' prefixes separately
358+
if let Some(rest) = input.strip_prefix(['\'', '"']) {
359359
let mut chars = rest.char_indices().fuse();
360360
let v = chars
361361
.next()
@@ -459,11 +459,11 @@ fn parse(
459459

460460
// If nothing has been parsed, check if this is a special value, or declare the parsing unsuccessful
461461
if let Some((0, _)) = chars.peek() {
462-
if integral_only {
463-
return Err(ExtendedParserError::NotNumeric);
462+
return if integral_only {
463+
Err(ExtendedParserError::NotNumeric)
464464
} else {
465-
return parse_special_value(unsigned, negative);
466-
}
465+
parse_special_value(unsigned, negative)
466+
};
467467
}
468468

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

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

Lines changed: 46 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).unwrap_or(6);
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).unwrap_or(6);
436449
let f: ExtendedBigDecimal = args.get_extended_big_decimal();
437450

438451
if precision as u64 > i32::MAX as u64 {
@@ -446,7 +459,11 @@ impl Spec {
446459
case: *case,
447460
force_decimal: *force_decimal,
448461
positive_sign: *positive_sign,
449-
alignment: *alignment,
462+
alignment: if neg_width {
463+
NumberAlignment::Left
464+
} else {
465+
*alignment
466+
},
450467
}
451468
.fmt(writer, &f)
452469
.map_err(FormatError::IoError)
@@ -455,18 +472,7 @@ impl Spec {
455472
}
456473
}
457474

458-
fn resolve_asterisk<'a>(
459-
option: Option<CanAsterisk<usize>>,
460-
mut args: impl ArgumentIter<'a>,
461-
) -> Option<usize> {
462-
match option {
463-
None => None,
464-
Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)),
465-
Some(CanAsterisk::Fixed(w)) => Some(w),
466-
}
467-
}
468-
469-
fn resolve_asterisk_maybe_negative<'a>(
475+
fn resolve_asterisk_width<'a>(
470476
option: Option<CanAsterisk<usize>>,
471477
mut args: impl ArgumentIter<'a>,
472478
) -> Option<(usize, bool)> {
@@ -484,6 +490,21 @@ fn resolve_asterisk_maybe_negative<'a>(
484490
}
485491
}
486492

493+
fn resolve_asterisk_precision<'a>(
494+
option: Option<CanAsterisk<usize>>,
495+
mut args: impl ArgumentIter<'a>,
496+
) -> Option<usize> {
497+
match option {
498+
None => None,
499+
Some(CanAsterisk::Asterisk) => match args.get_i64() {
500+
v if v >= 0 => usize::try_from(v).ok(),
501+
v if v < 0 => Some(0usize),
502+
_ => None,
503+
},
504+
Some(CanAsterisk::Fixed(w)) => Some(w),
505+
}
506+
}
507+
487508
fn write_padded(
488509
mut writer: impl Write,
489510
text: &[u8],

‎tests/by-util/test_printf.rs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,19 @@ fn escaped_unicode_eight_digit() {
8282
.stdout_only("ĥ");
8383
}
8484

85+
#[test]
86+
fn escaped_unicode_null_byte() {
87+
new_ucmd!()
88+
.args(&["\\0001_"])
89+
.succeeds()
90+
.stdout_is_bytes([0u8, b'1', b'_']);
91+
92+
new_ucmd!()
93+
.args(&["%b", "\\0001_"])
94+
.succeeds()
95+
.stdout_is_bytes([1u8, b'_']);
96+
}
97+
8598
#[test]
8699
fn escaped_percent_sign() {
87100
new_ucmd!()
@@ -260,6 +273,16 @@ fn sub_num_int_char_const_in() {
260273
.args(&["emoji is %i", "'🙃"])
261274
.succeeds()
262275
.stdout_only("emoji is 128579");
276+
277+
new_ucmd!()
278+
.args(&["ninety seven is %i", "\"a"])
279+
.succeeds()
280+
.stdout_only("ninety seven is 97");
281+
282+
new_ucmd!()
283+
.args(&["emoji is %i", "\"🙃"])
284+
.succeeds()
285+
.stdout_only("emoji is 128579");
263286
}
264287

265288
#[test]
@@ -546,6 +569,76 @@ fn sub_any_asterisk_negative_first_param() {
546569
.stdout_only("a(x )b"); // Would be 'a( x)b' if -5 was 5
547570
}
548571

572+
#[test]
573+
fn sub_any_asterisk_first_param_with_integer() {
574+
new_ucmd!()
575+
.args(&["|%*d|", "3", "0"])
576+
.succeeds()
577+
.stdout_only("| 0|");
578+
579+
new_ucmd!()
580+
.args(&["|%*d|", "1", "0"])
581+
.succeeds()
582+
.stdout_only("|0|");
583+
584+
new_ucmd!()
585+
.args(&["|%*d|", "0", "0"])
586+
.succeeds()
587+
.stdout_only("|0|");
588+
589+
new_ucmd!()
590+
.args(&["|%*d|", "-1", "0"])
591+
.succeeds()
592+
.stdout_only("|0|");
593+
594+
// Negative widths are left-aligned
595+
new_ucmd!()
596+
.args(&["|%*d|", "-3", "0"])
597+
.succeeds()
598+
.stdout_only("|0 |");
599+
}
600+
601+
#[test]
602+
fn sub_any_asterisk_second_param_with_integer() {
603+
new_ucmd!()
604+
.args(&["|%.*d|", "3", "10"])
605+
.succeeds()
606+
.stdout_only("|010|");
607+
608+
new_ucmd!()
609+
.args(&["|%*.d|", "1", "10"])
610+
.succeeds()
611+
.stdout_only("|10|");
612+
613+
new_ucmd!()
614+
.args(&["|%.*d|", "0", "10"])
615+
.succeeds()
616+
.stdout_only("|10|");
617+
618+
new_ucmd!()
619+
.args(&["|%.*d|", "-1", "10"])
620+
.succeeds()
621+
.stdout_only("|10|");
622+
623+
new_ucmd!()
624+
.args(&["|%.*d|", "-2", "10"])
625+
.succeeds()
626+
.stdout_only("|10|");
627+
628+
new_ucmd!()
629+
.args(&["|%.*d|", &i64::MIN.to_string(), "10"])
630+
.succeeds()
631+
.stdout_only("|10|");
632+
633+
new_ucmd!()
634+
.args(&["|%.*d|", &format!("-{}", u128::MAX), "10"])
635+
.fails_with_code(1)
636+
.stdout_is("|10|")
637+
.stderr_is(
638+
"printf: '-340282366920938463463374607431768211455': Numerical result out of range\n",
639+
);
640+
}
641+
549642
#[test]
550643
fn sub_any_specifiers_no_params() {
551644
new_ucmd!()
@@ -901,6 +994,14 @@ fn negative_zero_padding_with_space_test() {
901994
.stdout_only("-01");
902995
}
903996

997+
#[test]
998+
fn spaces_before_numbers_are_ignored() {
999+
new_ucmd!()
1000+
.args(&["%*.*d", " 5", " 3", " 6"])
1001+
.succeeds()
1002+
.stdout_only(" 006");
1003+
}
1004+
9041005
#[test]
9051006
fn float_with_zero_precision_should_pad() {
9061007
new_ucmd!()

0 commit comments

Comments
 (0)