Skip to content

Commit a73d083

Browse files
committed
Modernize our code for looking up descriptive strings for Unix signals.
At least as far back as the 2008 spec, POSIX has defined strsignal(3) for looking up descriptive strings for signal numbers. We hadn't gotten the word though, and were still using the crufty old sys_siglist array, which is in no standard even though most Unixen provide it. Aside from not being formally standards-compliant, this was just plain ugly because it involved #ifdef's at every place using the code. To eliminate the #ifdef's, create a portability function pg_strsignal, which wraps strsignal(3) if available and otherwise falls back to sys_siglist[] if available. The set of Unixen with neither API is probably empty these days, but on any platform with neither, you'll just get "unrecognized signal". All extant callers print the numeric signal number too, so no need to work harder than that. Along the way, upgrade pg_basebackup's child-error-exit reporting to match the rest of the system. Discussion: https://postgr.es/m/25758.1544983503@sss.pgh.pa.us
1 parent 16fda4b commit a73d083

File tree

12 files changed

+97
-54
lines changed

12 files changed

+97
-54
lines changed

configure

+1-1
Original file line numberDiff line numberDiff line change
@@ -15230,7 +15230,7 @@ fi
1523015230
LIBS_including_readline="$LIBS"
1523115231
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
1523215232

15233-
for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul symlink sync_file_range utime utimes wcstombs_l
15233+
for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range utime utimes wcstombs_l
1523415234
do :
1523515235
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
1523615236
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"

configure.in

