Clean up inconsistent use of fflush().

More than twenty years ago (79fcde48b), we hacked the postmaster
to avoid a core-dump on systems that didn't support fflush(NULL).
We've mostly, though not completely, hewed to that rule ever since.
But such systems are surely gone in the wild, so in the spirit of
cleaning out no-longer-needed portability hacks let's get rid of
multiple per-file fflush() calls in favor of using fflush(NULL).

Also, we were fairly inconsistent about whether to fflush() before
popen() and system() calls.  While we've received no bug reports
about that, it seems likely that at least some of these call sites
are at risk of odd behavior, such as error messages appearing in
an unexpected order.  Rather than expend a lot of brain cells
figuring out which places are at hazard, let's just establish a
uniform coding rule that we should fflush(NULL) before these calls.
A no-op fflush() is surely of trivial cost compared to launching
a sub-process via a shell; while if it's not a no-op then we likely
need it.

Discussion: https://postgr.es/m/2923412.1661722825@sss.pgh.pa.us

diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index 4101a30e374ab3ea420d69142b0169e9630772a2..e2b7176f2f63b141ef7723727514a349bf799e78 100644 (file)
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -169,6 +169,7 @@ RestoreArchivedFile(char *path, const char *xlogfname,
        /*
         * Copy xlog from archival storage to XLOGDIR
         */
+       fflush(NULL);
        pgstat_report_wait_start(WAIT_EVENT_RESTORE_COMMAND);
        rc = system(xlogRestoreCmd);
        pgstat_report_wait_end();
@@ -358,6 +359,7 @@ ExecuteRecoveryCommand(const char *command, const char *commandName,
        /*
         * execute the constructed command
         */
+       fflush(NULL);
        pgstat_report_wait_start(wait_event_info);
        rc = system(xlogRecoveryCmd);
        pgstat_report_wait_end();

diff --git a/src/backend/postmaster/fork_process.c b/src/backend/postmaster/fork_process.c
index c75be03d2c3fed912e73ac5ef8e6d36ef6a911b9..ec677614870f5c0ec167532a0c1f826bb0eec948 100644 (file)
--- a/src/backend/postmaster/fork_process.c
+++ b/src/backend/postmaster/fork_process.c
@@ -37,13 +37,8 @@ fork_process(void)
 
        /*
         * Flush stdio channels just before fork, to avoid double-output problems.
-        * Ideally we'd use fflush(NULL) here, but there are still a few non-ANSI
-        * stdio libraries out there (like SunOS 4.1.x) that coredump if we do.
-        * Presently stdout and stderr are the only stdio output channels used by
-        * the postmaster, so fflush'ing them should be sufficient.
         */
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
 
 #ifdef LINUX_PROFILE
 

diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 19e240c2053bc84caa8bb66127675ed10682fbac..8a54d02e7bbd82eaed949518c562623ae40814ef 100644 (file)
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -99,6 +99,7 @@ shell_archive_file(const char *file, const char *path)
                        (errmsg_internal("executing archive command \"%s\"",
                                                         xlogarchcmd)));
 
+       fflush(NULL);
        pgstat_report_wait_start(WAIT_EVENT_ARCHIVE_COMMAND);
        rc = system(xlogarchcmd);
        pgstat_report_wait_end();

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index e3b19ca1ed4a7e77ee7f99b9ac622e82ac376e68..1aaab6c5542caffc619f0f6c606fcab4c40f56f0 100644 (file)
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -2503,8 +2503,7 @@ OpenPipeStream(const char *command, const char *mode)
        ReleaseLruFiles();
 
 TryAgain:
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
        pqsignal(SIGPIPE, SIG_DFL);
        errno = 0;
        file = popen(command, mode);

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 95f32de4e29c6b9bd79a52e9f1b677db5de18743..cb3c28988980a0f457d1dcdce4f4684713046196 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -643,8 +643,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
                 * Any other code you might be tempted to add here should probably be
                 * in an on_proc_exit or on_shmem_exit callback instead.
                 */
-               fflush(stdout);
-               fflush(stderr);
+               fflush(NULL);
 
                /*
                 * Let the cumulative stats system know. Only mark the session as
@@ -670,8 +669,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
                 * XXX: what if we are *in* the postmaster?  abort() won't kill our
                 * children...
                 */
-               fflush(stdout);
-               fflush(stderr);
+               fflush(NULL);
                abort();
        }
 

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 29c28b73153ca591e9dc2a1e755e75ebd94b57eb..e00837ecacf4885cf2a176168c283f3e67c6eb53 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -489,8 +489,7 @@ popen_check(const char *command, const char *mode)
 {
        FILE       *cmdfd;
 
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
        errno = 0;
        cmdfd = popen(command, mode);
        if (cmdfd == NULL)
@@ -914,6 +913,7 @@ test_config_settings(void)
                                 test_conns, test_buffs,
                                 dynamic_shared_memory_type,
                                 DEVNULL, DEVNULL);
