]> granicus.if.org Git - postgresql/commitdiff
Modernize our code for looking up descriptive strings for Unix signals.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 17 Dec 2018 00:38:57 +0000 (19:38 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 17 Dec 2018 00:38:57 +0000 (19:38 -0500)
At least as far back as the 2008 spec, POSIX has defined strsignal(3)
for looking up descriptive strings for signal numbers.  We hadn't gotten
the word though, and were still using the crufty old sys_siglist array,
which is in no standard even though most Unixen provide it.

Aside from not being formally standards-compliant, this was just plain
ugly because it involved #ifdef's at every place using the code.

To eliminate the #ifdef's, create a portability function pg_strsignal,
which wraps strsignal(3) if available and otherwise falls back to
sys_siglist[] if available.  The set of Unixen with neither API is
probably empty these days, but on any platform with neither, you'll
just get "unrecognized signal".  All extant callers print the numeric
signal number too, so no need to work harder than that.

Along the way, upgrade pg_basebackup's child-error-exit reporting
to match the rest of the system.

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

12 files changed:
configure
configure.in
src/backend/postmaster/pgarch.c
src/backend/postmaster/postmaster.c
src/bin/pg_basebackup/pg_basebackup.c
src/common/wait_error.c
src/include/pg_config.h.in
src/include/pg_config.h.win32
src/include/port.h
src/port/Makefile
src/port/pgstrsignal.c [new file with mode: 0644]
src/test/regress/pg_regress.c

index dce6d98cf61f5a8b4db7251eb73cf4687c7f8f61..3dddd8d0ed5f9f20235c104e90da15ddb3bedf16 100755 (executable)
--- a/configure
+++ b/configure
@@ -15230,7 +15230,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul symlink sync_file_range utime utimes wcstombs_l
+for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range utime utimes wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
index e5123ac12219953b3e09db5d6fb30d4c70117bbc..4fd6b8fc80c585a2d330cb012aae06fd52439994 100644 (file)
@@ -1621,6 +1621,7 @@ AC_CHECK_FUNCS(m4_normalize([
        setsid
        shm_open
        strchrnul
+       strsignal
        symlink
        sync_file_range
        utime
index f663d7befaf44365e475614675acdc6d1083baee..e88d545d65b0f4295ff2255913be07acdb24c3f8 100644 (file)
@@ -650,17 +650,10 @@ pgarch_archiveXlog(char *xlog)
                                         errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
                                         errdetail("The failed archive command was: %s",
                                                           xlogarchcmd)));
-#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
-                       ereport(lev,
-                                       (errmsg("archive command was terminated by signal %d: %s",
-                                                       WTERMSIG(rc),
-                                                       WTERMSIG(rc) < NSIG ? sys_siglist[WTERMSIG(rc)] : "(unknown)"),
-                                        errdetail("The failed archive command was: %s",
-                                                          xlogarchcmd)));
 #else
                        ereport(lev,
-                                       (errmsg("archive command was terminated by signal %d",
-                                                       WTERMSIG(rc)),
+                                       (errmsg("archive command was terminated by signal %d: %s",
+                                                       WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
                                         errdetail("The failed archive command was: %s",
                                                           xlogarchcmd)));
 #endif
index a33a131182988b0ca358f4d5b7768e2bf6346834..eedc617db4c4913c83977bc741e05c7768c3147f 100644 (file)
@@ -3612,6 +3612,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
                                                procname, pid, WEXITSTATUS(exitstatus)),
                                 activity ? errdetail("Failed process was running: %s", activity) : 0));
        else if (WIFSIGNALED(exitstatus))
+       {
 #if defined(WIN32)
                ereport(lev,
 
@@ -3622,7 +3623,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
                                                procname, pid, WTERMSIG(exitstatus)),
                                 errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
                                 activity ? errdetail("Failed process was running: %s", activity) : 0));
