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
--- 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 @@
*/
p = arg + 1;
if (!*p) /* A single '-', so echo it. */
- goto just_echo;
+ break;
do {
if (!strrchr("neE", *p))
@@ -95,19 +91,27 @@
}
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 @@
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 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 @@
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 @@
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 @@
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; }