Skip to content

Commit 0140dec

Browse files
tglsfdctglsfdc
committed
Replace use of sys_siglist[] with strsignal().
This commit back-patches the v12-era commits a73d083, cc92cca, and 7570df0 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
1 parent 23924fe commit 0140dec

File tree

12 files changed

+94
-84
lines changed

12 files changed

+94
-84
lines changed

‎configure

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13256,7 +13256,7 @@ fi
1325613256
LIBS_including_readline="$LIBS"
1325713257
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
1325813258

13259-
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
13259+
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
1326013260
do :
1326113261
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
1326213262
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -14122,24 +14122,6 @@ esac
1412214122

1412314123
fi
1412414124

14125-
ac_fn_c_check_decl "$LINENO" "sys_siglist" "ac_cv_have_decl_sys_siglist" "#include <signal.h>
14126-
/* NetBSD declares sys_siglist in unistd.h. */
14127-
#ifdef HAVE_UNISTD_H
14128-
# include <unistd.h>
14129-
#endif
14130-
14131-
"
14132-
if test "x$ac_cv_have_decl_sys_siglist" = xyes; then :
14133-
ac_have_decl=1
14134-
else
14135-
ac_have_decl=0
14136-
fi
14137-
14138-
cat >>confdefs.h <<_ACEOF
14139-
#define HAVE_DECL_SYS_SIGLIST $ac_have_decl
14140-
_ACEOF
14141-
14142-
1414314125
ac_fn_c_check_func "$LINENO" "syslog" "ac_cv_func_syslog"
1414414126
if test "x$ac_cv_func_syslog" = xyes; then :
1414514127
ac_fn_c_check_header_mongrel "$LINENO" "syslog.h" "ac_cv_header_syslog_h" "$ac_includes_default"

