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>

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;
 }
 
 /*-

shell/ash_test/ash-misc/echo_write_error.right

@@ -0,0 +1,2 @@
+ash: write error: Broken pipe
+Ok: 1

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; }

shell/ash_test/ash-redir/redir.right

@@ -1 +1,2 @@
+ash: write error: Bad file descriptor
 TEST

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;

shell/hush_test/hush-misc/echo_write_error.right

@@ -0,0 +1,2 @@
+hush: write error: Broken pipe
+Ok: 1

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; }