+               fflush(NULL);
                status = system(cmd);
                if (status == 0)
                {
@@ -950,6 +950,7 @@ test_config_settings(void)
                                 n_connections, test_buffs,
                                 dynamic_shared_memory_type,
                                 DEVNULL, DEVNULL);
+               fflush(NULL);
                status = system(cmd);
                if (status == 0)
                        break;

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 73e20081d1dafc38d348b5b1165235e033f20925..d7ff3902aa7301f95e5060039fa3f7ff578349a2 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -448,8 +448,7 @@ start_postmaster(void)
        pgpid_t         pm_pid;
 
        /* Flush stdio channels just before fork, to avoid double-output problems */
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
 
 #ifdef EXEC_BACKEND
        pg_disable_aslr();
@@ -916,6 +915,7 @@ do_init(void)
                cmd = psprintf("\"%s\" %s%s > \"%s\"",
                                           exec_path, pgdata_opt, post_opts, DEVNULL);
 
+       fflush(NULL);
        if (system(cmd) != 0)
        {
                write_stderr(_("%s: database system initialization failed\n"), progname);
@@ -2222,6 +2222,7 @@ adjust_data_dir(void)
                                   my_exec_path,
                                   pgdata_opt ? pgdata_opt : "",
                                   post_opts ? post_opts : "");
+       fflush(NULL);
 
        fd = popen(cmd, "r");
        if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL)

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 68c455f84b22df38c4442723a2783c644697d1f2..d665b257c93589c61ac6a3cb410928a0a0b2f4b5 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1578,8 +1578,7 @@ runPgDump(const char *dbname, const char *create_opts)
 
        pg_log_info("running \"%s\"", cmd->data);
 
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
 
        ret = system(cmd->data);
 

diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 1ff8da1676481fa39737a3c6e305f35e822f4e74..3cd77c09b1a593904d26876a3aaf6b30334f74d8 100644 (file)
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -1151,6 +1151,7 @@ ensureCleanShutdown(const char *argv0)
        appendPQExpBufferStr(postgres_cmd, " template1 < ");
        appendShellString(postgres_cmd, DEVNULL);
 
