Improve frontend error logging style.

Get rid of the separate "FATAL" log level, as it was applied
so inconsistently as to be meaningless.  This mostly involves
s/pg_log_fatal/pg_log_error/g.

Create a macro pg_fatal() to handle the common use-case of
pg_log_error() immediately followed by exit(1).  Various
modules had already invented either this or equivalent macros;
standardize on pg_fatal() and apply it where possible.

Invent the ability to add "detail" and "hint" messages to a
frontend message, much as we have long had in the backend.

Except where rewording was needed to convert existing coding
to detail/hint style, I have (mostly) resisted the temptation
to change existing message wording.

Patch by me.  Design and patch reviewed at various stages by
Robert Haas, Kyotaro Horiguchi, Peter Eisentraut and
Daniel Gustafsson.

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

1 files changed, 60 insertions, 150 deletions

diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index 1eb4509fcac..d4772a29650 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -161,14 +161,11 @@ main(int argc, char *argv[])
 					/*------
 					  translator: the second %s is a command line argument (-e, etc) */
 					pg_log_error("invalid argument for option %s", "-e");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (set_xid_epoch == -1)
-				{
-					pg_log_error("transaction ID epoch (-e) must not be -1");
-					exit(1);
-				}
+					pg_fatal("transaction ID epoch (-e) must not be -1");
 				break;
 
 			case 'u':
@@ -177,14 +174,11 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-u");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (!TransactionIdIsNormal(set_oldest_xid))
-				{
-					pg_log_error("oldest transaction ID (-u) must be greater than or equal to %u", FirstNormalTransactionId);
-					exit(1);
-				}
+					pg_fatal("oldest transaction ID (-u) must be greater than or equal to %u", FirstNormalTransactionId);
 				break;
 
 			case 'x':
@@ -193,14 +187,11 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-x");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (!TransactionIdIsNormal(set_xid))
-				{
-					pg_log_error("transaction ID (-x) must be greater than or equal to %u", FirstNormalTransactionId);
-					exit(1);
-				}
+					pg_fatal("transaction ID (-x) must be greater than or equal to %u", FirstNormalTransactionId);
 				break;
 
 			case 'c':
