Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 85230a2

Browse filesBrowse files
committed
pg_regress: Save errno in emit_tap_output_v() and switch to %m
emit_tap_output_v() includes some fprintf() calls for some output related to the TAP protocol, that may clobber errno and break %m. This commit makes the logging of pg_regress smarter by saving errno before restoring it in vfprintf() where the input strings are used, removing the need for strerror(). All logs are switched to %m rather than strerror(), shaving some code. This was not a problem until now as pg_regress.c did not use %m, but the change is simple enough that we have no reason to not support this placeholder, and that will avoid future mistakes if new logs that include %m are added. Author: Dagfinn Ilmari Mannsåker Reviewed-by: Peter Eisentraunt, Michael Paquier Discussion: https://postgr.es/m/87sf13jhuw.fsf@wibble.ilmari.org
1 parent 71b6617 commit 85230a2
Copy full SHA for 85230a2

File tree

Expand file treeCollapse file tree

1 file changed

+34
-50
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+34
-50
lines changed

‎src/test/regress/pg_regress.c

Copy file name to clipboardExpand all lines: src/test/regress/pg_regress.c
+34-50Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,14 @@ emit_tap_output_v(TAPtype type, const char *fmt, va_list argp)
341341
{
342342
va_list argp_logfile;
343343
FILE *fp;
344+
int save_errno;
345+
346+
/*
347+
* The fprintf() calls used to output TAP-protocol elements might clobber
348+
* errno, so save it here and restore it before vfprintf()-ing the user's
349+
* format string, in case it contains %m placeholders.
350+
*/
351+
save_errno = errno;
344352

345353
/*
346354
* Diagnostic output will be hidden by prove unless printed to stderr. The
@@ -379,9 +387,13 @@ emit_tap_output_v(TAPtype type, const char *fmt, va_list argp)
379387
if (logfile)
380388
fprintf(logfile, "# ");
381389
}
390+
errno = save_errno;
382391
vfprintf(fp, fmt, argp);
383392
if (logfile)
393+
{
394+
errno = save_errno;
384395
vfprintf(logfile, fmt, argp_logfile);
396+
}
385397

386398
/*
387399
* If we are entering into a note with more details to follow, register
@@ -492,10 +504,7 @@ make_temp_sockdir(void)
492504

493505
temp_sockdir = mkdtemp(template);
494506
if (temp_sockdir == NULL)
495-
{
496-
bail("could not create directory \"%s\": %s",
497-
template, strerror(errno));
498-
}
507+
bail("could not create directory \"%s\": %m", template);
499508

500509
/* Stage file names for remove_temp(). Unsafe in a signal handler. */
501510
UNIXSOCK_PATH(sockself, port, temp_sockdir);
@@ -616,8 +625,7 @@ load_resultmap(void)
616625
/* OK if it doesn't exist, else complain */
617626
if (errno == ENOENT)
618627
return;
619-
bail("could not open file \"%s\" for reading: %s",
620-
buf, strerror(errno));
628+
bail("could not open file \"%s\" for reading: %m", buf);
621629
}
622630

623631
while (fgets(buf, sizeof(buf), f))
@@ -1046,10 +1054,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
10461054
#define CW(cond) \
10471055
do { \
10481056
if (!(cond)) \
1049-
{ \
1050-
bail("could not write to file \"%s\": %s", \
1051-
fname, strerror(errno)); \
1052-
} \
1057+
bail("could not write to file \"%s\": %m", fname); \
10531058
} while (0)
10541059