-#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
+#else
                ereport(lev,
 
                /*------
@@ -3630,19 +3631,10 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
                  "server process" */
                                (errmsg("%s (PID %d) was terminated by signal %d: %s",
                                                procname, pid, WTERMSIG(exitstatus),
-                                               WTERMSIG(exitstatus) < NSIG ?
-                                               sys_siglist[WTERMSIG(exitstatus)] : "(unknown)"),
-                                activity ? errdetail("Failed process was running: %s", activity) : 0));
-#else
-               ereport(lev,
-
-               /*------
-                 translator: %s is a noun phrase describing a child process, such as
-                 "server process" */
-                               (errmsg("%s (PID %d) was terminated by signal %d",
-                                               procname, pid, WTERMSIG(exitstatus)),
+                                               pg_strsignal(WTERMSIG(exitstatus))),
                                 activity ? errdetail("Failed process was running: %s", activity) : 0));
 #endif
+       }
        else
                ereport(lev,
 
index af1c4de58f98767e85992a788c93f8e0a43c05ec..85c3812dd11bb67124687784ffd569df0b2847aa 100644 (file)
@@ -2066,7 +2066,7 @@ BaseBackup(void)
        {
 #ifndef WIN32
                int                     status;
-               int                     r;
+               pid_t           r;
 #else
                DWORD           status;
 
@@ -2094,7 +2094,7 @@ BaseBackup(void)
 
                /* Just wait for the background process to exit */
                r = waitpid(bgchild, &status, 0);
-               if (r == -1)
+               if (r == (pid_t) -1)
                {
                        fprintf(stderr, _("%s: could not wait for child process: %s\n"),
                                        progname, strerror(errno));
@@ -2103,19 +2103,13 @@ BaseBackup(void)
                if (r != bgchild)
                {
                        fprintf(stderr, _("%s: child %d died, expected %d\n"),
-                                       progname, r, (int) bgchild);
+                                       progname, (int) r, (int) bgchild);
                        disconnect_and_exit(1);
                }
-               if (!WIFEXITED(status))
-               {
-                       fprintf(stderr, _("%s: child process did not exit normally\n"),
-                                       progname);
-                       disconnect_and_exit(1);
-               }
-               if (WEXITSTATUS(status) != 0)
+               if (status != 0)
                {
-                       fprintf(stderr, _("%s: child process exited with error %d\n"),
-                                       progname, WEXITSTATUS(status));
+                       fprintf(stderr, "%s: %s\n",
+                                       progname, wait_result_to_str(status));
                        disconnect_and_exit(1);
                }
                /* Exited normally, we're happy! */
index 27f528499823656e0fc84ebdabd6fbef01dc3a96..cef36ec01a2d467952e6ea7e989c52a9576d6caf 100644 (file)
@@ -56,25 +56,17 @@ wait_result_to_str(int exitstatus)
                }
        }
        else if (WIFSIGNALED(exitstatus))
+       {
 #if defined(WIN32)
                snprintf(str, sizeof(str),
                                 _("child process was terminated by exception 0x%X"),
                                 WTERMSIG(exitstatus));
-#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
-       {
-               char            str2[256];
-
-               snprintf(str2, sizeof(str2), "%d: %s", WTERMSIG(exitstatus),
-                                WTERMSIG(exitstatus) < NSIG ?
-                                sys_siglist[WTERMSIG(exitstatus)] : "(unknown)");
-               snprintf(str, sizeof(str),
-                                _("child process was terminated by signal %s"), str2);
-       }
 #else
                snprintf(str, sizeof(str),
-                                _("child process was terminated by signal %d"),
-                                WTERMSIG(exitstatus));
+                                _("child process was terminated by signal %d: %s"),
+                                WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus)));
 #endif
