Replace use of sys_siglist[] with strsignal().

This commit back-patches the v12-era commits a73d08319, cc92cca43,
and 7570df0f3 into supported pre-v12 branches.  The net effect is to
eliminate our former dependency on the never-standard sys_siglist[]
array, instead using POSIX-standard strsignal(3).

What motivates doing this now is that glibc just removed sys_siglist[]
from the set of symbols available to newly-built programs.  While our
code can survive without sys_siglist[], it then fails to print any
description of the signal that killed a child process, which is a
non-negligible loss of friendliness.  We can expect that people will
be wanting to build the back branches on platforms that include this
change, so we need to do something.

Since strsignal(3) has existed for quite a long time, and we've not
had any trouble with these patches so far in v12, it seems safe to
back-patch into older branches.

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

diff --git a/configure b/configure
index fda3f9dc1d14d4c267f0842a8396d64f4fecf439..d068a28760be194905be01f2b7b12c05e7e1cbf8 100755 (executable)
--- a/configure
+++ b/configure
@@ -12584,7 +12584,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower uselocale utime utimes wcstombs wcstombs_l
+for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask strsignal symlink sync_file_range towlower uselocale utime utimes wcstombs 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"
@@ -13476,25 +13476,6 @@ $as_echo "#define HAVE_SIGSETJMP 1" >>confdefs.h
 
 fi
 
-ac_fn_c_check_decl "$LINENO" "sys_siglist" "ac_cv_have_decl_sys_siglist" "#include <signal.h>
-/* NetBSD declares sys_siglist in unistd.h.  */
-#ifdef HAVE_UNISTD_H
-# include <unistd.h>
-#endif
-
-"
-if test "x$ac_cv_have_decl_sys_siglist" = xyes; then :
-  ac_have_decl=1
-else
-  ac_have_decl=0
-fi
-
-cat >>confdefs.h <<_ACEOF
-#define HAVE_DECL_SYS_SIGLIST $ac_have_decl
-_ACEOF
-
-
-
 ac_fn_c_check_func "$LINENO" "syslog" "ac_cv_func_syslog"
 if test "x$ac_cv_func_syslog" = xyes; then :
   ac_fn_c_check_header_mongrel "$LINENO" "syslog.h" "ac_cv_header_syslog_h" "$ac_includes_default"

