Re: Using the %m printf format more - Mailing list pgsql-hackers

From Dagfinn Ilmari Mannsåker
Subject Re: Using the %m printf format more
Date
Msg-id 87plw1j9sb.fsf@wibble.ilmari.org
Whole thread Raw
In response to Re: Using the %m printf format more  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Using the %m printf format more
List pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes:

> On Wed, Mar 06, 2024 at 07:11:19PM +0000, Dagfinn Ilmari Mannsåker wrote:
>
>> The attached patch does so everywhere appropriate. One place where it's
>> not appropriate is the TAP-emitting functions in pg_regress, since those
>> call fprintf()
>
> I am not really following your argument with pg_regress.c and
> fprintf().  d6c55de1f99a should make that possible even in the case of
> emit_tap_output_v(), no? 

The problem isn't that emit_tap_output_v() doesn't support %m, which it
does, but that it potentially calls fprintf() to output TAP protocol
elements such as "\n" and "# " before it calls vprintf(…, fmt, …), and
those calls might clobber errno.  An option is to make it save errno at
the start and restore it before the vprintf() calls, as in the second
attached patch.

>> and other potentially errno-modifying functions before
>> evaluating the format string.
>
> Sure.

On closer look, fprintf() is actually the only errno-clobbering function
it calls, I was just hedging my bets in that statement.

- ilmari

From 3727341aad84fbd275acb24f92591cc734fdd6a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 6 Mar 2024 16:58:33 +0000
Subject: [PATCH v2 1/2] Use %m printf format instead of strerror(errno) where
 appropriate

Now that all live branches (12-) support %m in printf formats, let's
use it everywhere appropriate.

Particularly, we're _not_ converting the TAP output calls in
pg_regress, since those functions call fprintf() and friends before
evaluating the format string, which can clobber errno.
---
 src/backend/postmaster/postmaster.c           | 21 ++--
 src/backend/postmaster/syslogger.c            |  2 +-
 src/backend/utils/misc/guc.c                  |  9 +-
 src/bin/initdb/findtimezone.c                 |  4 +-
 src/bin/pg_ctl/pg_ctl.c                       | 74 +++++++-------
 src/bin/pg_dump/compress_gzip.c               |  2 +-
 src/bin/pg_dump/compress_none.c               |  5 +-
 src/bin/pg_upgrade/check.c                    | 41 +++-----
 src/bin/pg_upgrade/controldata.c              |  6 +-
 src/bin/pg_upgrade/exec.c                     | 14 ++-
 src/bin/pg_upgrade/file.c                     | 98 +++++++++----------
 src/bin/pg_upgrade/function.c                 |  3 +-
 src/bin/pg_upgrade/option.c                   | 10 +-
 src/bin/pg_upgrade/parallel.c                 | 12 +--
 src/bin/pg_upgrade/pg_upgrade.c               |  4 +-
 src/bin/pg_upgrade/relfilenumber.c            |  5 +-
 src/bin/pg_upgrade/tablespace.c               |  4 +-
 src/bin/pg_upgrade/version.c                  |  9 +-
 src/common/psprintf.c                         |  4 +-
 src/interfaces/ecpg/preproc/ecpg.c            | 12 +--
 src/port/path.c                               |  3 +-
 src/test/isolation/isolationtester.c          |  2 +-
 .../modules/libpq_pipeline/libpq_pipeline.c   |  4 +-
 src/tools/ifaddrs/test_ifaddrs.c              |  2 +-
 24 files changed, 158 insertions(+), 192 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index f3c09e8dc0..af8a1efe66 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1375,12 +1375,12 @@ PostmasterMain(int argc, char *argv[])
 
             /* Make PID file world readable */
             if (chmod(external_pid_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0)
-                write_stderr("%s: could not change permissions of external PID file \"%s\": %s\n",
-                             progname, external_pid_file, strerror(errno));
+                write_stderr("%s: could not change permissions of external PID file \"%s\": %m\n",
+                             progname, external_pid_file);
         }
         else
-            write_stderr("%s: could not write external PID file \"%s\": %s\n",
-                         progname, external_pid_file, strerror(errno));
+            write_stderr("%s: could not write external PID file \"%s\": %m\n",
+                         progname, external_pid_file);
 
         on_proc_exit(unlink_external_pid_file, 0);
     }
@@ -1589,8 +1589,8 @@ checkControlFile(void)
     {
         write_stderr("%s: could not find the database system\n"
                      "Expected to find it in the directory \"%s\",\n"
-                     "but could not open file \"%s\": %s\n",
-                     progname, DataDir, path, strerror(errno));
+                     "but could not open file \"%s\": %m\n",
+                     progname, DataDir, path);
         ExitPostmaster(2);
     }
     FreeFile(fp);
@@ -6277,15 +6277,13 @@ read_backend_variables(char *id, Port **port, BackgroundWorker **worker)
     fp = AllocateFile(id, PG_BINARY_R);
     if (!fp)
     {
-        write_stderr("could not open backend variables file \"%s\": %s\n",
-                     id, strerror(errno));
+        write_stderr("could not open backend variables file \"%s\": %m\n", id);
         exit(1);
     }
 
     if (fread(¶m, sizeof(param), 1, fp) != 1)
     {
-        write_stderr("could not read from backend variables file \"%s\": %s\n",
-                     id, strerror(errno));
+        write_stderr("could not read from backend variables file \"%s\": %m\n", id);
         exit(1);
     }
 
@@ -6293,8 +6291,7 @@ read_backend_variables(char *id, Port **port, BackgroundWorker **worker)
     FreeFile(fp);
     if (unlink(id) != 0)
     {
-        write_stderr("could not remove file \"%s\": %s\n",
-                     id, strerror(errno));
+        write_stderr("could not remove file \"%s\": %m\n", id);
         exit(1);
     }
 #else
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index c2a6a226e7..d9d042f562 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -1173,7 +1173,7 @@ write_syslogger_file(const char *buffer, int count, int destination)
      * to our input pipe which would result in a different sort of looping.
      */
     if (rc != count)
