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

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();

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
 

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();

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

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

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;

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)

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

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");

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",

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)
 	{

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)

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",

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)
 	{

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");
 

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

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

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)

src/bin/psql/psqlscanslash.l

@@ -777,6 +777,7 @@ evaluate_backtick(PsqlScanState state)
 
 	initPQExpBuffer(&cmd_output);
 
+	fflush(NULL);
 	fd = popen(cmd, "r");
 	if (!fd)
 	{

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)

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

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)

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)
 				{

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;