‎configure.in

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,7 @@ AC_CHECK_FUNCS(m4_normalize([
15161516
setproctitle
15171517
setsid
15181518
shm_open
1519+
strsignal
15191520
symlink
15201521
sync_file_range
15211522
towlower
@@ -1717,14 +1718,6 @@ if test "$PORTNAME" = "cygwin"; then
17171718
AC_LIBOBJ(dirmod)
17181719
fi
17191720

1720-
AC_CHECK_DECLS([sys_siglist], [], [],
1721-
[#include <signal.h>
1722-
/* NetBSD declares sys_siglist in unistd.h. */
1723-
#ifdef HAVE_UNISTD_H
1724-
# include <unistd.h>
1725-
#endif
1726-
])
1727-
17281721
AC_CHECK_FUNC(syslog,
17291722
[AC_CHECK_HEADER(syslog.h,
17301723
[AC_DEFINE(HAVE_SYSLOG, 1, [Define to 1 if you have the syslog interface.])])])

‎src/backend/postmaster/pgarch.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -596,17 +596,10 @@ pgarch_archiveXlog(char *xlog)
596596
errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
597597
errdetail("The failed archive command was: %s",
598598
xlogarchcmd)));
599-
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
600-
ereport(lev,
601-
(errmsg("archive command was terminated by signal %d: %s",
602-
WTERMSIG(rc),
603-
WTERMSIG(rc) < NSIG ? sys_siglist[WTERMSIG(rc)] : "(unknown)"),
604-
errdetail("The failed archive command was: %s",
605-
xlogarchcmd)));
606599
#else
607600
ereport(lev,
608-
(errmsg("archive command was terminated by signal %d",
609-
WTERMSIG(rc)),
601+
(errmsg("archive command was terminated by signal %d: %s",
602+
WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
610603
errdetail("The failed archive command was: %s",
611604
xlogarchcmd)));
612605
#endif

‎src/backend/postmaster/postmaster.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3611,6 +3611,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
36113611
procname, pid, WEXITSTATUS(exitstatus)),
36123612
activity ? errdetail("Failed process was running: %s", activity) : 0));
36133613
else if (WIFSIGNALED(exitstatus))
3614+
{
36143615
#if defined(WIN32)
36153616
ereport(lev,
36163617

@@ -3621,27 +3622,18 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
36213622
procname, pid, WTERMSIG(exitstatus)),
36223623
errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
36233624
activity ? errdetail("Failed process was running: %s", activity) : 0));
3624-
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
3625+
#else
36253626
ereport(lev,
36263627

36273628
/*------
36283629
translator: %s is a noun phrase describing a child process, such as
36293630
"server process" */
36303631
(errmsg("%s (PID %d) was terminated by signal %d: %s",
36313632
procname, pid, WTERMSIG(exitstatus),
3632-
WTERMSIG(exitstatus) < NSIG ?
3633-
sys_siglist[WTERMSIG(exitstatus)] : "(unknown)"),
3634-
activity ? errdetail("Failed process was running: %s", activity) : 0));
3635-
#else
3636-
ereport(lev,
3637-
3638-
/*------
3639-
translator: %s is a noun phrase describing a child process, such as
3640-
"server process" */
3641-
(errmsg("%s (PID %d) was terminated by signal %d",
3642-
procname, pid, WTERMSIG(exitstatus)),
3633+
pg_strsignal(WTERMSIG(exitstatus))),
36433634
activity ? errdetail("Failed process was running: %s", activity) : 0));
36443635
#endif
3636+
}
36453637
else
36463638
ereport(lev,
36473639

‎src/bin/pg_basebackup/pg_basebackup.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1965,7 +1965,7 @@ BaseBackup(void)
19651965
{
19661966
#ifndef WIN32
19671967
int status;
1968-
int r;
1968+
pid_t r;
19691969
#else
19701970
DWORD status;
19711971

@@ -1993,7 +1993,7 @@ BaseBackup(void)
19931993

19941994
/* Just wait for the background process to exit */
19951995
r = waitpid(bgchild, &status, 0);
1996-
if (r == -1)
1996+
if (r == (pid_t) -1)
19971997
{
19981998
fprintf(stderr, _("%s: could not wait for child process: %s\n"),
19991999
progname, strerror(errno));
@@ -2002,19 +2002,13 @@ BaseBackup(void)
20022002
if (r != bgchild)
20032003
{
20042004
fprintf(stderr, _("%s: child %d died, expected %d\n"),
2005-
progname, r, (int) bgchild);
2005+
progname, (int) r, (int) bgchild);
20062006
disconnect_and_exit(1);
20072007
}
2008-
if (!WIFEXITED(status))
2009-
{
2010-
fprintf(stderr, _("%s: child process did not exit normally\n"),
2011-
progname);
2012-
disconnect_and_exit(1);
2013-
}
2014-
if (WEXITSTATUS(status) != 0)
2008+
if (status != 0)
20152009
{
2016-
fprintf(stderr, _("%s: child process exited with error %d\n"),
2017-
progname, WEXITSTATUS(status));
2010+
fprintf(stderr, "%s: %s\n",
2011+
progname, wait_result_to_str(status));
20182012
disconnect_and_exit(1);
20192013
}
20202014
/* Exited normally, we're happy! */

‎src/common/wait_error.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,25 +56,17 @@ wait_result_to_str(int exitstatus)
5656
}
5757
}
5858
else if (WIFSIGNALED(exitstatus))
59+
{
5960
#if defined(WIN32)
6061
snprintf(str, sizeof(str),
6162
_("child process was terminated by exception 0x%X"),
6263
WTERMSIG(exitstatus));
63-
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
64-
{
65-
char str2[256];
66-
67-
snprintf(str2, sizeof(str2), "%d: %s", WTERMSIG(exitstatus),
68-
WTERMSIG(exitstatus) < NSIG ?
69-
sys_siglist[WTERMSIG(exitstatus)] : "(unknown)");
70-
snprintf(str, sizeof(str),
71-
_("child process was terminated by signal %s"), str2);
72-
}
7364
#else
7465
snprintf(str, sizeof(str),
75-
_("child process was terminated by signal %d"),
76-
WTERMSIG(exitstatus));
66+
_("child process was terminated by signal %d: %s"),
67+
WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus)));
7768
#endif
69+
}
7870
else
7971
snprintf(str, sizeof(str),
8072
_("child process exited with unrecognized status %d"),

‎src/include/pg_config.h.in

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,6 @@
158158
don't. */
159159
#undef HAVE_DECL_STRTOULL
160160

161-
/* Define to 1 if you have the declaration of `sys_siglist', and to 0 if you
162-
don't. */
163-
#undef HAVE_DECL_SYS_SIGLIST
164-
165161
/* Define to 1 if you have the declaration of `vsnprintf', and to 0 if you
166162
don't. */
167163
#undef HAVE_DECL_VSNPRINTF
@@ -511,6 +507,9 @@
511507
/* Define to use have a strong random number source */
512508
#undef HAVE_STRONG_RANDOM
513509

510+
/* Define to 1 if you have the `strsignal' function. */
511+
#undef HAVE_STRSIGNAL
512+
514513
/* Define to 1 if you have the `strtoll' function. */
515514
#undef HAVE_STRTOLL
516515

‎src/include/pg_config.h.win32

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@
364364
/* Define to use have a strong random number source */
365365
#define HAVE_STRONG_RANDOM 1
366366

367+
/* Define to 1 if you have the `strsignal' function. */
368+
/* #undef HAVE_STRSIGNAL */
369+
367370
/* Define to 1 if you have the `strtoll' function. */
368371
#ifdef HAVE_LONG_LONG_INT_64
369372
#define HAVE_STRTOLL 1

‎src/include/port.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ extern char *pgwin32_setlocale(int category, const char *locale);
205205
#define setlocale(a,b) pgwin32_setlocale(a,b)
206206
#endif /* WIN32 */
207207

208+
/* Wrap strsignal(), or provide our own version if necessary */
209+
extern const char *pg_strsignal(int signum);
210+
208211
/* Portable prompt handling */
209212
extern void simple_prompt(const char *prompt, char *destination, size_t destlen,
210213
bool echo);

‎src/port/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ LIBS += $(PTHREAD_LIBS)
3232

3333
OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
3434
noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
35-
pgstrcasecmp.o pqsignal.o \
35+
pgstrcasecmp.o pgstrsignal.o pqsignal.o \
3636
qsort.o qsort_arg.o quotes.o sprompt.o tar.o thread.o
3737

3838
ifeq ($(enable_strong_random), yes)

‎src/port/pgstrsignal.c

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* pgstrsignal.c
4+
* Identify a Unix signal number
5+
*
6+
* On platforms compliant with modern POSIX, this just wraps strsignal(3).
7+
* Elsewhere, we do the best we can.
8+
*
9+
* This file is not currently built in MSVC builds, since it's useless
10+
* on non-Unix platforms.
11+
*
12+
* Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
13+
* Portions Copyright (c) 1994, Regents of the University of California
14+
*
15+
* IDENTIFICATION
16+
* src/port/pgstrsignal.c
17+
*
18+
*-------------------------------------------------------------------------
19+
*/
20+
21+
#include "c.h"
22+
23+
24+
/*
25+
* pg_strsignal
26+
*
27+
* Return a string identifying the given Unix signal number.
28+
*
29+
* The result is declared "const char *" because callers should not
30+
* modify the string. Note, however, that POSIX does not promise that
31+
* the string will remain valid across later calls to strsignal().
32+
*
33+
* This version guarantees to return a non-NULL pointer, although
34+
* some platforms' versions of strsignal() reputedly do not.
35+
*
36+
* Note that the fallback cases just return constant strings such as
37+
* "unrecognized signal". Project style is for callers to print the
38+
* numeric signal value along with the result of this function, so
39+
* there's no need to work harder than that.
40+
*/
41+
const char *
42+
pg_strsignal(int signum)
43+
{
44+
const char *result;
45+
46+
/*
47+
* If we have strsignal(3), use that --- but check its result for NULL.
48+
*/
49+
#ifdef HAVE_STRSIGNAL
50+
result = strsignal(signum);
51+
if (result == NULL)
52+
result = "unrecognized signal";
53+
#else
54+
55+
/*
56+
* We used to have code here to try to use sys_siglist[] if available.
57+
* However, it seems that all platforms with sys_siglist[] have also had
58+
* strsignal() for many years now, so that was just a waste of code.
59+
*/
60+
result = "(signal names not available on this platform)";
61+
#endif
62+
63+
return result;
64+
}

‎src/test/regress/pg_regress.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,14 +1559,9 @@ log_child_failure(int exitstatus)
15591559
#if defined(WIN32)
15601560
status(_(" (test process was terminated by exception 0x%X)"),
15611561
WTERMSIG(exitstatus));
1562-
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
1563-
status(_(" (test process was terminated by signal %d: %s)"),
1564-
WTERMSIG(exitstatus),
1565-
WTERMSIG(exitstatus) < NSIG ?
1566-
sys_siglist[WTERMSIG(exitstatus)] : "(unknown))");
15671562
#else
1568-
status(_(" (test process was terminated by signal %d)"),
1569-
WTERMSIG(exitstatus));
1563+
status(_(" (test process was terminated by signal %d: %s)"),
1564+
WTERMSIG(exitstatus), pg_strsignal(WTERMSIG(exitstatus)));
15701565
#endif
15711566
}
15721567
else

0 commit comments

Comments
 (0)