10551060
res = snprintf(fname, sizeof(fname), "%s/pg_hba.conf", pgdata);
@@ -1064,8 +1069,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
10641069
hba = fopen(fname, "w");
10651070
if (hba == NULL)
10661071
{
1067-
bail("could not open file \"%s\" for writing: %s",
1068-
fname, strerror(errno));
1072+
bail("could not open file \"%s\" for writing: %m", fname);
10691073
}
10701074
CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
10711075
CW(fputs("host all all 127.0.0.1/32 sspi include_realm=1 map=regress\n",
@@ -1079,8 +1083,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
10791083
ident = fopen(fname, "w");
10801084
if (ident == NULL)
10811085
{
1082-
bail("could not open file \"%s\" for writing: %s",
1083-
fname, strerror(errno));
1086+
bail("could not open file \"%s\" for writing: %m", fname);
10841087
}
10851088
CW(fputs("# Configuration written by config_sspi_auth()\n", ident) >= 0);
10861089

@@ -1210,7 +1213,7 @@ spawn_process(const char *cmdline)
12101213
pid = fork();
12111214
if (pid == -1)
12121215
{
1213-
bail("could not fork: %s", strerror(errno));
1216+
bail("could not fork: %m");
12141217
}
12151218
if (pid == 0)
12161219
{
@@ -1226,7 +1229,7 @@ spawn_process(const char *cmdline)
12261229
cmdline2 = psprintf("exec %s", cmdline);
12271230
execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
12281231
/* Not using the normal bail() here as we want _exit */
1229-
bail_noatexit("could not exec \"%s\": %s", shellprog, strerror(errno));
1232+
bail_noatexit("could not exec \"%s\": %m", shellprog);
12301233
}
12311234
/* in parent */
12321235
return pid;
@@ -1262,8 +1265,7 @@ file_size(const char *file)
12621265

12631266
if (!f)
12641267
{
1265-
diag("could not open file \"%s\" for reading: %s",
1266-
file, strerror(errno));
1268+
diag("could not open file \"%s\" for reading: %m", file);
12671269
return -1;
12681270
}
12691271
fseek(f, 0, SEEK_END);
@@ -1284,8 +1286,7 @@ file_line_count(const char *file)
12841286

12851287
if (!f)
12861288
{
1287-
diag("could not open file \"%s\" for reading: %s",
1288-
file, strerror(errno));
1289+
diag("could not open file \"%s\" for reading: %m", file);
12891290
return -1;
12901291
}
12911292
while ((c = fgetc(f)) != EOF)
@@ -1325,9 +1326,7 @@ static void
13251326
make_directory(const char *dir)
13261327
{
13271328
if (mkdir(dir, S_IRWXU | S_IRWXG | S_IRWXO) < 0)
1328-
{
1329-
bail("could not create directory \"%s\": %s", dir, strerror(errno));
1330-
}
1329+
bail("could not create directory \"%s\": %m", dir);
13311330
}
13321331

13331332
/*
@@ -1456,10 +1455,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
14561455

14571456
alt_expectfile = get_alternative_expectfile(expectfile, i);
14581457
if (!alt_expectfile)
1459-
{
1460-
bail("Unable to check secondary comparison files: %s",
1461-
strerror(errno));
1462-
}
1458+
bail("Unable to check secondary comparison files: %m");
14631459

14641460
if (!file_exists(alt_expectfile))
14651461
{
@@ -1572,9 +1568,7 @@ wait_for_tests(PID_TYPE * pids, int *statuses, instr_time *stoptimes,
15721568
p = wait(&exit_status);
15731569

15741570
if (p == INVALID_PID)
1575-
{
1576-
bail("failed to wait for subprocesses: %s", strerror(errno));
1577-
}
1571+
bail("failed to wait for subprocesses: %m");
15781572
#else
15791573
DWORD exit_status;
15801574
int r;
@@ -1664,10 +1658,7 @@ run_schedule(const char *schedule, test_start_function startfunc,
16641658

16651659
scf = fopen(schedule, "r");
16661660
if (!scf)
1667-
{
1668-
bail("could not open file \"%s\" for reading: %s",
1669-
schedule, strerror(errno));
1670-
}
1661+
bail("could not open file \"%s\" for reading: %m", schedule);
16711662

16721663
while (fgets(scbuf, sizeof(scbuf), scf))
16731664
{
@@ -1931,20 +1922,15 @@ open_result_files(void)
19311922
logfilename = pg_strdup(file);
19321923
logfile = fopen(logfilename, "w");
19331924
if (!logfile)
1934-
{
1935-
bail("could not open file \"%s\" for writing: %s",
1936-
logfilename, strerror(errno));
1937-
}
1925+
bail("could not open file \"%s\" for writing: %m", logfilename);
19381926

19391927
/* create the diffs file as empty */
19401928
snprintf(file, sizeof(file), "%s/regression.diffs", outputdir);
19411929
difffilename = pg_strdup(file);
19421930
difffile = fopen(difffilename, "w");
19431931
if (!difffile)
1944-
{
1945-
bail("could not open file \"%s\" for writing: %s",
1946-
difffilename, strerror(errno));
1947-
}
1932+
bail("could not open file \"%s\" for writing: %m", difffilename);
1933+
19481934
/* we don't keep the diffs file open continuously */
19491935
fclose(difffile);
19501936

@@ -2406,10 +2392,8 @@ regression_main(int argc, char *argv[],
24062392
snprintf(buf, sizeof(buf), "%s/data/postgresql.conf", temp_instance);
24072393
pg_conf = fopen(buf, "a");
24082394
if (pg_conf == NULL)
2409-
{
2410-
bail("could not open \"%s\" for adding extra config: %s",
2411-
buf, strerror(errno));
2412-
}
2395+
bail("could not open \"%s\" for adding extra config: %m", buf);
2396+
24132397
fputs("\n# Configuration added by pg_regress\n\n", pg_conf);
24142398
fputs("log_autovacuum_min_duration = 0\n", pg_conf);
24152399
fputs("log_checkpoints = on\n", pg_conf);
@@ -2427,8 +2411,8 @@ regression_main(int argc, char *argv[],
24272411
extra_conf = fopen(temp_config, "r");
24282412
if (extra_conf == NULL)
24292413
{
2430-
bail("could not open \"%s\" to read extra config: %s",
2431-
temp_config, strerror(errno));
2414+
bail("could not open \"%s\" to read extra config: %m",
2415+
temp_config);
24322416
}
24332417
while (fgets(line_buf, sizeof(line_buf), extra_conf) != NULL)
24342418
fputs(line_buf, pg_conf);
@@ -2503,7 +2487,7 @@ regression_main(int argc, char *argv[],
25032487
outputdir);
25042488
postmaster_pid = spawn_process(buf);
25052489
if (postmaster_pid == INVALID_PID)
2506-
bail("could not spawn postmaster: %s", strerror(errno));
2490+
bail("could not spawn postmaster: %m");
25072491

25082492
/*
25092493
* Wait till postmaster is able to accept connections; normally takes
@@ -2566,7 +2550,7 @@ regression_main(int argc, char *argv[],
25662550
*/
25672551
#ifndef WIN32
25682552
if (kill(postmaster_pid, SIGKILL) != 0 && errno != ESRCH)
2569-
bail("could not kill failed postmaster: %s", strerror(errno));
2553+
bail("could not kill failed postmaster: %m");
25702554
#else
25712555
if (TerminateProcess(postmaster_pid, 255) == 0)
25722556
bail("could not kill failed postmaster: error code %lu",

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.