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>

7 files changed, 74 insertions, 40 deletions

diff --git a/coreutils/echo.c b/coreutils/echo.c
index 3821e594e..5fa3d10c5 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 000000000..3e91a13d3
--- /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 000000000..0a40c9ff7
--- /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 2a02d41ce..c1a6e729a 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 10788b8e7..e857e7464 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 000000000..ddcad4363
--- /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 000000000..0a40c9ff7
--- /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; }