+       fflush(NULL);
        if (system(postgres_cmd->data) != 0)
        {
                pg_log_error("postgres single-user mode in target cluster failed");

diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index e2086a07dea4b250d7e8ddc95b76526d1ba5a293..018cd310f7c8b3d380c2389d98c86701058b8c33 100644 (file)
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -123,8 +123,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
                /* only pg_controldata outputs the cluster state */
                snprintf(cmd, sizeof(cmd), "\"%s/pg_controldata\" \"%s\"",
                                 cluster->bindir, cluster->pgdata);
-               fflush(stdout);
-               fflush(stderr);
+               fflush(NULL);
 
                if ((output = popen(cmd, "r")) == NULL)
                        pg_fatal("could not get control data using %s: %s",
@@ -191,8 +190,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
                         cluster->bindir,
                         live_check ? "pg_controldata\"" : resetwal_bin,
                         cluster->pgdata);
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
 
        if ((output = popen(cmd, "r")) == NULL)
                pg_fatal("could not get control data using %s: %s",

diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index aa3d2f88ed840bf4052d37c30800dddc3651ad21..60f4b5443e68dd2930d425c74522c4a43934fefb 100644 (file)
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -39,6 +39,7 @@ get_bin_version(ClusterInfo *cluster)
                                v2 = 0;
 
        snprintf(cmd, sizeof(cmd), "\"%s/pg_ctl\" --version", cluster->bindir);
+       fflush(NULL);
 
        if ((output = popen(cmd, "r")) == NULL ||
                fgets(cmd_output, sizeof(cmd_output), output) == NULL)
@@ -125,7 +126,10 @@ exec_prog(const char *log_filename, const char *opt_log_file,
         * the file do not see to help.
         */
        if (mainThreadId != GetCurrentThreadId())
+       {
+               fflush(NULL);
                result = system(cmd);
+       }
 #endif
 
        log = fopen(log_file, "a");
@@ -174,7 +178,10 @@ exec_prog(const char *log_filename, const char *opt_log_file,
        /* see comment above */
        if (mainThreadId == GetCurrentThreadId())
 #endif
+       {
+               fflush(NULL);
                result = system(cmd);
+       }
 
        if (result != 0 && report_error)
        {

diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index fbab1c4fb72c4924417fa7b8819675acc3fe7492..3f719f1762c591aefdea5800af51df05f8ddcbef 100644 (file)
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -416,6 +416,7 @@ adjust_data_dir(ClusterInfo *cluster)
         */
        snprintf(cmd, sizeof(cmd), "\"%s/postgres\" -D \"%s\" -C data_directory",
                         cluster->bindir, cluster->pgconfig);
+       fflush(NULL);
 
        if ((output = popen(cmd, "r")) == NULL ||
                fgets(cmd_output, sizeof(cmd_output), output) == NULL)

diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index bd18b4491d5dda981bf6634616ea2bc6fabae14c..63d02719a610bcc1b732768886194f6b392a97ac 100644 (file)
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -811,6 +811,7 @@ parse_required_wal(verifier_context *context, char *pg_waldump_path,
                                                                  pg_waldump_path, wal_directory, this_wal_range->tli,
                                                                  LSN_FORMAT_ARGS(this_wal_range->start_lsn),
                                                                  LSN_FORMAT_ARGS(this_wal_range->end_lsn));
+               fflush(NULL);
                if (system(pg_waldump_cmd) != 0)
                        report_backup_error(context,
                                                                "WAL parsing failed for timeline %u",

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 07b7e0cf37c687417111c8877ef3b5e3c4918f03..9aadcaad71e25793feb3bf4fcd34ff6454649a26 100644 (file)
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -2973,6 +2973,8 @@ runShellCommand(Variables *variables, char *variable, char **argv, int argc)
 
        command[len] = '\0';
 
+       fflush(NULL);                           /* needed before either system() or popen() */
+
        /* Fast path for non-assignment case */
        if (variable == NULL)
        {

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a81bd3307b47fd80f7817b42cd5cd97fbc5e9750..61188d96f2a67ff94000f4e0184be96af48ec3ab 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2661,6 +2661,7 @@ exec_command_write(PsqlScanState scan_state, bool active_branch,
                                if (fname[0] == '|')
                                {
                                        is_pipe = true;
+                                       fflush(NULL);
                                        disable_sigpipe_trap();
                                        fd = popen(&fname[1], "w");
                                }
@@ -3834,6 +3835,7 @@ editFile(const char *fname, int lineno)
                sys = psprintf("\"%s\" \"%s\"",
                                           editorName, fname);
 #endif
+       fflush(NULL);
        result = system(sys);
        if (result == -1)
                pg_log_error("could not start editor \"%s\"", editorName);
@@ -4956,6 +4958,7 @@ do_shell(const char *command)
 {
        int                     result;
 
+       fflush(NULL);
        if (!command)
        {
                char       *sys;
@@ -5065,6 +5068,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
 #endif
        if (pagerprog && myopt.topt.pager)
        {
+               fflush(NULL);
                disable_sigpipe_trap();
                pagerpipe = popen(pagerprog, "w");
 

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 9f95869eca6bc07c2973c8736570dd86954748ca..e611e3266d0c8cd8f69803cb0c06bcaca5318e68 100644 (file)
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -59,6 +59,7 @@ openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe)
        }
        else if (*fname == '|')
        {
+               fflush(NULL);
                *fout = popen(fname + 1, "w");
                *is_pipe = true;
        }

diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index c181682a1326292e157d5aac7c7f7908fd8dea97..0f66ebc2edf269507b442ceb66e2ef4f6150438e 100644 (file)
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -288,8 +288,7 @@ do_copy(const char *args)
                {
                        if (options->program)
                        {
-                               fflush(stdout);
-                               fflush(stderr);
+                               fflush(NULL);
                                errno = 0;
                                copystream = popen(options->file, PG_BINARY_R);
                        }
@@ -307,10 +306,9 @@ do_copy(const char *args)
                {
                        if (options->program)
                        {
-                               fflush(stdout);
-                               fflush(stderr);
-                               errno = 0;
+                               fflush(NULL);
                                disable_sigpipe_trap();
+                               errno = 0;
                                copystream = popen(options->file, PG_BINARY_W);
                        }
                        else

diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 509e6422b7eb1a76bf91ac88c6efa35b4c6bdccc..3955d72206c2fd878dba5826ca3dae5581ddf539 100644 (file)
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -266,8 +266,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
                                        {
                                                int                     cmdend = strcspn(p + 1, "`");
                                                char       *file = pnstrdup(p + 1, cmdend);
-                                               FILE       *fd = popen(file, "r");
+                                               FILE       *fd;
 
+                                               fflush(NULL);
+                                               fd = popen(file, "r");
                                                if (fd)
                                                {
                                                        if (fgets(buf, sizeof(buf), fd) == NULL)

diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 878da9f48e972dafc82fd4fe0b737a5416ec9b61..a467b72144915fe11dcf6b35cca65d2165fc7a88 100644 (file)
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -777,6 +777,7 @@ evaluate_backtick(PsqlScanState state)
 
        initPQExpBuffer(&cmd_output);
 
+       fflush(NULL);
        fd = popen(cmd, "r");
        if (!fd)
        {

diff --git a/src/common/exec.c b/src/common/exec.c
index fdcad0ee8cf23c087769166cd80f5a225c331e75..22f04aafbecd0f69611a70f21faf08b4c31d9d74 100644 (file)
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -388,9 +388,7 @@ pipe_read_line(char *cmd, char *line, int maxsize)
 {
        FILE       *pgver;
 
-       /* flush output buffers in case popen does not... */
-       fflush(stdout);
-       fflush(stderr);
+       fflush(NULL);
 
        errno = 0;
        if ((pgver = popen(cmd, "r")) == NULL)

diff --git a/src/fe_utils/archive.c b/src/fe_utils/archive.c
index 53d42c2be4118e4d6bed1e80452a775522bfddf8..fda93036615482bddf3e6e4027ba57a8f6c1c342 100644 (file)
--- a/src/fe_utils/archive.c
+++ b/src/fe_utils/archive.c
@@ -55,6 +55,7 @@ RestoreArchivedFile(const char *path, const char *xlogfname,
         * Execute restore_command, which should copy the missing file from
         * archival storage.
         */
+       fflush(NULL);
        rc = system(xlogRestoreCmd);
        pfree(xlogRestoreCmd);
 

diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index a48c9196978ee21fe43cbf88d115e56cb185e7c7..55288f8876c5725df3d3e57cc0c7848291aa5fb9 100644 (file)
--- a/src/fe_utils/print.c
+++ b/src/fe_utils/print.c
@@ -3118,6 +3118,7 @@ PageOutput(int lines, const printTableOpt *topt)
                                if (strspn(pagerprog, " \t\r\n") == strlen(pagerprog))
                                        return stdout;
                        }
+                       fflush(NULL);
                        disable_sigpipe_trap();
                        pagerpipe = popen(pagerprog, "w");
                        if (pagerpipe)

diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.c
index 783cd9b756f60e5d50b027af1b052130b7ee1433..21d346a08b4ce768c811f7bb4a7bf3a21ac66d3b 100644 (file)
--- a/src/interfaces/libpq/fe-print.c
+++ b/src/interfaces/libpq/fe-print.c
@@ -180,6 +180,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
                                  - (po->header != 0) * 2       /* row count and newline */
                                  )))
                        {
+                               fflush(NULL);
                                fout = popen(pagerenv, "w");
                                if (fout)
                                {

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index a803355f8eee09b10f0a1dcd376a286d669063e7..7290948eeee771fd29d4630fa70ccb985bd3c93b 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -263,15 +263,12 @@ stop_postmaster(void)
                char            buf[MAXPGPATH * 2];
                int                     r;
 
-               /* On Windows, system() seems not to force fflush, so... */
-               fflush(stdout);
-               fflush(stderr);
-
                snprintf(buf, sizeof(buf),
                                 "\"%s%spg_ctl\" stop -D \"%s/data\" -s",
                                 bindir ? bindir : "",
                                 bindir ? "/" : "",
                                 temp_instance);
+               fflush(NULL);
                r = system(buf);
                if (r != 0)
                {
@@ -1029,6 +1026,7 @@ psql_end_command(StringInfo buf, const char *database)
                                         database);
 
        /* And now we can execute the shell command */
+       fflush(NULL);
        if (system(buf->data) != 0)
        {
                /* psql probably already reported the error */
@@ -1063,13 +1061,9 @@ spawn_process(const char *cmdline)
        pid_t           pid;
 
        /*
-        * Must flush I/O buffers before fork.  Ideally we'd use fflush(NULL) here
-        * ... does anyone still care about systems where that doesn't work?
+        * Must flush I/O buffers before fork.
         */
-       fflush(stdout);
-       fflush(stderr);
-       if (logfile)
-               fflush(logfile);
+       fflush(NULL);
 
 #ifdef EXEC_BACKEND
        pg_disable_aslr();
@@ -1247,6 +1241,7 @@ run_diff(const char *cmd, const char *filename)
 {
        int                     r;
 
+       fflush(NULL);
        r = system(cmd);
        if (!WIFEXITED(r) || WEXITSTATUS(r) > 1)
        {
@@ -2264,6 +2259,7 @@ regression_main(int argc, char *argv[],
                                 debug ? " --debug" : "",
                                 nolocale ? " --no-locale" : "",
                                 outputdir);
+               fflush(NULL);
                if (system(buf))
                {
                        fprintf(stderr, _("\n%s: initdb failed\nExamine %s/log/initdb.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf);
@@ -2335,6 +2331,7 @@ regression_main(int argc, char *argv[],
 
                for (i = 0; i < 16; i++)
                {
+                       fflush(NULL);
                        if (system(buf2) == 0)
                        {
                                char            s[16];
@@ -2398,6 +2395,7 @@ regression_main(int argc, char *argv[],
                for (i = 0; i < wait_seconds; i++)
                {
                        /* Done if psql succeeds */
+                       fflush(NULL);
                        if (system(buf2) == 0)
                                break;