-        write_stderr("could not write to log file: %s\n", strerror(errno));
+        write_stderr("could not write to log file: %m\n");
 }
 
 #ifdef WIN32
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index dd5a46469a..391866145e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1799,10 +1799,9 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 
     if (configdir && stat(configdir, &stat_buf) != 0)
     {
-        write_stderr("%s: could not access directory \"%s\": %s\n",
+        write_stderr("%s: could not access directory \"%s\": %m\n",
                      progname,
-                     configdir,
-                     strerror(errno));
+                     configdir);
         if (errno == ENOENT)
             write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
         return false;
@@ -1851,8 +1850,8 @@ SelectConfigFiles(const char *userDoption, const char *progname)
      */
     if (stat(ConfigFileName, &stat_buf) != 0)
     {
-        write_stderr("%s: could not access the server configuration file \"%s\": %s\n",
-                     progname, ConfigFileName, strerror(errno));
+        write_stderr("%s: could not access the server configuration file \"%s\": %m\n",
+                     progname, ConfigFileName);
         free(configdir);
         return false;
     }
diff --git a/src/bin/initdb/findtimezone.c b/src/bin/initdb/findtimezone.c
index e15630fdee..95204e482b 100644
--- a/src/bin/initdb/findtimezone.c
+++ b/src/bin/initdb/findtimezone.c
@@ -680,8 +680,8 @@ scan_available_timezones(char *tzdir, char *tzdirsub, struct tztry *tt,
         if (stat(tzdir, &statbuf) != 0)
         {
 #ifdef DEBUG_IDENTIFY_TIMEZONE
-            fprintf(stderr, "could not stat \"%s\": %s\n",
-                    tzdir, strerror(errno));
+            fprintf(stderr, "could not stat \"%s\": %m\n",
+                    tzdir);
 #endif
             tzdir[tzdir_orig_len] = '\0';
             continue;
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 6900b27675..405e223c19 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -254,8 +254,8 @@ get_pgpid(bool is_status_request)
             write_stderr(_("%s: directory \"%s\" does not exist\n"), progname,
                          pg_data);
         else
-            write_stderr(_("%s: could not access directory \"%s\": %s\n"), progname,
-                         pg_data, strerror(errno));
+            write_stderr(_("%s: could not access directory \"%s\": %m\n"), progname,
+                         pg_data);
 
         /*
          * The Linux Standard Base Core Specification 3.1 says this should
@@ -280,8 +280,8 @@ get_pgpid(bool is_status_request)
             return 0;
         else
         {
-            write_stderr(_("%s: could not open PID file \"%s\": %s\n"),
-                         progname, pid_file, strerror(errno));
+            write_stderr(_("%s: could not open PID file \"%s\": %m\n"),
+                         progname, pid_file);
             exit(1);
         }
     }
@@ -454,8 +454,8 @@ start_postmaster(void)
     if (pm_pid < 0)
     {
         /* fork failed */
-        write_stderr(_("%s: could not start server: %s\n"),
-                     progname, strerror(errno));
+        write_stderr(_("%s: could not start server: %m\n"),
+                     progname);
         exit(1);
     }
     if (pm_pid > 0)
@@ -474,8 +474,8 @@ start_postmaster(void)
 #ifdef HAVE_SETSID
     if (setsid() < 0)
     {
-        write_stderr(_("%s: could not start server due to setsid() failure: %s\n"),
-                     progname, strerror(errno));
+        write_stderr(_("%s: could not start server due to setsid() failure: %m\n"),
+                     progname);
         exit(1);
     }
 #endif
@@ -496,8 +496,8 @@ start_postmaster(void)
     (void) execl("/bin/sh", "/bin/sh", "-c", cmd, (char *) NULL);
 
     /* exec failed */
-    write_stderr(_("%s: could not start server: %s\n"),
-                 progname, strerror(errno));
+    write_stderr(_("%s: could not start server: %m\n"),
+                 progname);
     exit(1);
 
     return 0;                    /* keep dumb compilers quiet */
@@ -544,8 +544,8 @@ start_postmaster(void)
              */
             if (errno != ENOENT)
             {
-                write_stderr(_("%s: could not open log file \"%s\": %s\n"),
-                             progname, log_file, strerror(errno));
+                write_stderr(_("%s: could not open log file \"%s\": %m\n"),
+                             progname, log_file);
                 exit(1);
             }
         }
@@ -851,8 +851,8 @@ trap_sigint_during_startup(SIGNAL_ARGS)
     if (postmasterPID != -1)
     {
         if (kill(postmasterPID, SIGINT) != 0)
-            write_stderr(_("%s: could not send stop signal (PID: %d): %s\n"),
-                         progname, (int) postmasterPID, strerror(errno));
+            write_stderr(_("%s: could not send stop signal (PID: %d): %m\n"),
+                         progname, (int) postmasterPID);
     }
 
     /*
@@ -1035,8 +1035,7 @@ do_stop(void)
 
     if (kill(pid, sig) != 0)
     {
-        write_stderr(_("%s: could not send stop signal (PID: %d): %s\n"), progname, (int) pid,
-                     strerror(errno));
+        write_stderr(_("%s: could not send stop signal (PID: %d): %m\n"), progname, (int) pid);
         exit(1);
     }
 
@@ -1103,8 +1102,7 @@ do_restart(void)
     {
         if (kill(pid, sig) != 0)
         {
-            write_stderr(_("%s: could not send stop signal (PID: %d): %s\n"), progname, (int) pid,
-                         strerror(errno));
+            write_stderr(_("%s: could not send stop signal (PID: %d): %m\n"), progname, (int) pid);
             exit(1);
         }
 
@@ -1159,8 +1157,8 @@ do_reload(void)
 
     if (kill(pid, sig) != 0)
     {
-        write_stderr(_("%s: could not send reload signal (PID: %d): %s\n"),
-                     progname, (int) pid, strerror(errno));
+        write_stderr(_("%s: could not send reload signal (PID: %d): %m\n"),
+                     progname, (int) pid);
         exit(1);
     }
 
@@ -1207,25 +1205,25 @@ do_promote(void)
 
     if ((prmfile = fopen(promote_file, "w")) == NULL)
     {
-        write_stderr(_("%s: could not create promote signal file \"%s\": %s\n"),
-                     progname, promote_file, strerror(errno));
+        write_stderr(_("%s: could not create promote signal file \"%s\": %m\n"),
+                     progname, promote_file);
         exit(1);
     }
     if (fclose(prmfile))
     {
-        write_stderr(_("%s: could not write promote signal file \"%s\": %s\n"),
-                     progname, promote_file, strerror(errno));
+        write_stderr(_("%s: could not write promote signal file \"%s\": %m\n"),
+                     progname, promote_file);
         exit(1);
     }
 
     sig = SIGUSR1;
     if (kill(pid, sig) != 0)
     {
-        write_stderr(_("%s: could not send promote signal (PID: %d): %s\n"),
-                     progname, (int) pid, strerror(errno));
+        write_stderr(_("%s: could not send promote signal (PID: %d): %m\n"),
+                     progname, (int) pid);
         if (unlink(promote_file) != 0)
-            write_stderr(_("%s: could not remove promote signal file \"%s\": %s\n"),
-                         progname, promote_file, strerror(errno));
+            write_stderr(_("%s: could not remove promote signal file \"%s\": %m\n"),
+                         progname, promote_file);
         exit(1);
     }
 
@@ -1280,25 +1278,25 @@ do_logrotate(void)
 
     if ((logrotatefile = fopen(logrotate_file, "w")) == NULL)
     {
-        write_stderr(_("%s: could not create log rotation signal file \"%s\": %s\n"),
-                     progname, logrotate_file, strerror(errno));
+        write_stderr(_("%s: could not create log rotation signal file \"%s\": %m\n"),
+                     progname, logrotate_file);
         exit(1);
     }
     if (fclose(logrotatefile))
     {
-        write_stderr(_("%s: could not write log rotation signal file \"%s\": %s\n"),
-                     progname, logrotate_file, strerror(errno));
+        write_stderr(_("%s: could not write log rotation signal file \"%s\": %m\n"),
+                     progname, logrotate_file);
         exit(1);
     }
 
     sig = SIGUSR1;
     if (kill(pid, sig) != 0)
     {
-        write_stderr(_("%s: could not send log rotation signal (PID: %d): %s\n"),
-                     progname, (int) pid, strerror(errno));
+        write_stderr(_("%s: could not send log rotation signal (PID: %d): %m\n"),
+                     progname, (int) pid);
         if (unlink(logrotate_file) != 0)
-            write_stderr(_("%s: could not remove log rotation signal file \"%s\": %s\n"),
-                         progname, logrotate_file, strerror(errno));
+            write_stderr(_("%s: could not remove log rotation signal file \"%s\": %m\n"),
+                         progname, logrotate_file);
         exit(1);
     }
 
@@ -1396,8 +1394,8 @@ do_kill(pid_t pid)
 {
     if (kill(pid, sig) != 0)
     {
-        write_stderr(_("%s: could not send signal %d (PID: %d): %s\n"),
-                     progname, sig, (int) pid, strerror(errno));
+        write_stderr(_("%s: could not send signal %d (PID: %d): %m\n"),
+                     progname, sig, (int) pid);
         exit(1);
     }
 }
diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index c768a11750..26768b9f71 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -292,7 +292,7 @@ Gzip_getc(CompressFileHandle *CFH)
     if (ret == EOF)
     {
         if (!gzeof(gzfp))
-            pg_fatal("could not read from input file: %s", strerror(errno));
+            pg_fatal("could not read from input file: %m");
         else
             pg_fatal("could not read from input file: end of file");
     }
diff --git a/src/bin/pg_dump/compress_none.c b/src/bin/pg_dump/compress_none.c
index 06c400424a..f3a524d8a4 100644
--- a/src/bin/pg_dump/compress_none.c
+++ b/src/bin/pg_dump/compress_none.c
@@ -94,8 +94,7 @@ read_none(void *ptr, size_t size, size_t *rsize, CompressFileHandle *CFH)
 
     ret = fread(ptr, 1, size, fp);
     if (ret != size && !feof(fp))
-        pg_fatal("could not read from input file: %s",
-                 strerror(errno));
+        pg_fatal("could not read from input file: %m");
 
     if (rsize)
         *rsize = ret;
@@ -137,7 +136,7 @@ getc_none(CompressFileHandle *CFH)
     if (ret == EOF)
     {
         if (!feof(fp))
-            pg_fatal("could not read from input file: %s", strerror(errno));
+            pg_fatal("could not read from input file: %m");
         else
             pg_fatal("could not read from input file: end of file");
     }
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 5ab8fe8009..8ce6c674e3 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -505,8 +505,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
     prep_status("Creating script to delete old cluster");
 
     if ((script = fopen_priv(*deletion_script_file_name, "w")) == NULL)
-        pg_fatal("could not open file \"%s\": %s",
-                 *deletion_script_file_name, strerror(errno));
+        pg_fatal("could not open file \"%s\": %m",
+                 *deletion_script_file_name);
 
 #ifndef WIN32
     /* add shebang header */
@@ -556,8 +556,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
 
 #ifndef WIN32
     if (chmod(*deletion_script_file_name, S_IRWXU) != 0)
-        pg_fatal("could not add execute permission to file \"%s\": %s",
-                 *deletion_script_file_name, strerror(errno));
+        pg_fatal("could not add execute permission to file \"%s\": %m",
+                 *deletion_script_file_name);
 #endif
 
     check_ok();
@@ -678,8 +678,7 @@ check_proper_datallowconn(ClusterInfo *cluster)
             if (strcmp(datallowconn, "f") == 0)
             {
                 if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s",
-                             output_path, strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
 
                 fprintf(script, "%s\n", datname);
             }
@@ -794,8 +793,7 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
         for (rowno = 0; rowno < ntups; rowno++)
         {
             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -889,8 +887,7 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster)
         {
             if (script == NULL &&
                 (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -1018,8 +1015,7 @@ check_for_incompatible_polymorphics(ClusterInfo *cluster)
         {
             if (script == NULL &&
                 (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -1095,8 +1091,7 @@ check_for_tables_with_oids(ClusterInfo *cluster)
         for (rowno = 0; rowno < ntups; rowno++)
         {
             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -1374,8 +1369,7 @@ check_for_pg_role_prefix(ClusterInfo *cluster)
     for (int rowno = 0; rowno < ntups; rowno++)
     {
         if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-            pg_fatal("could not open file \"%s\": %s",
-                     output_path, strerror(errno));
+            pg_fatal("could not open file \"%s\": %m", output_path);
         fprintf(script, "%s (oid=%s)\n",
                 PQgetvalue(res, rowno, i_rolname),
                 PQgetvalue(res, rowno, i_roloid));
@@ -1448,8 +1442,7 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster)
         {
             if (script == NULL &&
                 (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -1631,8 +1624,7 @@ check_old_cluster_for_valid_slots(bool live_check)
             {
                 if (script == NULL &&
                     (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s",
-                             output_path, strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
 
                 fprintf(script, "The slot \"%s\" is invalid\n",
                         slot->slotname);
@@ -1651,8 +1643,7 @@ check_old_cluster_for_valid_slots(bool live_check)
             {
                 if (script == NULL &&
                     (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s",
-                             output_path, strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
 
                 fprintf(script,
                         "The slot \"%s\" has not consumed the WAL yet\n",
@@ -1721,8 +1712,7 @@ check_old_cluster_subscription_state(void)
             for (int i = 0; i < ntup; i++)
             {
                 if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s",
-                             output_path, strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
                 fprintf(script, "The replication origin is missing for database:\"%s\" subscription:\"%s\"\n",
                         PQgetvalue(res, i, 0),
                         PQgetvalue(res, i, 1));
@@ -1774,8 +1764,7 @@ check_old_cluster_subscription_state(void)
         for (int i = 0; i < ntup; i++)
         {
             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s",
-                         output_path, strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
 
             fprintf(script, "The table sync state \"%s\" is not allowed for database:\"%s\" subscription:\"%s\"
schema:\"%s\"relation:\"%s\"\n",
 
                     PQgetvalue(res, i, 0),
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index d10e31426d..3c836c7c13 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -126,8 +126,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
         fflush(NULL);
 
         if ((output = popen(cmd, "r")) == NULL)
-            pg_fatal("could not get control data using %s: %s",
-                     cmd, strerror(errno));
+            pg_fatal("could not get control data using %s: %m", cmd);
 
         /* we have the result of cmd in "output". so parse it line by line now */
         while (fgets(bufin, sizeof(bufin), output))
@@ -197,8 +196,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
     fflush(NULL);
 
     if ((output = popen(cmd, "r")) == NULL)
-        pg_fatal("could not get control data using %s: %s",
-                 cmd, strerror(errno));
+        pg_fatal("could not get control data using %s: %m", cmd);
 
     /* Only in <= 9.2 */
     if (GET_MAJOR_VERSION(cluster->major_version) <= 902)
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 3552cf00af..058530ab3e 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -44,8 +44,7 @@ get_bin_version(ClusterInfo *cluster)
 
     if ((output = popen(cmd, "r")) == NULL ||
         fgets(cmd_output, sizeof(cmd_output), output) == NULL)
-        pg_fatal("could not get pg_ctl version data using %s: %s",
-                 cmd, strerror(errno));
+        pg_fatal("could not get pg_ctl version data using %s: %m", cmd);
 
     rc = pclose(output);
     if (rc != 0)
@@ -242,8 +241,7 @@ pid_lock_file_exists(const char *datadir)
     {
         /* ENOTDIR means we will throw a more useful error later */
         if (errno != ENOENT && errno != ENOTDIR)
-            pg_fatal("could not open file \"%s\" for reading: %s",
-                     path, strerror(errno));
+            pg_fatal("could not open file \"%s\" for reading: %m", path);
 
         return false;
     }
@@ -322,8 +320,8 @@ check_single_dir(const char *pg_data, const char *subdir)
              subdir);
 
     if (stat(subDirName, &statBuf) != 0)
-        report_status(PG_FATAL, "check for \"%s\" failed: %s",
-                      subDirName, strerror(errno));
+        report_status(PG_FATAL, "check for \"%s\" failed: %m",
+                      subDirName);
     else if (!S_ISDIR(statBuf.st_mode))
         report_status(PG_FATAL, "\"%s\" is not a directory",
                       subDirName);
@@ -388,8 +386,8 @@ check_bin_dir(ClusterInfo *cluster, bool check_versions)
 
     /* check bindir */
     if (stat(cluster->bindir, &statBuf) != 0)
-        report_status(PG_FATAL, "check for \"%s\" failed: %s",
-                      cluster->bindir, strerror(errno));
+        report_status(PG_FATAL, "check for \"%s\" failed: %m",
+                      cluster->bindir);
     else if (!S_ISDIR(statBuf.st_mode))
         report_status(PG_FATAL, "\"%s\" is not a directory",
                       cluster->bindir);
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index beba376f2e..73932504ca 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -41,20 +41,20 @@ cloneFile(const char *src, const char *dst,
 {
 #if defined(HAVE_COPYFILE) && defined(COPYFILE_CLONE_FORCE)
     if (copyfile(src, dst, NULL, COPYFILE_CLONE_FORCE) < 0)
-        pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
-                 schemaName, relName, src, dst, strerror(errno));
+        pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %m",
+                 schemaName, relName, src, dst);
 #elif defined(__linux__) && defined(FICLONE)
     int            src_fd;
     int            dest_fd;
 
     if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
-        pg_fatal("error while cloning relation \"%s.%s\": could not open file \"%s\": %s",
-                 schemaName, relName, src, strerror(errno));
+        pg_fatal("error while cloning relation \"%s.%s\": could not open file \"%s\": %m",
+                 schemaName, relName, src);
 
     if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                         pg_file_create_mode)) < 0)
-        pg_fatal("error while cloning relation \"%s.%s\": could not create file \"%s\": %s",
-                 schemaName, relName, dst, strerror(errno));
+        pg_fatal("error while cloning relation \"%s.%s\": could not create file \"%s\": %m",
+                 schemaName, relName, dst);
 
     if (ioctl(dest_fd, FICLONE, src_fd) < 0)
     {
@@ -88,13 +88,13 @@ copyFile(const char *src, const char *dst,
     char       *buffer;
 
     if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s",
-                 schemaName, relName, src, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %m",
+                 schemaName, relName, src);
 
     if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                         pg_file_create_mode)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s",
-                 schemaName, relName, dst, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %m",
+                 schemaName, relName, dst);
 
     /* copy in fairly large chunks for best efficiency */
 #define COPY_BUF_SIZE (50 * BLCKSZ)
@@ -107,8 +107,8 @@ copyFile(const char *src, const char *dst,
         ssize_t        nbytes = read(src_fd, buffer, COPY_BUF_SIZE);
 
         if (nbytes < 0)
-            pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s",
-                     schemaName, relName, src, strerror(errno));
+            pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %m",
+                     schemaName, relName, src);
 
         if (nbytes == 0)
             break;
@@ -119,8 +119,8 @@ copyFile(const char *src, const char *dst,
             /* if write didn't set errno, assume problem is no disk space */
             if (errno == 0)
                 errno = ENOSPC;
-            pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s",
-                     schemaName, relName, dst, strerror(errno));
+            pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %m",
+                     schemaName, relName, dst);
         }
     }
 
@@ -133,8 +133,8 @@ copyFile(const char *src, const char *dst,
     if (CopyFile(src, dst, true) == 0)
     {
         _dosmaperr(GetLastError());
-        pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
-                 schemaName, relName, src, dst, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %m",
+                 schemaName, relName, src, dst);
     }
 
 #endif                            /* WIN32 */
@@ -157,20 +157,20 @@ copyFileByRange(const char *src, const char *dst,
     ssize_t        nbytes;
 
     if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s",
-                 schemaName, relName, src, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %m",
+                 schemaName, relName, src);
 
     if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                         pg_file_create_mode)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s",
-                 schemaName, relName, dst, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %m",
+                 schemaName, relName, dst);
 
     do
     {
         nbytes = copy_file_range(src_fd, NULL, dest_fd, NULL, SSIZE_MAX, 0);
         if (nbytes < 0)
-            pg_fatal("error while copying relation \"%s.%s\": could not copy file range from \"%s\" to \"%s\": %s",
-                     schemaName, relName, src, dst, strerror(errno));
+            pg_fatal("error while copying relation \"%s.%s\": could not copy file range from \"%s\" to \"%s\": %m",
+                     schemaName, relName, src, dst);
     }
     while (nbytes > 0);
 
@@ -191,8 +191,8 @@ linkFile(const char *src, const char *dst,
          const char *schemaName, const char *relName)
 {
     if (link(src, dst) < 0)
-        pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
-                 schemaName, relName, src, dst, strerror(errno));
+        pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %m",
+                 schemaName, relName, src, dst);
 }
 
 
@@ -230,17 +230,17 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
     rewriteVmBytesPerPage = (BLCKSZ - SizeOfPageHeaderData) / 2;
 
     if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s",
-                 schemaName, relName, fromfile, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %m",
+                 schemaName, relName, fromfile);
 
     if (fstat(src_fd, &statbuf) != 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %s",
-                 schemaName, relName, fromfile, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %m",
+                 schemaName, relName, fromfile);
 
     if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                        pg_file_create_mode)) < 0)
-        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s",
-                 schemaName, relName, tofile, strerror(errno));
+        pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %m",
+                 schemaName, relName, tofile);
 
     /* Save old file size */
     src_filesize = statbuf.st_size;
@@ -263,8 +263,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
         if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ)
         {
             if (bytesRead < 0)
-                pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s",
-                         schemaName, relName, fromfile, strerror(errno));
+                pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %m",
+                         schemaName, relName, fromfile);
             else
                 pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"",
                          schemaName, relName, fromfile);
