← Commit list

8ee2adab2

Denys Vlasenko
Denys Vlasenko
2011-02-07
FAILED Duration: 221s

Subject

echo: do not retry on write errors

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

coreutils/echo.c +48 -39shell/ash_test/ash-misc/echo_write_error.right +2 -0shell/ash_test/ash-misc/echo_write_error.tests +7 -0shell/ash_test/ash-redir/redir.right +1 -0shell/hush.c +7 -1shell/hush_test/hush-misc/echo_write_error.right +2 -0shell/hush_test/hush-misc/echo_write_error.tests +7 -0
coreutils/echo.c+48 -39
@@ -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+2 -0
@@ -0,0 +1,2 @@
+ash: write error: Broken pipe
+Ok: 1
shell/ash_test/ash-misc/echo_write_error.tests+7 -0
@@ -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 -0
@@ -1 +1,2 @@
+ash: write error: Bad file descriptor
TEST
shell/hush.c+7 -1
@@ -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+2 -0
@@ -0,0 +1,2 @@
+hush: write error: Broken pipe
+Ok: 1
shell/hush_test/hush-misc/echo_write_error.tests+7 -0
@@ -0,0 +1,7 @@
+trap "" PIPE
+
+{
+sleep 1
+echo Cant write this - get EPIPE
+echo Ok: $? >&2
+} | { true; }