From: Denys Vlasenko Date: Mon, 7 Feb 2011 01:03:51 +0000 (+0100) Subject: echo: do not retry on write errors X-Git-Tag: 1_19_0~335 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8ee2adab21328761b80e0cbc513eda7eaa880b24;p=platform%2Fupstream%2Fbusybox.git echo: do not retry on write errors function old new delta echo_main 297 336 +39 stpcpy - 22 +22 run_pipe 1561 1566 +5 pseudo_exec_argv 187 192 +5 hush_exit 75 80 +5 ------------------------------------------------------------------------------ (add/remove: 3/0 grow/shrink: 4/0 up/down: 98/0) Total: 76 bytes Signed-off-by: Denys Vlasenko --- diff --git a/coreutils/echo.c b/coreutils/echo.c index 3821e59..5fa3d10 100644 --- a/coreutils/echo.c +++ b/coreutils/echo.c @@ -29,46 +29,42 @@ /* NB: can be used by shell even if not enabled as applet */ +/* + * NB2: we don't use stdio, we need better error handing. + * Examples include writing into non-opened stdout and error on write. + * + * With stdio, output gets shoveled into stdout buffer, and even + * fflush cannot clear it out. It seems that even if libc receives + * EBADF on write attempts, it feels determined to output data no matter what. + * If echo is called by shell, it will try writing again later, and possibly + * will clobber future output. Not good. + * + * Solaris has fpurge which discards buffered input. glibc has __fpurge. + * But this function is not standard. + */ + int echo_main(int argc UNUSED_PARAM, char **argv) { + char **pp; const char *arg; + char *out; + char *buffer; + unsigned buflen; + int r; #if !ENABLE_FEATURE_FANCY_ECHO enum { eflag = '\\', nflag = 1, /* 1 -- print '\n' */ }; - /* We must check that stdout is not closed. - * The reason for this is highly non-obvious. - * echo_main is used from shell. Shell must correctly handle "echo foo" - * if stdout is closed. With stdio, output gets shoveled into - * stdout buffer, and even fflush cannot clear it out. It seems that - * even if libc receives EBADF on write attempts, it feels determined - * to output data no matter what. So it will try later, - * and possibly will clobber future output. Not good. */ -// TODO: check fcntl() & O_ACCMODE == O_WRONLY or O_RDWR? - if (fcntl(1, F_GETFL) == -1) - return 1; /* match coreutils 6.10 (sans error msg to stderr) */ - //if (dup2(1, 1) != 1) - old way - // return 1; - - arg = *++argv; - if (!arg) - goto newline_ret; + argv++; #else const char *p; char nflag = 1; char eflag = 0; - /* We must check that stdout is not closed. */ - if (fcntl(1, F_GETFL) == -1) - return 1; - - while (1) { - arg = *++argv; - if (!arg) - goto newline_ret; - if (*arg != '-') + while ((arg = *++argv) != NULL) { + if (!arg || arg[0] != '-') break; /* If it appears that we are handling options, then make sure @@ -77,7 +73,7 @@ int echo_main(int argc UNUSED_PARAM, char **argv) */ p = arg + 1; if (!*p) /* A single '-', so echo it. */ - goto just_echo; + break; do { if (!strrchr("neE", *p)) @@ -95,19 +91,27 @@ int echo_main(int argc UNUSED_PARAM, char **argv) } just_echo: #endif - while (1) { - /* arg is already == *argv and isn't NULL */ + + buflen = 1; + pp = argv; + while ((arg = *pp) != NULL) { + buflen += strlen(arg) + 1; + pp++; + } + out = buffer = xmalloc(buflen); + + while ((arg = *argv) != NULL) { int c; if (!eflag) { /* optimization for very common case */ - fputs(arg, stdout); + out = stpcpy(out, arg); } else while ((c = *arg++)) { if (c == eflag) { /* Check for escape seq. */ if (*arg == 'c') { /* '\c' means cancel newline and * ignore all subsequent chars. */ - goto ret; + goto do_write; } #if !ENABLE_FEATURE_FANCY_ECHO /* SUSv3 specifies that octal escapes must begin with '0'. */ @@ -127,21 +131,26 @@ int echo_main(int argc UNUSED_PARAM, char **argv) c = bb_process_escape_sequence(&arg); } } - bb_putchar(c); + *out++ = c; } - arg = *++argv; - if (!arg) + if (!*++argv) break; - bb_putchar(' '); + *out++ = ' '; } - newline_ret: if (nflag) { - bb_putchar('\n'); + *out++ = '\n'; } - ret: - return fflush_all(); + + do_write: + r = full_write(STDOUT_FILENO, buffer, out - buffer); + free(buffer); + if (r < 0) { + bb_perror_msg(bb_msg_write_error); + return 1; + } + return 0; } /*- diff --git a/shell/ash_test/ash-misc/echo_write_error.right b/shell/ash_test/ash-misc/echo_write_error.right new file mode 100644 index 0000000..3e91a13 --- /dev/null +++ b/shell/ash_test/ash-misc/echo_write_error.right @@ -0,0 +1,2 @@ +ash: write error: Broken pipe +Ok: 1 diff --git a/shell/ash_test/ash-misc/echo_write_error.tests b/shell/ash_test/ash-misc/echo_write_error.tests new file mode 100644 index 0000000..0a40c9f --- /dev/null +++ b/shell/ash_test/ash-misc/echo_write_error.tests @@ -0,0 +1,7 @@ +trap "" PIPE + +{ +sleep 1 +echo Cant write this - get EPIPE +echo Ok: $? >&2 +} | { true; } diff --git a/shell/ash_test/ash-redir/redir.right b/shell/ash_test/ash-redir/redir.right index 2a02d41..c1a6e72 100644 --- a/shell/ash_test/ash-redir/redir.right +++ b/shell/ash_test/ash-redir/redir.right @@ -1 +1,2 @@ +ash: write error: Bad file descriptor TEST diff --git a/shell/hush.c b/shell/hush.c index 10788b8..e857e74 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1418,6 +1418,7 @@ static void sigexit(int sig) static void hush_exit(int exitcode) NORETURN; static void hush_exit(int exitcode) { + fflush_all(); if (G.exiting <= 0 && G.traps && G.traps[0] && G.traps[0][0]) { /* Prevent recursion: * trap "echo Hi; exit" EXIT; exit @@ -6105,10 +6106,13 @@ static void exec_builtin(char ***to_free, char **argv) { #if BB_MMU - int rcode = x->b_function(argv); + int rcode; + fflush_all(); + rcode = x->b_function(argv); fflush_all(); _exit(rcode); #else + fflush_all(); /* On NOMMU, we must never block! * Example: { sleep 99 | read line; } & echo Ok */ @@ -6832,6 +6836,7 @@ static NOINLINE int run_pipe(struct pipe *pi) if (!funcp) { debug_printf_exec(": builtin '%s' '%s'...\n", x->b_cmd, argv_expanded[1]); + fflush_all(); rcode = x->b_function(argv_expanded) & 0xff; fflush_all(); } @@ -7641,6 +7646,7 @@ int hush_main(int argc, char **argv) G.global_argc -= builtin_argc; /* skip [BARGV...] "" */ G.global_argv += builtin_argc; G.global_argv[-1] = NULL; /* replace "" */ + fflush_all(); G.last_exitcode = x->b_function(argv + optind - 1); } goto final_return; diff --git a/shell/hush_test/hush-misc/echo_write_error.right b/shell/hush_test/hush-misc/echo_write_error.right new file mode 100644 index 0000000..ddcad43 --- /dev/null +++ b/shell/hush_test/hush-misc/echo_write_error.right @@ -0,0 +1,2 @@ +hush: write error: Broken pipe +Ok: 1 diff --git a/shell/hush_test/hush-misc/echo_write_error.tests b/shell/hush_test/hush-misc/echo_write_error.tests new file mode 100755 index 0000000..0a40c9f --- /dev/null +++ b/shell/hush_test/hush-misc/echo_write_error.tests @@ -0,0 +1,7 @@ +trap "" PIPE + +{ +sleep 1 +echo Cant write this - get EPIPE +echo Ok: $? >&2 +} | { true; }