On Thursday 18 June 2009 17:52, Colin Watson wrote:
> This resolves a FIXME left over from commit
> a48656b441224a53d2bb3face920ba5487eaae09. SUSv3 says:
>
> If an argument operand cannot be completely converted into an internal
> value appropriate to the corresponding conversion specification, a
> diagnostic message shall be written to standard error and the utility
> shall not exit with a zero exit status, but shall continue processing
> any remaining operands and shall write the value accumulated at the
> time the error was detected to standard output.
>
> ... so we make sure to accumulate error statuses rather than just
> returning straight away.
>
> Signed-off-by: Colin Watson <[email protected]>
> @@ -69,7 +67,9 @@ static int multiconvert(const char *arg, void *result,
> converter convert)
> errno = 0;
> convert(arg, result);
> if (errno) {
> + int save_errno = errno;
> bb_error_msg("%s: invalid number", arg);
> + errno = save_errno;
> return 1;
> }
Why?
> static void print_direc(char *format, unsigned fmt_length,
> int field_width, int precision,
> - const char *argument)
> + const char *argument, int *status)
We can just observe errno != 0 on return, no need to have int *status.
(BTW, if it would be needed, it's better to return it as a return value,
not a pointer param...)
Generally, good idea.
I propose this modification.
--
vda
diff -d -urpN busybox.6/coreutils/printf.c busybox.7/coreutils/printf.c
--- busybox.6/coreutils/printf.c 2009-06-17 23:00:02.000000000 +0200
+++ busybox.7/coreutils/printf.c 2009-06-18 22:16:10.000000000 +0200
@@ -53,10 +53,7 @@
* (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.
+ * We try to be compatible.
*/
typedef void FAST_FUNC (*converter)(const char *arg, void *result);
@@ -163,7 +160,6 @@ static void print_direc(char *format, un
case 'i':
llv = my_xstrtoll(argument);
print_long:
- /* if (errno) return; - see comment at the top */
if (!have_width) {
if (!have_prec)
printf(format, llv);
@@ -210,7 +206,6 @@ static void print_direc(char *format, un
case 'g':
case 'G':
dv = my_xstrtod(argument);
- /* if (errno) return; */
if (!have_width) {
if (!have_prec)
printf(format, dv);
@@ -241,7 +236,7 @@ static int get_width_prec(const char *st
/* Print the text in FORMAT, using ARGV for arguments to any '%' directives.
Return advanced ARGV. */
-static char **print_formatted(char *f, char **argv)
+static char **print_formatted(char *f, char **argv, int *conv_err)
{
char *direc_start; /* Start of % directive. */
unsigned direc_length; /* Length of % directive. */
@@ -299,8 +294,8 @@ static char **print_formatted(char *f, c
/* Remove "lLhz" size modifiers, repeatedly.
* bash does not like "%lld", but coreutils
- * would happily take even "%Llllhhzhhzd"!
- * We will be permissive like coreutils */
+ * happily takes even "%Llllhhzhhzd"!
+ * We are permissive like coreutils */
while ((*f | 0x20) == 'l' || *f == 'h' || *f == 'z') {
overlapping_strcpy(f, f + 1);
}
@@ -330,12 +325,12 @@ static char **print_formatted(char *f, c
}
if (*argv) {
print_direc(direc_start, direc_length, field_width,
- precision, *argv);
- ++argv;
+ precision, *argv++);
} else {
print_direc(direc_start, direc_length, field_width,
precision, "");
}
+ *conv_err |= errno;
free(p);
}
break;
@@ -356,6 +351,7 @@ static char **print_formatted(char *f, c
int printf_main(int argc UNUSED_PARAM, char **argv)
{
+ int conv_err;
char *format;
char **argv2;
@@ -392,9 +388,10 @@ int printf_main(int argc UNUSED_PARAM, c
format = argv[1];
argv2 = argv + 2;
+ conv_err = 0;
do {
argv = argv2;
- argv2 = print_formatted(format, argv);
+ argv2 = print_formatted(format, argv, &conv_err);
} while (argv2 > argv && *argv2);
/* coreutils compat (bash doesn't do this):
@@ -402,5 +399,6 @@ int printf_main(int argc UNUSED_PARAM, c
fprintf(stderr, "excess args ignored");
*/
- return (argv2 < argv); /* if true, print_formatted errored out */
+ return (argv2 < argv) /* if true, print_formatted errored out */
+ || conv_err; /* print_formatted saw invalid number */
}
diff -d -urpN busybox.6/testsuite/printf.tests busybox.7/testsuite/printf.tests
--- busybox.6/testsuite/printf.tests 2009-06-17 23:00:05.000000000 +0200
+++ busybox.7/testsuite/printf.tests 2009-06-18 22:18:30.000000000 +0200
@@ -87,13 +87,12 @@ testing "printf understands %Ld" \
# "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" \
+"4\n""1\n" \
"" ""
testing "printf aborts on bare %" \
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
