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 e69106c86e34e7775d3283a4a0bbffca027b9125..986d3906a9556a5509b180064634d9551b78a3ba 100755 (executable)
--- a/configure
+++ b/configure
@@ -13256,7 +13256,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 dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower uselocale utime utimes wcstombs wcstombs_l
+for ac_func in cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open 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"
@@ -14122,24 +14122,6 @@ esac
 
 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 95580ae7ee3580d7e83dc8511e07aee7466a1d26..22dfc3b7a4fcdd2abecf3fd0c53e05fd10f285a7 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1516,6 +1516,7 @@ AC_CHECK_FUNCS(m4_normalize([
    setproctitle
    setsid
    shm_open
+   strsignal
    symlink
    sync_file_range
    towlower
@@ -1717,14 +1718,6 @@ if test "$PORTNAME" = "cygwin"; then
   AC_LIBOBJ(dirmod)
 fi
 
-AC_CHECK_DECLS([sys_siglist], [], [],
-[#include <signal.h>
-/* NetBSD declares sys_siglist in unistd.h.  */
-#ifdef HAVE_UNISTD_H
-# include <unistd.h>
-#endif
-])
-
 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 dc2739bf36db4bbc318a82dba42f8e34d58b68ef..1d2bedf8beebca5daa0e28d39d9d59726aef245d 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -596,17 +596,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 5c1215b0a121e9368835f52a17b09e611c7f5b49..9edf412d96a502d3ec9822e865902992f5e2bb6e 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3611,6 +3611,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,
 
@@ -3621,7 +3622,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,
 
        /*------
@@ -3629,19 +3630,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 2a3e33eb08eaff72260626c20b70fc0d139c7e97..9793a08cc0cd69cdd20f70c0c89318a623762756 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1965,7 +1965,7 @@ BaseBackup(void)
    {
 #ifndef WIN32
        int         status;
-       int         r;
+       pid_t       r;
 #else
        DWORD       status;
 
@@ -1993,7 +1993,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));
@@ -2002,19 +2002,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 4fb5276961d3255d9da967da507bd9b622464426..cb5df8c89eb96fd43cf90cbb6e4ace12f870e5cd 100644 (file)
--- a/src/common/wait_error.c
+++ b/src/common/wait_error.c
@@ -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"),

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 3c82718bf99e7e63d0f91e32354b5630459afc9c..a27eb57048c24009d24388d6a86b2ae35c089275 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -158,10 +158,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
@@ -511,6 +507,9 @@
 /* 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
 

diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index f84077afa03eea9ca7348004c35d20c40c2c2d2f..720653d588111652c29febeba81ca8595a9c0f67 100644 (file)
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -364,6 +364,9 @@
 /* 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

diff --git a/src/include/port.h b/src/include/port.h
index 80a3556889d293a82fb267fea7702165074eb105..f2b966fdca57b90ed18aac9722184b8bde1541e1 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -205,6 +205,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 void simple_prompt(const char *prompt, char *destination, size_t destlen,
              bool echo);

diff --git a/src/port/Makefile b/src/port/Makefile
index 81f01b25bbc6949df930de17a198107282ccd375..d20999a0b5fad815b195379a971c9ef17d9bcce4 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
 
 ifeq ($(enable_strong_random), yes)

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 5d08d1e12e5ecc46012b905b30e6be321794b905..38b53d2aa3e7f696ef71192e758be6702088798a 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1559,14 +1559,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