@@ -209,30 +200,24 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != ',' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-c");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				set_newest_commit_ts_xid = strtoul(endptr + 1, &endptr2, 0);
 				if (endptr2 == endptr + 1 || *endptr2 != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-c");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 
 				if (set_oldest_commit_ts_xid < 2 &&
 					set_oldest_commit_ts_xid != 0)
-				{
-					pg_log_error("transaction ID (-c) must be either 0 or greater than or equal to 2");
-					exit(1);
-				}
+					pg_fatal("transaction ID (-c) must be either 0 or greater than or equal to 2");
 
 				if (set_newest_commit_ts_xid < 2 &&
 					set_newest_commit_ts_xid != 0)
-				{
-					pg_log_error("transaction ID (-c) must be either 0 or greater than or equal to 2");
-					exit(1);
-				}
+					pg_fatal("transaction ID (-c) must be either 0 or greater than or equal to 2");
 				break;
 
 			case 'o':
@@ -241,14 +226,11 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-o");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (set_oid == 0)
-				{
-					pg_log_error("OID (-o) must not be 0");
-					exit(1);
-				}
+					pg_fatal("OID (-o) must not be 0");
 				break;
 
 			case 'm':
@@ -257,7 +239,7 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != ',' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-m");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 
@@ -265,24 +247,18 @@ main(int argc, char *argv[])
 				if (endptr2 == endptr + 1 || *endptr2 != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-m");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (set_mxid == 0)
-				{
-					pg_log_error("multitransaction ID (-m) must not be 0");
-					exit(1);
-				}
+					pg_fatal("multitransaction ID (-m) must not be 0");
 
 				/*
 				 * XXX It'd be nice to have more sanity checks here, e.g. so
 				 * that oldest is not wrapped around w.r.t. nextMulti.
 				 */
 				if (set_oldestmxid == 0)
-				{
-					pg_log_error("oldest multitransaction ID (-m) must not be 0");
-					exit(1);
-				}
+					pg_fatal("oldest multitransaction ID (-m) must not be 0");
 				break;
 
 			case 'O':
@@ -291,21 +267,18 @@ main(int argc, char *argv[])
 				if (endptr == optarg || *endptr != '\0' || errno != 0)
 				{
 					pg_log_error("invalid argument for option %s", "-O");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 				if (set_mxoff == -1)
-				{
-					pg_log_error("multitransaction offset (-O) must not be -1");
-					exit(1);
-				}
+					pg_fatal("multitransaction offset (-O) must not be -1");
 				break;
 
 			case 'l':
 				if (strspn(optarg, "01234567890ABCDEFabcdef") != XLOG_FNAME_LEN)
 				{
 					pg_log_error("invalid argument for option %s", "-l");
-					fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+					pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 					exit(1);
 				}
 
@@ -320,19 +293,14 @@ main(int argc, char *argv[])
 				errno = 0;
 				set_wal_segsize = strtol(optarg, &endptr, 10) * 1024 * 1024;
 				if (endptr == optarg || *endptr != '\0' || errno != 0)
-				{
-					pg_log_error("argument of --wal-segsize must be a number");
-					exit(1);
-				}
+					pg_fatal("argument of --wal-segsize must be a number");
 				if (!IsValidWalSegSize(set_wal_segsize))
-				{
-					pg_log_error("argument of --wal-segsize must be a power of 2 between 1 and 1024");
-					exit(1);
-				}
+					pg_fatal("argument of --wal-segsize must be a power of 2 between 1 and 1024");
 				break;
 
 			default:
-				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+				/* getopt_long already emitted a complaint */
+				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 				exit(1);
 		}
 	}
@@ -345,15 +313,14 @@ main(int argc, char *argv[])
 	{
 		pg_log_error("too many command-line arguments (first is \"%s\")",
 					 argv[optind]);
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
-				progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
 	if (DataDir == NULL)
 	{
 		pg_log_error("no data directory specified");
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
@@ -367,8 +334,8 @@ main(int argc, char *argv[])
 	if (geteuid() == 0)
 	{
 		pg_log_error("cannot be executed by \"root\"");
-		pg_log_info("You must run %s as the PostgreSQL superuser.",
-					progname);
+		pg_log_error_hint("You must run %s as the PostgreSQL superuser.",
+						  progname);
 		exit(1);
 	}
 #endif
@@ -377,20 +344,14 @@ main(int argc, char *argv[])
 
 	/* Set mask based on PGDATA permissions */
 	if (!GetDataDirectoryCreatePerm(DataDir))
-	{
-		pg_log_error("could not read permissions of directory \"%s\": %m",
-					 DataDir);
-		exit(1);
-	}
+		pg_fatal("could not read permissions of directory \"%s\": %m",
+				 DataDir);
 
 	umask(pg_mode_mask);
 
 	if (chdir(DataDir) < 0)
-	{
-		pg_log_error("could not change directory to \"%s\": %m",
-					 DataDir);
-		exit(1);
-	}
+		pg_fatal("could not change directory to \"%s\": %m",
+				 DataDir);
 
 	/* Check that data directory matches our server version */
 	CheckDataVersion();
@@ -402,16 +363,13 @@ main(int argc, char *argv[])
 	if ((fd = open("postmaster.pid", O_RDONLY, 0)) < 0)
 	{
 		if (errno != ENOENT)
-		{
-			pg_log_error("could not open file \"%s\" for reading: %m",
-						 "postmaster.pid");
-			exit(1);
-		}
+			pg_fatal("could not open file \"%s\" for reading: %m",
+					 "postmaster.pid");
 	}
 	else
 	{
 		pg_log_error("lock file \"%s\" exists", "postmaster.pid");
-		pg_log_info("Is a server running?  If not, delete the lock file and try again.");
+		pg_log_error_hint("Is a server running?  If not, delete the lock file and try again.");
 		exit(1);
 	}
 
@@ -557,20 +515,16 @@ CheckDataVersion(void)
 	char		rawline[64];
 
 	if ((ver_fd = fopen(ver_file, "r")) == NULL)
-	{
-		pg_log_error("could not open file \"%s\" for reading: %m",
-					 ver_file);
-		exit(1);
-	}
+		pg_fatal("could not open file \"%s\" for reading: %m",
+				 ver_file);
 
 	/* version number has to be the first line read */
 	if (!fgets(rawline, sizeof(rawline), ver_fd))
 	{
 		if (!ferror(ver_fd))
-			pg_log_error("unexpected empty file \"%s\"", ver_file);
+			pg_fatal("unexpected empty file \"%s\"", ver_file);
 		else
-			pg_log_error("could not read file \"%s\": %m", ver_file);
-		exit(1);
+			pg_fatal("could not read file \"%s\": %m", ver_file);
 	}
 
 	/* strip trailing newline and carriage return */
@@ -579,8 +533,8 @@ CheckDataVersion(void)
 	if (strcmp(rawline, PG_MAJORVERSION) != 0)
 	{
 		pg_log_error("data directory is of wrong version");
-		pg_log_info("File \"%s\" contains \"%s\", which is not compatible with this program's version \"%s\".",
-					ver_file, rawline, PG_MAJORVERSION);
+		pg_log_error_detail("File \"%s\" contains \"%s\", which is not compatible with this program's version \"%s\".",
+							ver_file, rawline, PG_MAJORVERSION);
 		exit(1);
 	}
 
@@ -612,10 +566,10 @@ read_controlfile(void)
 		pg_log_error("could not open file \"%s\" for reading: %m",
 					 XLOG_CONTROL_FILE);
 		if (errno == ENOENT)
-			pg_log_info("If you are sure the data directory path is correct, execute\n"
-						"  touch %s\n"
-						"and try again.",
-						XLOG_CONTROL_FILE);
+			pg_log_error_hint("If you are sure the data directory path is correct, execute\n"
+							  "  touch %s\n"
+							  "and try again.",
+							  XLOG_CONTROL_FILE);
 		exit(1);
 	}
 
@@ -624,10 +578,7 @@ read_controlfile(void)
 
 	len = read(fd, buffer, PG_CONTROL_FILE_SIZE);
 	if (len < 0)
-	{
-		pg_log_error("could not read file \"%s\": %m", XLOG_CONTROL_FILE);
-		exit(1);
-	}
+		pg_fatal("could not read file \"%s\": %m", XLOG_CONTROL_FILE);
 	close(fd);
 
 	if (len >= sizeof(ControlFileData) &&
@@ -968,10 +919,7 @@ FindEndOfXLOG(void)
 	 */
 	xldir = opendir(XLOGDIR);
 	if (xldir == NULL)
-	{
-		pg_log_error("could not open directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not open directory \"%s\": %m", XLOGDIR);
 
 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
 	{
@@ -1003,16 +951,10 @@ FindEndOfXLOG(void)
 	}
 
 	if (errno)
-	{
-		pg_log_error("could not read directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not read directory \"%s\": %m", XLOGDIR);
 
 	if (closedir(xldir))
-	{
-		pg_log_error("could not close directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not close directory \"%s\": %m", XLOGDIR);
 
 	/*
 	 * Finally, convert to new xlog seg size, and advance by one to ensure we
@@ -1036,10 +978,7 @@ KillExistingXLOG(void)
 
 	xldir = opendir(XLOGDIR);
 	if (xldir == NULL)
-	{
-		pg_log_error("could not open directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not open directory \"%s\": %m", XLOGDIR);
 
 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
 	{
@@ -1048,24 +987,15 @@ KillExistingXLOG(void)
 		{
 			snprintf(path, sizeof(path), "%s/%s", XLOGDIR, xlde->d_name);
 			if (unlink(path) < 0)
-			{
-				pg_log_error("could not delete file \"%s\": %m", path);
-				exit(1);
-			}
+				pg_fatal("could not delete file \"%s\": %m", path);
 		}
 	}
 
 	if (errno)
-	{
-		pg_log_error("could not read directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not read directory \"%s\": %m", XLOGDIR);
 
 	if (closedir(xldir))
-	{
-		pg_log_error("could not close directory \"%s\": %m", XLOGDIR);
-		exit(1);
-	}
+		pg_fatal("could not close directory \"%s\": %m", XLOGDIR);
 }
 
 
@@ -1083,10 +1013,7 @@ KillExistingArchiveStatus(void)
 
 	xldir = opendir(ARCHSTATDIR);
 	if (xldir == NULL)
-	{
-		pg_log_error("could not open directory \"%s\": %m", ARCHSTATDIR);
-		exit(1);
-	}
+		pg_fatal("could not open directory \"%s\": %m", ARCHSTATDIR);
 
 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
 	{
@@ -1098,24 +1025,15 @@ KillExistingArchiveStatus(void)
 		{
 			snprintf(path, sizeof(path), "%s/%s", ARCHSTATDIR, xlde->d_name);
 			if (unlink(path) < 0)
-			{
-				pg_log_error("could not delete file \"%s\": %m", path);
-				exit(1);
-			}
+				pg_fatal("could not delete file \"%s\": %m", path);
 		}
 	}
 
 	if (errno)
-	{
-		pg_log_error("could not read directory \"%s\": %m", ARCHSTATDIR);
-		exit(1);
-	}
+		pg_fatal("could not read directory \"%s\": %m", ARCHSTATDIR);
 
 	if (closedir(xldir))
-	{
-		pg_log_error("could not close directory \"%s\": %m", ARCHSTATDIR);
-		exit(1);
-	}
+		pg_fatal("could not close directory \"%s\": %m", ARCHSTATDIR);
 }
 
 
@@ -1179,10 +1097,7 @@ WriteEmptyXLOG(void)
 	fd = open(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
 			  pg_file_create_mode);
 	if (fd < 0)
-	{
-		pg_log_error("could not open file \"%s\": %m", path);
-		exit(1);
-	}
+		pg_fatal("could not open file \"%s\": %m", path);
 
 	errno = 0;
 	if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
@@ -1190,8 +1105,7 @@ WriteEmptyXLOG(void)
 		/* if write didn't set errno, assume problem is no disk space */
 		if (errno == 0)
 			errno = ENOSPC;
-		pg_log_error("could not write file \"%s\": %m", path);
-		exit(1);
+		pg_fatal("could not write file \"%s\": %m", path);
 	}
 
 	/* Fill the rest of the file with zeroes */
@@ -1203,16 +1117,12 @@ WriteEmptyXLOG(void)
 		{
 			if (errno == 0)
 				errno = ENOSPC;
-			pg_log_error("could not write file \"%s\": %m", path);
-			exit(1);
+			pg_fatal("could not write file \"%s\": %m", path);
 		}
 	}
 
 	if (fsync(fd) != 0)
-	{
-		pg_log_error("fsync error: %m");
-		exit(1);
-	}
+		pg_fatal("fsync error: %m");
 
 	close(fd);
 }