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 <vda.linux@googlemail.com>

diff --git a/coreutils/echo.c b/coreutils/echo.c
index 3821e59..5fa3d10 100644 (file)
--- 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 (file)
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 (file)
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 (file)
--- 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 (file)
--- 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 (file)
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 (executable)
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; }