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 4e1b4be7fb980d6bf0312e59c59f63c52ee05e1b..7382a34d60e3250512b8ad18d47dc32ddc6c26a0 100755 (executable)
--- a/configure
+++ b/configure
@@ -15004,7 +15004,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 uselocale utime utimes 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 uselocale 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"
@@ -15893,24 +15893,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 fab1658bcab5536d6e881eb81cf7362ad1afac45..defcb8ff99974b05325db72c3a1859f0414d9dee 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1622,6 +1622,7 @@ AC_CHECK_FUNCS(m4_normalize([
        setproctitle
        setsid
        shm_open
+       strsignal
        symlink
        sync_file_range
        uselocale
@@ -1821,14 +1822,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 252220c770915759f08d3bcb130f8eb462cbd4d7..08577f5e5f87ee81d8d3411b93b8beede03e342d 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 3bfc299be1bda8cb16878e5d0e07f0d0a759a0ad..75a9e070412a4ed7c162d76feb30f00a4d15816e 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3563,6 +3563,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,
 
@@ -3573,7 +3574,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,
 
                /*------
@@ -3581,19 +3582,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 305dba20cb09c85330aa05d79177921db982e2b8..662ba2bc3e5317c0fb78b9e3738f82a99f22baad 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2014,7 +2014,7 @@ BaseBackup(void)
        {
 #ifndef WIN32
                int                     status;
-               int                     r;
+               pid_t           r;
 #else
                DWORD           status;
 
@@ -2042,7 +2042,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));
@@ -2051,19 +2051,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 27f528499823656e0fc84ebdabd6fbef01dc3a96..cef36ec01a2d467952e6ea7e989c52a9576d6caf 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 a19f6981d74d91b8bb30275d59d884201117f3c6..a33acbf2d891315007f4e6e47a3d66d675e35586 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -182,10 +182,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
@@ -547,6 +543,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 9bc4b3fc0f65f6673cda349658d7bd74268370ad..e48ded8973db181a77eb985d09a26d0fd9d24719 100644 (file)
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -414,6 +414,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 a9d557b55cab17ccb926c434e67a7efd2271fb26..c97d9be2502b69763eda9ebb1fdab7126bdd1260 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -189,6 +189,9 @@ extern int  pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);
 #endif
 #endif                                                 /* USE_REPL_SNPRINTF */
 
+/* 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 d7467fb0788d04b1a57e00930b685288d4e83bc6..ae37898beeaa9beff413b9ccfa41e8809d408934 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 9c6d2efb562756436ca596ebad47d39588ff30e0..2c469413a34bdff6408f2da68a9dc565650a2ac5 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -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