diff --git a/configure.in b/configure.in
index 2931804c687d6960fe31b521067d3f6c70bb4840..5cd388a12da558a98a4c1455ad83b02eee9c5e99 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1483,6 +1483,7 @@ AC_CHECK_FUNCS(m4_normalize([
    setsid
    shm_open
    sigprocmask
+   strsignal
    symlink
    sync_file_range
    towlower
@@ -1695,8 +1696,6 @@ if test x"$pgac_cv_func_sigsetjmp" = x"yes"; then
   AC_DEFINE(HAVE_SIGSETJMP, 1, [Define to 1 if you have sigsetjmp().])
 fi
 
-AC_DECL_SYS_SIGLIST
-
 AC_CHECK_FUNC(syslog,
               [AC_CHECK_HEADER(syslog.h,
                                [AC_DEFINE(HAVE_SYSLOG, 1, [Define to 1 if you have the syslog interface.])])])

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 382fc65bb09473f052eef2cb5ae002ed03d02ac4..9691649ac7d25dd6d7262c6c4c62d9408a09ea46 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -598,17 +598,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

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 28ec91f201b212873891a13dbaf226215a951cd3..1bcdf28b0108319d75986a77e1d9c294bfaac1e7 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3528,6 +3528,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,
 
@@ -3538,7 +3539,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,
 
    /*------
@@ -3546,19 +3547,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,
 

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 49417084c4326906069451fb758fa2089b28e7b8..0ffcddd931675bb06e9858512dad2a400b3b833f 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1848,7 +1848,7 @@ BaseBackup(void)
    {
 #ifndef WIN32
        int         status;
-       int         r;
+       pid_t       r;
 #else
        DWORD       status;
        /*
@@ -1875,7 +1875,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));
@@ -1884,19 +1884,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! */

diff --git a/src/common/wait_error.c b/src/common/wait_error.c
index 02675f86877a1ce1052821e958ee2b355e284c25..4c7a3cdba43cc778096aeaf768d44a46468f3055 100644 (file)
--- a/src/common/wait_error.c
+++ b/src/common/wait_error.c
@@ -58,25 +58,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"),

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index a4442db53a656afb8fe25d5b80e887450b208395..36198ea54c4ce9a5225c3d71f4d76c7ad5ff60fd 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -152,10 +152,6 @@
    don't. */
 #undef HAVE_DECL_STRTOULL
 
-/* Define to 1 if you have the declaration of `sys_siglist', and to 0 if you
-   don't. */
-#undef HAVE_DECL_SYS_SIGLIST
-
 /* Define to 1 if you have the declaration of `vsnprintf', and to 0 if you
    don't. */
 #undef HAVE_DECL_VSNPRINTF
@@ -520,6 +516,9 @@
 /* Define to 1 if you have the `strlcpy' function. */
 #undef HAVE_STRLCPY
 
+/* Define to 1 if you have the `strsignal' function. */
+#undef HAVE_STRSIGNAL
+
 /* Define to 1 if you have the `strtoll' function. */
 #undef HAVE_STRTOLL
 

diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index a7ce3cc377b9317c9439f9165b9cb8094329b323..06c2a7c3367a494c1d16d001308983656be2ea6b 100644 (file)
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -373,6 +373,9 @@
 /* Define to 1 if you have the <string.h> header file. */
 #define HAVE_STRING_H 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

diff --git a/src/include/port.h b/src/include/port.h
index 4d5162551e6ecf30e97c9f20affd8c034143ab19..5c89e87c7682328ade98e603fecfa56f6149111d 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -201,6 +201,9 @@ extern char *pgwin32_setlocale(int category, const char *locale);
 #define setlocale(a,b) pgwin32_setlocale(a,b)
 #endif   /* WIN32 */
 
+/* Wrap strsignal(), or provide our own version if necessary */
+extern const char *pg_strsignal(int signum);
+
 /* Portable prompt handling */
 extern char *simple_prompt(const char *prompt, int maxlen, bool echo);
 

diff --git a/src/port/Makefile b/src/port/Makefile
index bc9b63add0479459d146550c0338e1a22a478400..0553aaf09e90341c3c6a4733153283f5d1e74880 100644 (file)
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -32,7 +32,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 sprompt.o tar.o thread.o
 
 # foo_srv.o and foo.o are both built from foo.c, but only foo.o has -DFRONTEND

diff --git a/src/port/pgstrsignal.c b/src/port/pgstrsignal.c
new file mode 100644 (file)
index 0000000..7724edf
--- /dev/null
+++ b/src/port/pgstrsignal.c
@@ -0,0 +1,64 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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() reputedly do not.
+ *
+ * Note that the fallback cases just return constant strings such as
+ * "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 that.
+ */
+const char *
+pg_strsignal(int signum)
+{
+   const char *result;
+
+   /*
+    * If we have strsignal(3), use that --- but check its result for NULL.
+    */
+#ifdef HAVE_STRSIGNAL
+   result = strsignal(signum);
+   if (result == NULL)
+       result = "unrecognized signal";
+#else
+
+   /*
+    * We used to have code here to try to use sys_siglist[] if available.
+    * However, it seems that all platforms with sys_siglist[] have also had
+    * strsignal() for many years now, so that was just a waste of code.
+    */
+   result = "(signal names not available on this platform)";
+#endif
+
+   return result;
+}

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 40681afe6e36ccbc872ee9404fdfded470d5f661..3c8f8622e58fee07a537b49cda01c9a49a99b9cb 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1534,14 +1534,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