@@ -341,8 +341,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
                 /* if write didn't set errno, assume problem is no disk space */
                 if (errno == 0)
                     errno = ENOSPC;
-                pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s",
-                         schemaName, relName, tofile, strerror(errno));
+                pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %m",
+                         schemaName, relName, tofile);
             }
 
             /* Advance for next new page */
@@ -368,25 +368,23 @@ check_file_clone(void)
 
 #if defined(HAVE_COPYFILE) && defined(COPYFILE_CLONE_FORCE)
     if (copyfile(existing_file, new_link_file, NULL, COPYFILE_CLONE_FORCE) < 0)
-        pg_fatal("could not clone file between old and new data directories: %s",
-                 strerror(errno));
+        pg_fatal("could not clone file between old and new data directories: %m");
 #elif defined(__linux__) && defined(FICLONE)
     {
         int            src_fd;
         int            dest_fd;
 
         if ((src_fd = open(existing_file, O_RDONLY | PG_BINARY, 0)) < 0)
-            pg_fatal("could not open file \"%s\": %s",
-                     existing_file, strerror(errno));
+            pg_fatal("could not open file \"%s\": %m",
+                     existing_file);
 
         if ((dest_fd = open(new_link_file, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                             pg_file_create_mode)) < 0)
-            pg_fatal("could not create file \"%s\": %s",
-                     new_link_file, strerror(errno));
+            pg_fatal("could not create file \"%s\": %m",
+                     new_link_file);
 
         if (ioctl(dest_fd, FICLONE, src_fd) < 0)
-            pg_fatal("could not clone file between old and new data directories: %s",
-                     strerror(errno));
+            pg_fatal("could not clone file between old and new data directories: %m");
 
         close(src_fd);
         close(dest_fd);
@@ -414,17 +412,16 @@ check_copy_file_range(void)
         int            dest_fd;
 
         if ((src_fd = open(existing_file, O_RDONLY | PG_BINARY, 0)) < 0)
-            pg_fatal("could not open file \"%s\": %s",
-                     existing_file, strerror(errno));
+            pg_fatal("could not open file \"%s\": %m",
+                     existing_file);
 
         if ((dest_fd = open(new_link_file, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
                             pg_file_create_mode)) < 0)
-            pg_fatal("could not create file \"%s\": %s",
-                     new_link_file, strerror(errno));
+            pg_fatal("could not create file \"%s\": %m",
+                     new_link_file);
 
         if (copy_file_range(src_fd, NULL, dest_fd, NULL, SSIZE_MAX, 0) < 0)
-            pg_fatal("could not copy file range between old and new data directories: %s",
-                     strerror(errno));
+            pg_fatal("could not copy file range between old and new data directories: %m");
 
         close(src_fd);
         close(dest_fd);
@@ -447,9 +444,8 @@ check_hard_link(void)
     unlink(new_link_file);        /* might fail */
 
     if (link(existing_file, new_link_file) < 0)
-        pg_fatal("could not create hard link between old and new data directories: %s\n"
-                 "In link mode the old and new data directories must be on the same file system.",
-                 strerror(errno));
+        pg_fatal("could not create hard link between old and new data directories: %m\n"
+                 "In link mode the old and new data directories must be on the same file system.");
 
     unlink(new_link_file);
 }
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index d65153de81..7e3abed098 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -186,8 +186,7 @@ check_loadable_libraries(void)
                 was_load_failure = true;
 
                 if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s",
