From a48656b441224a53d2bb3face920ba5487eaae09 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Fri, 18 Jul 2008 11:10:51 +0000 Subject: printf: fix %b, fix several bugs in %*.*, fix compat issues with aborting too early, support %zd; expand testsuite function old new delta get_width_prec - 46 +46 multiconvert 82 99 +17 conv_strtod 44 54 +10 print_direc 382 391 +9 printf_main 629 633 +4 conv_strtoul 20 16 -4 conv_strtol 20 16 -4 my_xstrtoul 20 - -20 my_xstrtol 20 - -20 my_xstrtod 21 - -21 ------------------------------------------------------------------------------ (add/remove: 1/3 grow/shrink: 4/2 up/down: 86/-69) Total: 17 bytes --- coreutils/printf.c | 199 ++++++++++++++++++++++++++++++------------------- testsuite/printf.tests | 88 +++++++++++++++++++--- 2 files changed, 200 insertions(+), 87 deletions(-) diff --git a/coreutils/printf.c b/coreutils/printf.c index d877e0581..ca035cc48 100644 --- a/coreutils/printf.c +++ b/coreutils/printf.c @@ -36,14 +36,32 @@ David MacKenzie */ - // 19990508 Busy Boxed! Dave Cinege #include "libbb.h" -typedef void (*converter)(const char *arg, void *result); - -static void multiconvert(const char *arg, void *result, converter convert) +/* A note on bad input: neither bash 3.2 nor coreutils 6.10 stop on it. + * They report it: + * bash: printf: XXX: invalid number + * printf: XXX: expected a numeric value + * bash: printf: 123XXX: invalid number + * printf: 123XXX: value not completely converted + * but then they use 0 (or partially converted numeric prefix) as a value + * and continue. They exit with 1 in this case. + * Both accept insane field width/precision (e.g. %9999999999.9999999999d). + * Both print error message and assume 0 if %*.*f width/precision is "bad" + * (but negative numbers are not "bad"). + * Both accept negative numbers for %u specifier. + * + * We try to be compatible. We are not compatible here: + * - we do not accept -NUM for %u + * - exit code is 0 even if "invalid number" was seen (FIXME) + * See "if (errno)" checks in the code below. + */ + +typedef void FAST_FUNC (*converter)(const char *arg, void *result); + +static int multiconvert(const char *arg, void *result, converter convert) { char s[sizeof(int)*3 + 2]; @@ -51,43 +69,50 @@ static void multiconvert(const char *arg, void *result, converter convert) sprintf(s, "%d", (unsigned char)arg[1]); arg = s; } + errno = 0; convert(arg, result); - /* if there was conversion error, print unconverted string */ - if (errno) - fputs(arg, stderr); + if (errno) { + bb_error_msg("%s: invalid number", arg); + return 1; + } + return 0; } -static void conv_strtoul(const char *arg, void *result) +static void FAST_FUNC conv_strtoul(const char *arg, void *result) { *(unsigned long*)result = bb_strtoul(arg, NULL, 0); } -static void conv_strtol(const char *arg, void *result) +static void FAST_FUNC conv_strtol(const char *arg, void *result) { *(long*)result = bb_strtol(arg, NULL, 0); } -static void conv_strtod(const char *arg, void *result) +static void FAST_FUNC conv_strtod(const char *arg, void *result) { char *end; - /* Well, this one allows leading whitespace... so what */ - /* What I like much less is that "-" is accepted too! :( */ + /* Well, this one allows leading whitespace... so what? */ + /* What I like much less is that "-" accepted too! :( */ *(double*)result = strtod(arg, &end); - if (end[0]) errno = ERANGE; + if (end[0]) { + errno = ERANGE; + *(double*)result = 0; + } } +/* Callers should check errno to detect errors */ static unsigned long my_xstrtoul(const char *arg) { unsigned long result; - multiconvert(arg, &result, conv_strtoul); + if (multiconvert(arg, &result, conv_strtoul)) + result = 0; return result; } - static long my_xstrtol(const char *arg) { long result; - multiconvert(arg, &result, conv_strtol); + if (multiconvert(arg, &result, conv_strtol)) + result = 0; return result; } - static double my_xstrtod(const char *arg) { double result; @@ -97,14 +122,14 @@ static double my_xstrtod(const char *arg) static void print_esc_string(char *str) { - for (; *str; str++) { + while (*str) { if (*str == '\\') { str++; bb_putchar(bb_process_escape_sequence((const char **)&str)); } else { bb_putchar(*str); + str++; } - } } @@ -112,88 +137,109 @@ static void print_direc(char *format, unsigned fmt_length, int field_width, int precision, const char *argument) { + long lv; + double dv; char saved; + char *have_prec, *have_width; + + have_prec = strstr(format, ".*"); + have_width = strchr(format, '*'); + if (have_width - 1 == have_prec) + have_width = NULL; saved = format[fmt_length]; format[fmt_length] = '\0'; switch (format[fmt_length - 1]) { + case 'c': + printf(format, *argument); + break; case 'd': case 'i': - if (field_width < 0) { - if (precision < 0) - printf(format, my_xstrtol(argument)); + lv = my_xstrtol(argument); + print_long: + /* if (errno) return; - see comment at the top */ + if (!have_width) { + if (!have_prec) + printf(format, lv); else - printf(format, precision, my_xstrtol(argument)); + printf(format, precision, lv); } else { - if (precision < 0) - printf(format, field_width, my_xstrtol(argument)); + if (!have_prec) + printf(format, field_width, lv); else - printf(format, field_width, precision, my_xstrtol(argument)); + printf(format, field_width, precision, lv); } break; case 'o': case 'u': case 'x': case 'X': - if (field_width < 0) { - if (precision < 0) - printf(format, my_xstrtoul(argument)); - else - printf(format, precision, my_xstrtoul(argument)); - } else { - if (precision < 0) - printf(format, field_width, my_xstrtoul(argument)); - else - printf(format, field_width, precision, my_xstrtoul(argument)); + lv = my_xstrtoul(argument); + /* cheat: unsigned long and long have same width, so... */ + goto print_long; + case 's': + /* Are char* and long the same? (true for most arches) */ + if (sizeof(argument) == sizeof(lv)) { + lv = (long)(ptrdiff_t)argument; + goto print_long; + } else { /* Hope compiler will optimize it out */ + if (!have_width) { + if (!have_prec) + printf(format, argument); + else + printf(format, precision, argument); + } else { + if (!have_prec) + printf(format, field_width, argument); + else + printf(format, field_width, precision, argument); + } + break; } - break; case 'f': case 'e': case 'E': case 'g': case 'G': - if (field_width < 0) { - if (precision < 0) - printf(format, my_xstrtod(argument)); + dv = my_xstrtod(argument); + /* if (errno) return; */ + if (!have_width) { + if (!have_prec) + printf(format, dv); else - printf(format, precision, my_xstrtod(argument)); + printf(format, precision, dv); } else { - if (precision < 0) - printf(format, field_width, my_xstrtod(argument)); + if (!have_prec) + printf(format, field_width, dv); else - printf(format, field_width, precision, my_xstrtod(argument)); + printf(format, field_width, precision, dv); } break; - case 'c': - printf(format, *argument); - break; - case 's': - if (field_width < 0) { - if (precision < 0) - printf(format, argument); - else - printf(format, precision, argument); - } else { - if (precision < 0) - printf(format, field_width, argument); - else - printf(format, field_width, precision, argument); - } - break; - } + } /* switch */ format[fmt_length] = saved; } +/* Handle params for "%*.*f". Negative numbers are ok (compat). */ +static int get_width_prec(const char *str) +{ + int v = bb_strtoi(str, NULL, 10); + if (errno) { + bb_error_msg("%s: invalid number", str); + v = 0; + } + return v; +} + /* Print the text in FORMAT, using ARGV for arguments to any '%' directives. Return advanced ARGV. */ static char **print_formatted(char *f, char **argv) { char *direc_start; /* Start of % directive. */ unsigned direc_length; /* Length of % directive. */ - int field_width; /* Arg to first '*', or -1 if none. */ - int precision; /* Arg to second '*', or -1 if none. */ + int field_width; /* Arg to first '*' */ + int precision; /* Arg to second '*' */ char **saved_argv = argv; for (; *f; ++f) { @@ -201,7 +247,7 @@ static char **print_formatted(char *f, char **argv) case '%': direc_start = f++; direc_length = 1; - field_width = precision = -1; + field_width = precision = 0; if (*f == '%') { bb_putchar('%'); break; @@ -220,11 +266,8 @@ static char **print_formatted(char *f, char **argv) if (*f == '*') { ++f; ++direc_length; - if (*argv) { - field_width = my_xstrtoul(*argv); - ++argv; - } else - field_width = 0; + if (*argv) + field_width = get_width_prec(*argv++); } else { while (isdigit(*f)) { ++f; @@ -237,24 +280,22 @@ static char **print_formatted(char *f, char **argv) if (*f == '*') { ++f; ++direc_length; - if (*argv) { - precision = my_xstrtoul(*argv); - ++argv; - } else - precision = 0; - } else + if (*argv) + precision = get_width_prec(*argv++); + } else { while (isdigit(*f)) { ++f; ++direc_length; } + } } - if (*f == 'l' || *f == 'L' || *f == 'h') { + if ((*f | 0x20) == 'l' || *f == 'h' || *f == 'z') { ++f; ++direc_length; } /* needed - try "printf %" without it */ if (!strchr("diouxXfeEgGcs", *f)) { - bb_error_msg("invalid directive '%s'", direc_start); + bb_error_msg("%s: invalid format", direc_start); /* causes main() to exit with error */ return saved_argv - 1; } @@ -263,9 +304,11 @@ static char **print_formatted(char *f, char **argv) print_direc(direc_start, direc_length, field_width, precision, *argv); ++argv; - } else + } else { print_direc(direc_start, direc_length, field_width, precision, ""); + } + /* if (errno) return saved_argv - 1; */ break; case '\\': if (*++f == 'c') { diff --git a/testsuite/printf.tests b/testsuite/printf.tests index d6b60fb8c..a5c71ec9d 100755 --- a/testsuite/printf.tests +++ b/testsuite/printf.tests @@ -1,6 +1,6 @@ #!/bin/sh - -set -e +# Copyright 2008 by Denys Vlasenko +# Licensed under GPL v2, see file LICENSE for details. . testing.sh @@ -9,25 +9,95 @@ bb="busybox " # testing "test name" "command" "expected result" "file input" "stdin" -testing "printf produce no further output 1" \ +testing "printf produces no further output 1" \ "${bb}printf '\c' foo" \ "" \ "" "" -testing "printf produce no further output 2" \ - "${bb}printf '%s\c' foo \$HOME" \ +testing "printf produces no further output 2" \ + "${bb}printf '%s\c' foo bar" \ "foo" \ "" "" -testing "printf repeatedly use pattern for each argv" \ +testing "printf repeatedly uses pattern for each argv" \ "${bb}printf '%s\n' foo \$HOME" \ "foo\n$HOME\n" \ "" "" -# Why ()s are necessary I have no idea... +testing "printf understands %b escaped_string" \ + "${bb}printf '%b' 'a\tb' 'c\\d\n' 2>&1; echo \$?" \ + "a\tbc\\d\n""0\n" \ + "" "" + +testing "printf understands %d '\"x' \"'y\" \"'zTAIL\"" \ + "${bb}printf '%d\n' '\"x' \"'y\" \"'zTAIL\" 2>&1; echo \$?" \ + "120\n""121\n""122\n""0\n" \ + "" "" + +testing "printf understands %s '\"x' \"'y\" \"'zTAIL\"" \ + "${bb}printf '%s\n' '\"x' \"'y\" \"'zTAIL\" 2>&1; echo \$?" \ + "\"x\n""'y\n""'zTAIL\n""0\n" \ + "" "" + +testing "printf understands %23.12f" \ + "${bb}printf '|%23.12f|\n' 5.25 2>&1; echo \$?" \ + "| 5.250000000000|\n""0\n" \ + "" "" + +testing "printf understands %*.*f" \ + "${bb}printf '|%*.*f|\n' 23 12 5.25 2>&1; echo \$?" \ + "| 5.250000000000|\n""0\n" \ + "" "" + +testing "printf understands %*f with negative width" \ + "${bb}printf '|%*f|\n' -23 5.25 2>&1; echo \$?" \ + "|5.250000 |\n""0\n" \ + "" "" + +testing "printf understands %.*f with negative precision" \ + "${bb}printf '|%.*f|\n' -12 5.25 2>&1; echo \$?" \ + "|5.250000|\n""0\n" \ + "" "" + +testing "printf understands %*.*f with negative width/precision" \ + "${bb}printf '|%*.*f|\n' -23 -12 5.25 2>&1; echo \$?" \ + "|5.250000 |\n""0\n" \ + "" "" + +testing "printf understands %zd" \ + "${bb}printf '%zd\n' -5 2>&1; echo \$?" \ + "-5\n""0\n" \ + "" "" + +testing "printf understands %ld" \ + "${bb}printf '%ld\n' -5 2>&1; echo \$?" \ + "-5\n""0\n" \ + "" "" + +# We are "more correct" here than bash/coreutils: they happily print -2 +# as if it is a huge unsigned number +testing "printf handles %u -N" \ + "${bb}printf '%u\n' 1 -2 3 2>&1; echo \$?" \ + "1\n""printf: -2: invalid number\n""0\n""3\n""0\n" \ + "" "" + +# Actually, we are wrong here: exit code should be 1 +testing "printf handles %d bad_input" \ + "${bb}printf '%d\n' 1 - 2 bad 3 123bad 4 2>&1; echo \$?" \ +"1\n""printf: -: invalid number\n""0\n"\ +"2\n""printf: bad: invalid number\n""0\n"\ +"3\n""printf: 123bad: invalid number\n""0\n"\ +"4\n""0\n" \ + "" "" + testing "printf aborts on bare %" \ - "(${bb}printf '%' a b c) 2>&1; echo \$?" \ - "printf: invalid directive '%'\n""1\n" \ + "${bb}printf '%' a b c 2>&1; echo \$?" \ + "printf: %: invalid format\n""1\n" \ + "" "" + +testing "printf aborts on %r" \ + "${bb}printf '%r' a b c 2>&1; echo \$?" \ + "printf: %r: invalid format\n""1\n" \ "" "" exit $FAILCOUNT -- cgit v1.2.3