]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Replace use of sys_siglist[] with strsignal().
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Jul 2020 02:05:13 +0000 (22:05 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 16 Jul 2020 02:05:13 +0000 (22:05 -0400)
This commit back-patches the v12-era commits a73d08319cc92cca43,
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

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 23f3b81fa9f6cf0d59fdfb4cbdfd139464fd799c..013b0615a6e644b9b32121b65da3eea6545bd55a 100755 (executable)
--- a/configure
+++ b/configure
@@ -12658,7 +12658,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 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 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"
@@ -13524,24 +13524,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"
index 76097f467d3b7c641be3771122ffb46fc6b66e31..e8df59b8d9321823ef8914ec26e9574c74cfd959 100644 (file)
@@ -1523,6 +1523,7 @@ AC_CHECK_FUNCS(m4_normalize([
        setproctitle
        setsid
        shm_open
+       strsignal
        symlink
        sync_file_range
        towlower
@@ -1724,14 +1725,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.])])])
index 14350fb7dec0244784624898552cffda48311277..500fad75b71047d8d6ce2643ffbea8f2ebf45414 100644 (file)
@@ -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
index 17feb6bef94d569f5da1bc87d6b4ab7920e12d72..b8b0d65955abdf98c955f6795ef0f939835ea90e 100644 (file)
@@ -3558,6 +3558,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,
 
@@ -3568,7 +3569,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,
 
        /*------
@@ -3576,19 +3577,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 37ffa55d1215980f2a93d36ca895dee4d7c2a5d9..11fe505615cc578818258adef558c5b12ab8d744 100644 (file)
@@ -1866,7 +1866,7 @@ BaseBackup(void)
        {
 #ifndef WIN32
                int                     status;
-               int                     r;
+               pid_t           r;
 #else
                DWORD           status;
 
@@ -1894,7 +1894,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));
@@ -1903,19 +1903,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 4aa827d0664dae8aed5e941293430dfdb0cd5b7e..e129c778d02efd4d823cf4d97b852f1d0895ed11 100644 (file)
@@ -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"),
index 639c18ab57933d4eca3e014698aba474e6f46abe..adb0d336a336e2a509fa801097e5fe41af824228 100644 (file)
    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
 /* 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
 
index c640ebd2eb6744e7ec3255949c69f00da87fdf7d..28aac57cb58b1e5440914e4d3e2a9c55d99d1d76 100644 (file)
 /* 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
index d0aebe1958381edc5c8f8800199ac59e970fdd57..a3e1dac66cc03b7a0cc93b2fa206cada78f6e08e 100644 (file)
@@ -202,6 +202,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);
 
index bc9b63add0479459d146550c0338e1a22a478400..0553aaf09e90341c3c6a4733153283f5d1e74880 100644 (file)
@@ -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
@@ -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;
+}
index a38a93d78db35ca4cfef608a7eb9b80d888f2ac0..56f277bef699291f9c3d6647e5017e500b397ef1 100644 (file)
@@ -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