-                             output_path, strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
                 fprintf(script, _("could not load library \"%s\": %s"),
                         lib,
                         PQerrorMessage(conn));
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 8949c58de8..548ea4e623 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -445,8 +445,7 @@ adjust_data_dir(ClusterInfo *cluster)
 
     if ((output = popen(cmd, "r")) == NULL ||
         fgets(cmd_output, sizeof(cmd_output), output) == NULL)
-        pg_fatal("could not get data directory using %s: %s",
-                 cmd, strerror(errno));
+        pg_fatal("could not get data directory using %s: %m", cmd);
 
     rc = pclose(output);
     if (rc != 0)
@@ -491,16 +490,15 @@ get_sock_dir(ClusterInfo *cluster, bool live_check)
         snprintf(filename, sizeof(filename), "%s/postmaster.pid",
                  cluster->pgdata);
         if ((fp = fopen(filename, "r")) == NULL)
-            pg_fatal("could not open file \"%s\": %s",
-                     filename, strerror(errno));
+            pg_fatal("could not open file \"%s\": %m", filename);
 
         for (lineno = 1;
              lineno <= Max(LOCK_FILE_LINE_PORT, LOCK_FILE_LINE_SOCKET_DIR);
              lineno++)
         {
             if (fgets(line, sizeof(line), fp) == NULL)
-                pg_fatal("could not read line %d from file \"%s\": %s",
-                         lineno, filename, strerror(errno));
+                pg_fatal("could not read line %d from file \"%s\": %m",
+                         lineno, filename);
 
             /* potentially overwrite user-supplied value */
             if (lineno == LOCK_FILE_LINE_PORT)
diff --git a/src/bin/pg_upgrade/parallel.c b/src/bin/pg_upgrade/parallel.c
index ae15457a9d..05313a9b15 100644
--- a/src/bin/pg_upgrade/parallel.c
+++ b/src/bin/pg_upgrade/parallel.c
@@ -124,7 +124,7 @@ parallel_exec_prog(const char *log_file, const char *opt_log_file,
             _exit(!exec_prog(log_file, opt_log_file, true, true, "%s", cmd));
         else if (child < 0)
             /* fork failed */
-            pg_fatal("could not create worker process: %s", strerror(errno));
+            pg_fatal("could not create worker process: %m");
 #else
         /* empty array element are always at the end */
         new_arg = exec_thread_args[parallel_jobs - 1];
@@ -140,7 +140,7 @@ parallel_exec_prog(const char *log_file, const char *opt_log_file,
         child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_exec_prog,
                                         new_arg, 0, NULL);
         if (child == 0)
-            pg_fatal("could not create worker thread: %s", strerror(errno));
+            pg_fatal("could not create worker thread: %m");
 
         thread_handles[parallel_jobs - 1] = child;
 #endif
@@ -232,7 +232,7 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
         }
         else if (child < 0)
             /* fork failed */
-            pg_fatal("could not create worker process: %s", strerror(errno));
+            pg_fatal("could not create worker process: %m");
 #else
         /* empty array element are always at the end */
         new_arg = transfer_thread_args[parallel_jobs - 1];
@@ -250,7 +250,7 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
         child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_transfer_all_new_dbs,
                                         new_arg, 0, NULL);
         if (child == 0)
