Re: [bug-gawk] Behavior of fflush with SIGPIPE on stdout [PATCH]

[Arnold Robbins]

Hi All.

Thanks for the reports and discussions. I tackled the problem with the
patch below. I plan to commit it soon.  Andy - please review.

Thanks,

Arnold
-----------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index a0f26b2..9b41801 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2017-03-17         Arnold D. Robbins     <address@hidden>
+
+       * awk.h (ignore_sigpipe, set_sigpipe_to_default,
+       non_fatal_flush_std): Declare new functions.
+       * builtin.c (do_fflush): When nonfatal not in force, flush
+       of stdout/stderr and EPIPE exits, simulating SIGPIPE, as
+       in nawk/mawk. Flush of other redirections with EPIPE now
+       also fatals.
+       (do_system): Use ignore_sipipe and set_sigpipe_to_default
+       instead of uglier inline ifdefed code.
+       * main.c (main): Ditto.
+       * io.c (redirect_string, two_way_open, gawk_popen): Ditto.
+       (ignore_sigpipe, set_sigpipe_to_default,
+       non_fatal_flush_std): New functions.
+       (flush_io): Use non_fatal_flush_std for stdout and stderr.
+
 2017-03-16         Arnold D. Robbins     <address@hidden>
 
        * configure.ac: Some cleanups.
diff --git a/awk.h b/awk.h
index aae5e7e..2a01838 100644
--- a/awk.h
+++ b/awk.h
@@ -1584,6 +1584,10 @@ extern bool inrec(IOBUF *iop, int *errcode);
 extern int nextfile(IOBUF **curfile, bool skipping);
 extern bool is_non_fatal_std(FILE *fp);
 extern bool is_non_fatal_redirect(const char *str, size_t len);
+extern void ignore_sigpipe(void);
+extern void set_sigpipe_to_default(void);
+extern bool non_fatal_flush_std_file(FILE *fp);
+
 /* main.c */
 extern int arg_assign(char *arg, bool initing);
 extern int is_std_var(const char *var);
diff --git a/builtin.c b/builtin.c
index 2acd992..3bdd14c 100644
--- a/builtin.c
+++ b/builtin.c
@@ -133,7 +133,6 @@ wrerror:
        if (fp == stdout && errno == EPIPE)
                gawk_exit(EXIT_FATAL);
 
-
        /* otherwise die verbosely */
        if ((rp != NULL) ? is_non_fatal_redirect(rp->value, strlen(rp->value)) 
: is_non_fatal_std(fp))
                update_ERRNO_int(errno);
@@ -245,13 +244,18 @@ do_fflush(int nargs)
                        return make_number((AWKNUM) status);
                }
                fp = rp->output.fp;