+1
Original file line numberDiff line numberDiff line change
@@ -1621,6 +1621,7 @@ AC_CHECK_FUNCS(m4_normalize([
16211621
setsid
16221622
shm_open
16231623
strchrnul
1624+
strsignal
16241625
symlink
16251626
sync_file_range
16261627
utime

src/backend/postmaster/pgarch.c

+2-9
Original file line numberDiff line numberDiff line change
@@ -650,17 +650,10 @@ pgarch_archiveXlog(char *xlog)
650650
errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
651651
errdetail("The failed archive command was: %s",
652652
xlogarchcmd)));
653-
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
654-
ereport(lev,
655-
(errmsg("archive command was terminated by signal %d: %s",
656-
WTERMSIG(rc),
657-
WTERMSIG(rc) < NSIG ? sys_siglist[WTERMSIG(rc)] : "(unknown)"),
658-
errdetail("The failed archive command was: %s",
659-
xlogarchcmd)));
660653
#else
661654
ereport(lev,
662-
(errmsg("archive command was terminated by signal %d",
663-
WTERMSIG(rc)),
655+
(errmsg("archive command was terminated by signal %d: %s",
656+
WTERMSIG(rc), pg_strsignal(WTERMSIG(rc))),
664657
errdetail("The failed archive command was: %s",
665658
xlogarchcmd)));
666659
#endif

src/backend/postmaster/postmaster.c

+4-12
Original file line numberDiff line numberDiff line change
@@ -3612,6 +3612,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
36123612
procname, pid, WEXITSTATUS(exitstatus)),
36133613
activity ? errdetail("Failed process was running: %s", activity) : 0));
36143614
else if (WIFSIGNALED(exitstatus))
3615+
{
36153616
#if defined(WIN32)
36163617
ereport(lev,
36173618

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

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

src/bin/pg_basebackup/pg_basebackup.c

+6-12
Original file line numberDiff line numberDiff line change
@@ -2066,7 +2066,7 @@ BaseBackup(void)
20662066
{
20672067
#ifndef WIN32
20682068
int status;
2069-
int r;
2069+
pid_t r;
20702070
#else
20712071
DWORD status;
20722072

@@ -2094,7 +2094,7 @@ BaseBackup(void)
20942094

20952095
/* Just wait for the background process to exit */
20962096
r = waitpid(bgchild, &status, 0);
2097-
if (r == -1)
2097+
if (r == (pid_t) -1)
20982098
{
20992099
fprintf(stderr, _("%s: could not wait for child process: %s\n"),
21002100
progname, strerror(errno));
@@ -2103,19 +2103,13 @@ BaseBackup(void)
21032103
if (r != bgchild)
21042104
{
21052105
fprintf(stderr, _("%s: child %d died, expected %d\n"),
2106-
progname, r, (int) bgchild);
2106+
progname, (int) r, (int) bgchild);
21072107
disconnect_and_exit(1);
21082108
}
2109-
if (!WIFEXITED(status))
2110-
{
2111-
fprintf(stderr, _("%s: child process did not exit normally\n"),
2112-
progname);
2113-
disconnect_and_exit(1);
2114-
}
2115-
if (WEXITSTATUS(status) != 0)
2109+
if (status != 0)
21162110
{
2117-
fprintf(stderr, _("%s: child process exited with error %d\n"),
2118-
progname, WEXITSTATUS(status));
2111+
fprintf(stderr, "%s: %s\n",
2112+
progname, wait_result_to_str(status));
21192113
disconnect_and_exit(1);
21202114
}
21212115
/* Exited normally, we're happy! */

src/common/wait_error.c

+4-12
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

+3
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,9 @@
559559
/* Define to use have a strong random number source */
560560
#undef HAVE_STRONG_RANDOM
561561

562+
/* Define to 1 if you have the `strsignal' function. */
563+
#undef HAVE_STRSIGNAL
564+
562565
/* Define to 1 if you have the `strtoll' function. */
563566
#undef HAVE_STRTOLL
564567

src/include/pg_config.h.win32

+3
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@
415415
/* Define to use have a strong random number source */
416416
#define HAVE_STRONG_RANDOM 1
417417

418+
/* Define to 1 if you have the `strsignal' function. */
419+
/* #undef HAVE_STRSIGNAL */
420+
418421
/* Define to 1 if you have the `strtoll' function. */
419422
#ifdef HAVE_LONG_LONG_INT_64
420423
#define HAVE_STRTOLL 1

src/include/port.h

+3
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ extern char *pg_strerror_r(int errnum, char *buf, size_t buflen);
209209
#define strerror_r pg_strerror_r
210210
#define PG_STRERROR_R_BUFLEN 256 /* Recommended buffer size for strerror_r */
211211

212+
/* Wrap strsignal(), or provide our own version if necessary */
213+
extern const char *pg_strsignal(int signum);
214+
212215
/* Portable prompt handling */
213216
extern void simple_prompt(const char *prompt, char *destination, size_t destlen,
214217
bool echo);

src/port/Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ LIBS += $(PTHREAD_LIBS)
3737

3838
OBJS = $(LIBOBJS) $(PG_CRC32C_OBJS) chklocale.o erand48.o inet_net_ntop.o \
3939
noblock.o path.o pgcheckdir.o pgmkdirp.o pgsleep.o \
40-
pgstrcasecmp.o pqsignal.o \
40+
pgstrcasecmp.o pgstrsignal.o pqsignal.o \
4141
qsort.o qsort_arg.o quotes.o snprintf.o sprompt.o strerror.o \
4242
tar.o thread.o
4343

src/port/pgstrsignal.c

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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() do not.
35+
*/
36+
const char *
37+
pg_strsignal(int signum)
38+
{
39+
const char *result;
40+
41+
/*
42+
* If we have strsignal(3), use that --- but check its result for NULL.
43+
* Otherwise, if we have sys_siglist[], use that; just out of paranoia,
44+
* check for NULL there too. (We assume there is no point in trying both
45+
* APIs.)
46+
*/
47+
#if defined(HAVE_STRSIGNAL)
48+
result = strsignal(signum);
49+
if (result)
50+
return result;
51+
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
52+
if (signum > 0 && signum < NSIG)
53+
{
54+
result = sys_siglist[signum];
55+
if (result)
56+
return result;
57+
}
58+
#endif
59+
60+
/*
61+
* Fallback case: just return "unrecognized signal". Project style is for
62+
* callers to print the numeric signal value along with the result of this
63+
* function, so there's no need to work harder than this.
64+
*/
65+
result = "unrecognized signal";
66+
return result;
67+
}

src/test/regress/pg_regress.c

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

0 commit comments

Comments
 (0)