-            pg_fatal("could not create worker thread: %s", strerror(errno));
+            pg_fatal("could not create worker thread: %m");
 
         thread_handles[parallel_jobs - 1] = child;
 #endif
@@ -291,7 +291,7 @@ reap_child(bool wait_for_child)
 #ifndef WIN32
     child = waitpid(-1, &work_status, wait_for_child ? 0 : WNOHANG);
     if (child == (pid_t) -1)
-        pg_fatal("%s() failed: %s", "waitpid", strerror(errno));
+        pg_fatal("%s() failed: %m", "waitpid");
     if (child == 0)
         return false;            /* no children, or no dead children */
     if (work_status != 0)
@@ -310,7 +310,7 @@ reap_child(bool wait_for_child)
     /* get the result */
     GetExitCodeThread(thread_handles[thread_num], &res);
     if (res != 0)
-        pg_fatal("child worker exited abnormally: %s", strerror(errno));
+        pg_fatal("child worker exited abnormally: %m");
 
     /* dispose of handle to stop leaks */
     CloseHandle(thread_handles[thread_num]);
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index bb261353bd..f6143b6bc4 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -105,8 +105,8 @@ main(int argc, char **argv)
      * output directories with correct permissions.
      */
     if (!GetDataDirectoryCreatePerm(new_cluster.pgdata))