-               if (fp != NULL)
+               if (fp != NULL) {
+                       bool is_fatal = ! is_non_fatal_redirect(tmp->stptr, 
tmp->stlen);
+
                        status = rp->output.gawk_fflush(fp, rp->output.opaque);
-               else if ((rp->flag & RED_TWOWAY) != 0)
+                       if (is_fatal && status != 0)
+                               fatal(_("fflush: cannot flush file `%.*s': %s"),
+                                               len, file, strerror(errno));
+               } else if ((rp->flag & RED_TWOWAY) != 0)
                                warning(_("fflush: cannot flush: two-way pipe 
`%.*s' has closed write end"),
                                        len, file);
        } else if ((fp = stdfile(tmp->stptr, tmp->stlen)) != NULL) {
-               status = fflush(fp);
+               status = (non_fatal_flush_std_file(fp) == false);
        } else {
                status = -1;
                warning(_("fflush: `%.*s' is not an open file, pipe or 
co-process"), len, file);
@@ -2148,9 +2152,7 @@ do_system(int nargs)
                cmd[tmp->stlen] = '\0';
 
                os_restore_mode(fileno(stdin));
-#ifdef SIGPIPE
-               signal(SIGPIPE, SIG_DFL);
-#endif
+               set_sigpipe_to_default();
 
                status = system(cmd);
                /*
@@ -2176,9 +2178,7 @@ do_system(int nargs)
 
                if ((BINMODE & BINMODE_INPUT) != 0)
                        os_setbinmode(fileno(stdin), O_BINARY);
-#ifdef SIGPIPE
-               signal(SIGPIPE, SIG_IGN);
-#endif
+               ignore_sigpipe();
 
                cmd[tmp->stlen] = save;
        }
diff --git a/io.c b/io.c
index cced126..e211e31 100644
--- a/io.c
+++ b/io.c
@@ -899,9 +899,7 @@ redirect_string(const char *str, size_t explen, bool 
not_string,
                        (void) flush_io();
 
                        os_restore_mode(fileno(stdin));
-#ifdef SIGPIPE
-                       signal(SIGPIPE, SIG_DFL);
-#endif
+                       set_sigpipe_to_default();
                        /*
                         * Don't check failure_fatal; see input pipe below.
                         * Note that the failure happens upon failure to fork,
@@ -911,9 +909,7 @@ redirect_string(const char *str, size_t explen, bool 
not_string,
                        if ((rp->output.fp = popen(str, binmode("w"))) == NULL)
                                fatal(_("can't open pipe `%s' for output (%s)"),
                                                str, strerror(errno));
-#ifdef SIGPIPE
-                       signal(SIGPIPE, SIG_IGN);
-#endif
+                       ignore_sigpipe();
 
                        /* set close-on-exec */
                        os_close_on_exec(fileno(rp->output.fp), str, "pipe", 
"to");
@@ -1392,6 +1388,34 @@ checkwarn:
        return status;
 }
 
+/* non_fatal_flush_std_file --- flush a standard output file allowing for 
nonfatal setting */
+
+bool
+non_fatal_flush_std_file(FILE *fp)
+{
+       bool is_fatal = ! is_non_fatal_std(fp);
+
+       int status = fflush(fp);
+       if (is_fatal && status != 0) {
+               if (errno == EPIPE)
+                       exit(EXIT_SUCCESS);     // simulate SIGPIPE
+               else
+                       fatal(fp == stdout
+                               ? _("fflush: cannot flush standard output: %s")
+                               : _("fflush: cannot flush standard error: %s"),
+                               strerror(errno));
+               return false;
+       } else if (status != 0) {
+               warning(fp == stdout
+                       ? _("error writing standard output (%s)")
+                       : _("error writing standard error (%s)"),
+                               strerror(errno));
+               return false;
+       }
+
+       return true;
+}
+
 /* flush_io --- flush all open output files */
 
 int
@@ -1401,17 +1425,12 @@ flush_io()
        int status = 0;
 
        errno = 0;
-       /* we don't warn about stdout/stderr if EPIPE, but we do error exit */
-       if (fflush(stdout)) {
-               if (errno != EPIPE)
-                       warning(_("error writing standard output (%s)"), 
strerror(errno));
+       if (! non_fatal_flush_std_file(stdout))
                status++;
-       }
-       if (fflush(stderr)) {
-               if (errno != EPIPE)
-                       warning(_("error writing standard error (%s)"), 
strerror(errno));
+       if (! non_fatal_flush_std_file(stderr))
                status++;
-       }
+
+       // now for all open redirections
        for (rp = red_head; rp != NULL; rp = rp->next) {
                /* flush both files and pipes, what the heck */
                if ((rp->flag & RED_WRITE) != 0 && rp->output.fp != NULL) {
@@ -2076,7 +2095,7 @@ two_way_open(const char *str, struct redirect *rp, int 
extfd)
 
                        /* stderr does NOT get dup'ed onto child's stdout */
 
-                       signal(SIGPIPE, SIG_DFL);
+                       set_sigpipe_to_default();
 
                        execl("/bin/sh", "sh", "-c", str, NULL);
                        _exit(errno == ENOENT ? 127 : 126);
@@ -2253,7 +2272,7 @@ use_pipes:
                    || close(ctop[0]) == -1 || close(ctop[1]) == -1)
                        fatal(_("close of pipe failed (%s)"), strerror(errno));
                /* stderr does NOT get dup'ed onto child's stdout */
-               signal(SIGPIPE, SIG_DFL);
+               set_sigpipe_to_default();
                execl("/bin/sh", "sh", "-c", str, NULL);
                _exit(errno == ENOENT ? 127 : 126);
        }
@@ -2490,7 +2509,7 @@ gawk_popen(const char *cmd, struct redirect *rp)
                        fatal(_("moving pipe to stdout in child failed (dup: 
%s)"), strerror(errno));
                if (close(p[0]) == -1 || close(p[1]) == -1)
                        fatal(_("close of pipe failed (%s)"), strerror(errno));
-               signal(SIGPIPE, SIG_DFL);
+               set_sigpipe_to_default();
                execl("/bin/sh", "sh", "-c", cmd, NULL);
                _exit(errno == ENOENT ? 127 : 126);
        }
@@ -2555,17 +2574,13 @@ gawk_popen(const char *cmd, struct redirect *rp)
        FILE *current;
 
        os_restore_mode(fileno(stdin));
-#ifdef SIGPIPE
-       signal(SIGPIPE, SIG_DFL);
-#endif
+       set_sigpipe_to_default();
 
        current = popen(cmd, binmode("r"));
 
        if ((BINMODE & BINMODE_INPUT) != 0)
                os_setbinmode(fileno(stdin), O_BINARY);
-#ifdef SIGPIPE
-       signal(SIGPIPE, SIG_IGN);
-#endif
+       ignore_sigpipe();
 
        if (current == NULL)
                return NULL;
@@ -4261,3 +4276,23 @@ init_output_wrapper(awk_output_buf_t *outbuf)
        outbuf->gawk_ferror = gawk_ferror;
        outbuf->gawk_fclose = gawk_fclose;
 }
+
+/* ignore_sigpipe --- ignore SIGPIPE */
+
+void
+ignore_sigpipe()
+{
+#ifdef SIGPIPE
+       signal(SIGPIPE, SIG_IGN);
+#endif
+}
+
+/* set_sigpipe_to_default --- set SIGPIPE to default */
+
+void
+set_sigpipe_to_default()
+{
+#ifdef SIGPIPE
+       signal(SIGPIPE, SIG_DFL);
+#endif
+}
diff --git a/main.c b/main.c
index 56482f5..4f578d3 100644
--- a/main.c
+++ b/main.c
@@ -255,7 +255,7 @@ main(int argc, char **argv)
 #ifdef SIGBUS
        (void) signal(SIGBUS, catchsig);
 #endif
-#ifdef SIGPIPE
+
        /*
         * Ignore SIGPIPE so that writes to pipes that fail don't
         * kill the process but instead return -1 and set errno.
@@ -269,8 +269,7 @@ main(int argc, char **argv)
         * should not give us "broken pipe" messages --- mainly because
         * it did not do so in the past and people would complain.
         */
-       signal(SIGPIPE, SIG_IGN);
-#endif
+       ignore_sigpipe();
 
        (void) sigsegv_install_handler(catchsegv);
 #define STACK_SIZE (16*1024)