Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 85230a2

Browse 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

File tree

1 file changed

+34
-50
lines changed

1 file changed

+34
-50
lines changed

src/test/regress/pg_regress.c

+34-50
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)