-        pg_fatal("could not read permissions of directory \"%s\": %s",
-                 new_cluster.pgdata, strerror(errno));
+        pg_fatal("could not read permissions of directory \"%s\": %m",
+                 new_cluster.pgdata);
 
     umask(pg_mode_mask);
 
diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c
index a1fc5fec78..1d3054d78b 100644
--- a/src/bin/pg_upgrade/relfilenumber.c
+++ b/src/bin/pg_upgrade/relfilenumber.c
@@ -218,9 +218,8 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
                 if (errno == ENOENT)
                     return;
                 else
-                    pg_fatal("error while checking for file existence \"%s.%s\" (\"%s\" to \"%s\"): %s",
-                             map->nspname, map->relname, old_file, new_file,
-                             strerror(errno));
+                    pg_fatal("error while checking for file existence \"%s.%s\" (\"%s\" to \"%s\"): %m",
+                             map->nspname, map->relname, old_file, new_file);
             }
 
             /* If file is empty, just return */
diff --git a/src/bin/pg_upgrade/tablespace.c b/src/bin/pg_upgrade/tablespace.c
index 4602873219..043e5e721b 100644
--- a/src/bin/pg_upgrade/tablespace.c
+++ b/src/bin/pg_upgrade/tablespace.c
@@ -84,8 +84,8 @@ get_tablespace_paths(void)
                               os_info.old_tablespaces[tblnum]);
             else
                 report_status(PG_FATAL,
-                              "could not stat tablespace directory \"%s\": %s",
-                              os_info.old_tablespaces[tblnum], strerror(errno));
+                              "could not stat tablespace directory \"%s\": %m",
+                              os_info.old_tablespaces[tblnum]);
         }
         if (!S_ISDIR(statBuf.st_mode))
             report_status(PG_FATAL,
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 13b2c0f012..9dc1399f36 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -113,8 +113,7 @@ check_for_data_types_usage(ClusterInfo *cluster,
         {
             found = true;
             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s", output_path,
-                         strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 fprintf(script, "In database: %s\n", active_db->db_name);
@@ -289,8 +288,7 @@ old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode)
             if (!check_mode)
             {
                 if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                    pg_fatal("could not open file \"%s\": %s", output_path,
-                             strerror(errno));
+                    pg_fatal("could not open file \"%s\": %m", output_path);
                 if (!db_used)
                 {
                     PQExpBufferData connectbuf;
@@ -423,8 +421,7 @@ report_extension_updates(ClusterInfo *cluster)
         for (rowno = 0; rowno < ntups; rowno++)
         {
             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-                pg_fatal("could not open file \"%s\": %s", output_path,
-                         strerror(errno));
+                pg_fatal("could not open file \"%s\": %m", output_path);
             if (!db_used)
             {
                 PQExpBufferData connectbuf;
diff --git a/src/common/psprintf.c b/src/common/psprintf.c
index 147a695c86..a063fd26d5 100644
--- a/src/common/psprintf.c
+++ b/src/common/psprintf.c
@@ -115,8 +115,8 @@ pvsnprintf(char *buf, size_t len, const char *fmt, va_list args)
 #ifndef FRONTEND
         elog(ERROR, "vsnprintf failed: %m with format string \"%s\"", fmt);
 #else
-        fprintf(stderr, "vsnprintf failed: %s with format string \"%s\"\n",
-                strerror(errno), fmt);
+        fprintf(stderr, "vsnprintf failed: %m with format string \"%s\"\n",
+                fmt);
         exit(EXIT_FAILURE);
 #endif
     }
diff --git a/src/interfaces/ecpg/preproc/ecpg.c b/src/interfaces/ecpg/preproc/ecpg.c
index e4db21e0c1..93e66fc60f 100644
--- a/src/interfaces/ecpg/preproc/ecpg.c
+++ b/src/interfaces/ecpg/preproc/ecpg.c
@@ -216,8 +216,8 @@ main(int argc, char *const argv[])
 
                 if (base_yyout == NULL)
                 {
-                    fprintf(stderr, _("%s: could not open file \"%s\": %s\n"),
-                            progname, output_filename, strerror(errno));
+                    fprintf(stderr, _("%s: could not open file \"%s\": %m\n"),
+                            progname, output_filename);
                     output_filename = NULL;
                 }
                 else
@@ -331,8 +331,8 @@ main(int argc, char *const argv[])
                     base_yyout = fopen(output_filename, PG_BINARY_W);
                     if (base_yyout == NULL)
                     {
-                        fprintf(stderr, _("%s: could not open file \"%s\": %s\n"),
-                                progname, output_filename, strerror(errno));
+                        fprintf(stderr, _("%s: could not open file \"%s\": %m\n"),
+                                progname, output_filename);
                         free(output_filename);
                         output_filename = NULL;
                         free(input_filename);
@@ -342,8 +342,8 @@ main(int argc, char *const argv[])
             }
 
             if (base_yyin == NULL)
-                fprintf(stderr, _("%s: could not open file \"%s\": %s\n"),
-                        progname, argv[fnr], strerror(errno));
+                fprintf(stderr, _("%s: could not open file \"%s\": %m\n"),
+                        progname, argv[fnr]);
             else
             {
                 struct cursor *ptr;
diff --git a/src/port/path.c b/src/port/path.c
index 565871c637..330b3f9033 100644
--- a/src/port/path.c
+++ b/src/port/path.c
@@ -772,8 +772,7 @@ make_absolute_path(const char *path)
 #ifndef FRONTEND
                 elog(ERROR, "could not get current working directory: %m");
 #else
-                fprintf(stderr, _("could not get current working directory: %s\n"),
-                        strerror(errno));
+                fprintf(stderr, _("could not get current working directory: %m\n"));
                 return NULL;
 #endif
             }
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 0a66235153..ed110f740f 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -871,7 +871,7 @@ try_complete_step(TestSpec *testspec, PermutationStep *pstep, int flags)
         {
             if (errno == EINTR)
                 continue;
-            fprintf(stderr, "select failed: %s\n", strerror(errno));
+            fprintf(stderr, "select failed: %m\n");
             exit(1);
         }
         else if (ret == 0)        /* select() timeout: check for lock wait */
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 5f43aa40de..297e726d50 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -324,7 +324,7 @@ test_nosync(PGconn *conn)
         tv.tv_usec = 0;
         if (select(sock + 1, &input_mask, NULL, NULL, &tv) < 0)
         {
-            fprintf(stderr, "select() failed: %s\n", strerror(errno));
+            fprintf(stderr, "select() failed: %m\n");
             exit_nicely(conn);
         }
         if (FD_ISSET(sock, &input_mask) && PQconsumeInput(conn) != 1)
@@ -775,7 +775,7 @@ test_pipelined_insert(PGconn *conn, int n_rows)
 
         if (select(sock + 1, &input_mask, &output_mask, NULL, NULL) < 0)
         {
-            fprintf(stderr, "select() failed: %s\n", strerror(errno));
+            fprintf(stderr, "select() failed: %m\n");
             exit_nicely(conn);
         }
 
diff --git a/src/tools/ifaddrs/test_ifaddrs.c b/src/tools/ifaddrs/test_ifaddrs.c
index b9a1b7b5e8..a1037e1b57 100644
--- a/src/tools/ifaddrs/test_ifaddrs.c
+++ b/src/tools/ifaddrs/test_ifaddrs.c
@@ -66,6 +66,6 @@ main(int argc, char *argv[])
 #endif
 
     if (pg_foreach_ifaddr(callback, NULL) < 0)
-        fprintf(stderr, "pg_foreach_ifaddr failed: %s\n", strerror(errno));
+        fprintf(stderr, "pg_foreach_ifaddr failed: %m\n");
     return 0;
 }
-- 
2.39.2

From ba726cc6e9ac7c2501565df4f1c528aafd4facd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 11 Mar 2024 11:08:14 +0000
Subject: [PATCH v2 2/2] Save errno in emit_tap_output_v() and use %m in
 callers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This defends aganst the fprintf() calls before vprintf(…, fmt, …)
clobbering errno, thus breaking %m.
---
 src/test/regress/pg_regress.c | 77 ++++++++++++-----------------------
 1 file changed, 27 insertions(+), 50 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index f1f6011ae0..08d4101877 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -341,6 +341,7 @@ emit_tap_output_v(TAPtype type, const char *fmt, va_list argp)
 {
     va_list        argp_logfile;
     FILE       *fp;
+    int            save_errno = errno;
 
     /*
      * Diagnostic output will be hidden by prove unless printed to stderr. The
@@ -379,9 +380,13 @@ emit_tap_output_v(TAPtype type, const char *fmt, va_list argp)
         if (logfile)
             fprintf(logfile, "# ");
     }
+    errno = save_errno;
     vfprintf(fp, fmt, argp);
     if (logfile)
+    {
+        errno = save_errno;
         vfprintf(logfile, fmt, argp_logfile);
+    }
 
     /*
      * If we are entering into a note with more details to follow, register
@@ -492,10 +497,7 @@ make_temp_sockdir(void)
 
     temp_sockdir = mkdtemp(template);
     if (temp_sockdir == NULL)
-    {
-        bail("could not create directory \"%s\": %s",
-             template, strerror(errno));
-    }
+        bail("could not create directory \"%s\": %m", template);
 
     /* Stage file names for remove_temp().  Unsafe in a signal handler. */
     UNIXSOCK_PATH(sockself, port, temp_sockdir);
@@ -616,8 +618,7 @@ load_resultmap(void)
         /* OK if it doesn't exist, else complain */
         if (errno == ENOENT)
             return;
-        bail("could not open file \"%s\" for reading: %s",
-             buf, strerror(errno));
+        bail("could not open file \"%s\" for reading: %m", buf);
     }
 
     while (fgets(buf, sizeof(buf), f))
@@ -1046,10 +1047,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
 #define CW(cond)    \
     do { \
         if (!(cond)) \
-        { \
-            bail("could not write to file \"%s\": %s", \
-                 fname, strerror(errno)); \
-        } \
+            bail("could not write to file \"%s\": %m", fname); \
     } while (0)
 
     res = snprintf(fname, sizeof(fname), "%s/pg_hba.conf", pgdata);
@@ -1064,8 +1062,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
     hba = fopen(fname, "w");
     if (hba == NULL)
     {
-        bail("could not open file \"%s\" for writing: %s",
-             fname, strerror(errno));
+        bail("could not open file \"%s\" for writing: %m", fname);
     }
     CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
     CW(fputs("host all all 127.0.0.1/32  sspi include_realm=1 map=regress\n",
@@ -1079,8 +1076,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
     ident = fopen(fname, "w");
     if (ident == NULL)
     {
-        bail("could not open file \"%s\" for writing: %s",
-             fname, strerror(errno));
+        bail("could not open file \"%s\" for writing: %m", fname);
     }
     CW(fputs("# Configuration written by config_sspi_auth()\n", ident) >= 0);
 
@@ -1211,7 +1207,7 @@ spawn_process(const char *cmdline)
     pid = fork();
     if (pid == -1)
     {
-        bail("could not fork: %s", strerror(errno));
+        bail("could not fork: %m");
     }
     if (pid == 0)
     {
@@ -1227,7 +1223,7 @@ spawn_process(const char *cmdline)
         cmdline2 = psprintf("exec %s", cmdline);
         execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
         /* Not using the normal bail() here as we want _exit */
-        bail_noatexit("could not exec \"%s\": %s", shellprog, strerror(errno));
+        bail_noatexit("could not exec \"%s\": %m", shellprog);
     }
     /* in parent */
     return pid;
@@ -1263,8 +1259,7 @@ file_size(const char *file)
 
     if (!f)
     {
-        diag("could not open file \"%s\" for reading: %s",
-             file, strerror(errno));
+        diag("could not open file \"%s\" for reading: %m", file);
         return -1;
     }
     fseek(f, 0, SEEK_END);
@@ -1285,8 +1280,7 @@ file_line_count(const char *file)
 
     if (!f)
     {
-        diag("could not open file \"%s\" for reading: %s",
-             file, strerror(errno));
+        diag("could not open file \"%s\" for reading: %m", file);
         return -1;
     }
     while ((c = fgetc(f)) != EOF)
@@ -1326,9 +1320,7 @@ static void
 make_directory(const char *dir)
 {
     if (mkdir(dir, S_IRWXU | S_IRWXG | S_IRWXO) < 0)
-    {
-        bail("could not create directory \"%s\": %s", dir, strerror(errno));
-    }
+        bail("could not create directory \"%s\": %m", dir);
 }
 
 /*
@@ -1457,10 +1449,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
 
         alt_expectfile = get_alternative_expectfile(expectfile, i);
         if (!alt_expectfile)
-        {
-            bail("Unable to check secondary comparison files: %s",
-                 strerror(errno));
-        }
+            bail("Unable to check secondary comparison files: %m");
 
         if (!file_exists(alt_expectfile))
         {
@@ -1573,9 +1562,7 @@ wait_for_tests(PID_TYPE * pids, int *statuses, instr_time *stoptimes,
         p = wait(&exit_status);
 
         if (p == INVALID_PID)
-        {
-            bail("failed to wait for subprocesses: %s", strerror(errno));
-        }
+            bail("failed to wait for subprocesses: %m");
 #else
         DWORD        exit_status;
         int            r;
@@ -1665,10 +1652,7 @@ run_schedule(const char *schedule, test_start_function startfunc,
 
     scf = fopen(schedule, "r");
     if (!scf)
-    {
-        bail("could not open file \"%s\" for reading: %s",
-             schedule, strerror(errno));
-    }
+        bail("could not open file \"%s\" for reading: %m", schedule);
 
     while (fgets(scbuf, sizeof(scbuf), scf))
     {
@@ -1932,20 +1916,15 @@ open_result_files(void)
     logfilename = pg_strdup(file);
     logfile = fopen(logfilename, "w");
     if (!logfile)
-    {
-        bail("could not open file \"%s\" for writing: %s",
-             logfilename, strerror(errno));
-    }
+        bail("could not open file \"%s\" for writing: %m", logfilename);
 
     /* create the diffs file as empty */
     snprintf(file, sizeof(file), "%s/regression.diffs", outputdir);
     difffilename = pg_strdup(file);
     difffile = fopen(difffilename, "w");
     if (!difffile)
-    {
-        bail("could not open file \"%s\" for writing: %s",
-             difffilename, strerror(errno));
-    }
+        bail("could not open file \"%s\" for writing: %m", difffilename);
+
     /* we don't keep the diffs file open continuously */
     fclose(difffile);
 
@@ -2407,10 +2386,8 @@ regression_main(int argc, char *argv[],
         snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_instance);
         pg_conf = fopen(buf, "a");
         if (pg_conf == NULL)
-        {
-            bail("could not open \"%s\" for adding extra config: %s",
-                 buf, strerror(errno));
-        }
+            bail("could not open \"%s\" for adding extra config: %m", buf);
+
         fputs("\n# Configuration added by pg_regress\n\n", pg_conf);
         fputs("log_autovacuum_min_duration = 0\n", pg_conf);
         fputs("log_checkpoints = on\n", pg_conf);
@@ -2428,8 +2405,8 @@ regression_main(int argc, char *argv[],
             extra_conf = fopen(temp_config, "r");
             if (extra_conf == NULL)
             {
-                bail("could not open \"%s\" to read extra config: %s",
-                     temp_config, strerror(errno));
+                bail("could not open \"%s\" to read extra config: %m",
+                     temp_config);
             }
             while (fgets(line_buf, sizeof(line_buf), extra_conf) != NULL)
                 fputs(line_buf, pg_conf);
@@ -2504,7 +2481,7 @@ regression_main(int argc, char *argv[],
                  outputdir);
         postmaster_pid = spawn_process(buf);
         if (postmaster_pid == INVALID_PID)
-            bail("could not spawn postmaster: %s", strerror(errno));
+            bail("could not spawn postmaster: %m");
 
         /*
          * Wait till postmaster is able to accept connections; normally takes
@@ -2567,7 +2544,7 @@ regression_main(int argc, char *argv[],
              */
 #ifndef WIN32
             if (kill(postmaster_pid, SIGKILL) != 0 && errno != ESRCH)
-                bail("could not kill failed postmaster: %s", strerror(errno));
+                bail("could not kill failed postmaster: %m");
 #else
             if (TerminateProcess(postmaster_pid, 255) == 0)
                 bail("could not kill failed postmaster: error code %lu",
-- 
2.39.2


pgsql-hackers by date:

Previous
From: Laurenz Albe
Date:
Subject: Re: Reducing the log spam
Next
From: Alexander Korotkov
Date:
Subject: Re: POC, WIP: OR-clause support for indexes