+       }
        else
                snprintf(str, sizeof(str),
                                 _("child process exited with unrecognized status %d"),
index 6ac75cd02cce22e6b5808642aac0deff14343f34..0a5ddd2e92c82f0e7f6c77c79f5266ee2ea2f5c0 100644 (file)
 /* Define to use have a strong random number source */
 #undef HAVE_STRONG_RANDOM
 
+/* Define to 1 if you have the `strsignal' function. */
+#undef HAVE_STRSIGNAL
+
 /* Define to 1 if you have the `strtoll' function. */
 #undef HAVE_STRTOLL
 
index 894d658a204e3a817e854e7dd915aba6417e40f9..de0c4d9997bc7e964b9da3aed00344b9582349dd 100644 (file)
 /* Define to use have a strong random number source */
 #define HAVE_STRONG_RANDOM 1
 
+/* Define to 1 if you have the `strsignal' function. */
+/* #undef HAVE_STRSIGNAL */
+
 /* Define to 1 if you have the `strtoll' function. */
 #ifdef HAVE_LONG_LONG_INT_64
 #define HAVE_STRTOLL 1
index 03e9c12a35b69ada77683d55a8856feb57cb95b1..570a9052a28c75062bfc14b7ff00c0d967f98e09 100644 (file)
@@ -209,6 +209,9 @@ extern char *pg_strerror_r(int errnum, char *buf, size_t buflen);
 #define strerror_r pg_strerror_r
 #define PG_STRERROR_R_BUFLEN 256       /* Recommended buffer size for strerror_r */
 
+/* Wrap strsignal(), or provide our own version if necessary */
+extern const char *pg_strsignal(int signum);
+
 /* Portable prompt handling */
 extern void simple_prompt(const char *prompt, char *destination, size_t destlen,
                          bool echo);
index 585c53757bab6309f15ae2e524b938635c04ea94..ae3f973ae140d3ed1bb7dcbde7cd9d04b2c8995b 100644 (file)
@@ -37,7 +37,7 @@ LIBS += $(PTHREAD_LIBS)
 
 OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
        noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
-       pgstrcasecmp.o pqsignal.o \
+       pgstrcasecmp.o pgstrsignal.o pqsignal.o \
        qsort.o qsort_arg.o quotes.o snprintf.o sprompt.o strerror.o \
        tar.o thread.o
 
diff --git a/src/port/pgstrsignal.c b/src/port/pgstrsignal.c
new file mode 100644 (file)
index 0000000..ec98922
--- /dev/null
@@ -0,0 +1,67 @@
+/*-------------------------------------------------------------------------
+ *
+ * pgstrsignal.c
+ *       Identify a Unix signal number
+ *
+ * On platforms compliant with modern POSIX, this just wraps strsignal(3).
+ * Elsewhere, we do the best we can.
+ *
+ * This file is not currently built in MSVC builds, since it's useless
+ * on non-Unix platforms.
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *       src/port/pgstrsignal.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "c.h"
+
+
+/*
+ * pg_strsignal
+ *
+ * Return a string identifying the given Unix signal number.
+ *
+ * The result is declared "const char *" because callers should not
+ * modify the string.  Note, however, that POSIX does not promise that
+ * the string will remain valid across later calls to strsignal().
+ *
+ * This version guarantees to return a non-NULL pointer, although
+ * some platforms' versions of strsignal() do not.
+ */
+const char *
+pg_strsignal(int signum)
+{
+       const char *result;
+
+       /*
+        * If we have strsignal(3), use that --- but check its result for NULL.
+        * Otherwise, if we have sys_siglist[], use that; just out of paranoia,
+        * check for NULL there too.  (We assume there is no point in trying both
+        * APIs.)
+        */
+#if defined(HAVE_STRSIGNAL)
+       result = strsignal(signum);
+       if (result)
+               return result;
+#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
+       if (signum > 0 && signum < NSIG)
+       {
+               result = sys_siglist[signum];
+               if (result)
+                       return result;
+       }
+#endif
+
+       /*
+        * Fallback case: just return "unrecognized signal".  Project style is for
+        * callers to print the numeric signal value along with the result of this
+        * function, so there's no need to work harder than this.
+        */
+       result = "unrecognized signal";
+       return result;
+}
index 3248603da1966c323804d3995245853d07d6b2e9..63fe68914b8df0595aedd43c48f1884c9753b306 100644 (file)
@@ -1560,14 +1560,9 @@ log_child_failure(int exitstatus)
 #if defined(WIN32)
                status(_(" (test process was terminated by exception 0x%X)"),
                           WTERMSIG(exitstatus));
-#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
-               status(_(" (test process was terminated by signal %d: %s)"),
-                          WTERMSIG(exitstatus),
-                          WTERMSIG(exitstatus) < NSIG ?
-                          sys_siglist[WTERMSIG(exitstatus)] : "(unknown))");
 #else
-               status(_(" (test process was terminated by signal %d)"),
-                          WTERMSIG(exitstatus));
+               status(_(" (test process was terminated by signal %d: %s)"),
+                          WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus)));
 #endif
        }
        else