Skip to content

Commit 7652353

Browse files
tglsfdctglsfdc
committed
Remove trailing newlines in pg_upgrade's message strings.
pg_upgrade does not use common/logging.c, which is unfortunate but changing it to do so seems like more work than is justified. However, we really need to make it work more like common/logging.c in one respect: the latter expects supplied message strings to not end with a newline, instead adding one internally. As it stands, pg_upgrade's logging facilities expect a caller-supplied newline in some cases and not others, which is already an invitation to bugs, but the inconsistency with our other frontend code makes it worse. There are already several places with missing or extra newlines, and it's inevitable that there won't be more if we let this stand. Hence, run around and get rid of all trailing newlines in message strings, and add an Assert that there's not one, similar to the existing Assert in common/logging.c. Adjust the logging functions to supply a newline at the right places. (Some of these strings also have a *leading* newline, which would be a good thing to get rid of too; but this patch doesn't attempt that.) There are some consequent minor changes in output. The ones that aren't outright bug fixes are generally removal of extra blank lines that the original coding intentionally inserted. It didn't seem worth being bug-compatible with that. Patch by me, reviewed by Kyotaro Horiguchi and Peter Eisentraut Discussion: https://postgr.es/m/113191.1655233060@sss.pgh.pa.us
1 parent eea9fa9 commit 7652353

15 files changed

+317
-291
lines changed

‎src/bin/pg_upgrade/check.c

Lines changed: 59 additions & 59 deletions
Large diffs are not rendered by default.

‎src/bin/pg_upgrade/controldata.c

Lines changed: 80 additions & 74 deletions
Large diffs are not rendered by default.

‎src/bin/pg_upgrade/exec.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ get_bin_version(ClusterInfo *cluster)
4242

4343
if ((output = popen(cmd, "r")) == NULL ||
4444
fgets(cmd_output, sizeof(cmd_output), output) == NULL)
45-
pg_fatal("could not get pg_ctl version data using %s: %s\n",
45+
pg_fatal("could not get pg_ctl version data using %s: %s",
4646
cmd, strerror(errno));
4747

4848
pclose(output);
4949

5050
if (sscanf(cmd_output, "%*s %*s %d.%d", &v1, &v2) < 1)
51-
pg_fatal("could not get pg_ctl version output from %s\n", cmd);
51+
pg_fatal("could not get pg_ctl version output from %s", cmd);
5252

5353
if (v1 < 10)
5454
{
@@ -105,13 +105,13 @@ exec_prog(const char *log_filename, const char *opt_log_file,
105105
written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
106106
va_end(ap);
107107
if (written >= MAXCMDLEN)
108-
pg_fatal("command too long\n");
108+
pg_fatal("command too long");
109109
written += snprintf(cmd + written, MAXCMDLEN - written,
110110
" >> \"%s\" 2>&1", log_file);
111111
if (written >= MAXCMDLEN)
112-
pg_fatal("command too long\n");
112+
pg_fatal("command too long");
113113

114-
pg_log(PG_VERBOSE, "%s\n", cmd);
114+
pg_log(PG_VERBOSE, "%s", cmd);
115115

116116
#ifdef WIN32
117117

@@ -150,7 +150,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
150150
#endif
151151

152152
if (log == NULL)
153-
pg_fatal("could not open log file \"%s\": %m\n", log_file);
153+
pg_fatal("could not open log file \"%s\": %m", log_file);
154154

155155
#ifdef WIN32
156156
/* Are we printing "command:" before its output? */
@@ -182,16 +182,16 @@ exec_prog(const char *log_filename, const char *opt_log_file,
182182
report_status(PG_REPORT, "\n*failure*");
183183
fflush(stdout);
184184

185-
pg_log(PG_VERBOSE, "There were problems executing \"%s\"\n", cmd);
185+
pg_log(PG_VERBOSE, "There were problems executing \"%s\"", cmd);
186186
if (opt_log_file)
187187
pg_log(exit_on_error ? PG_FATAL : PG_REPORT,
188188
"Consult the last few lines of \"%s\" or \"%s\" for\n"
189-
"the probable cause of the failure.\n",
189+
"the probable cause of the failure.",
190190
log_file, opt_log_file);
191191
else
192192
pg_log(exit_on_error ? PG_FATAL : PG_REPORT,
193193
"Consult the last few lines of \"%s\" for\n"
194-
"the probable cause of the failure.\n",
194+
"the probable cause of the failure.",
195195
log_file);
196196
}
197197

@@ -205,7 +205,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
205205
* log these commands to a third file, but that just adds complexity.
206206
*/
207207
if ((log = fopen(log_file, "a")) == NULL)
208-
pg_fatal("could not write to log file \"%s\": %m\n", log_file);
208+
pg_fatal("could not write to log file \"%s\": %m", log_file);
209209
fprintf(log, "\n\n");
210210
fclose(log);
211211
#endif
@@ -231,7 +231,7 @@ pid_lock_file_exists(const char *datadir)
231231
{
232232
/* ENOTDIR means we will throw a more useful error later */
233233
if (errno != ENOENT && errno != ENOTDIR)
234-
pg_fatal("could not open file \"%s\" for reading: %s\n",
234+
pg_fatal("could not open file \"%s\" for reading: %s",
235235
path, strerror(errno));
236236

237237
return false;
@@ -258,7 +258,7 @@ verify_directories(void)
258258
#else
259259
if (win32_check_directory_write_permissions() != 0)
260260
#endif
261-
pg_fatal("You must have read and write access in the current directory.\n");
261+
pg_fatal("You must have read and write access in the current directory.");
262262

263263
check_bin_dir(&old_cluster, false);
264264
check_data_dir(&old_cluster);
@@ -311,10 +311,10 @@ check_single_dir(const char *pg_data, const char *subdir)
311311
subdir);
312312

313313
if (stat(subDirName, &statBuf) != 0)
314-
report_status(PG_FATAL, "check for \"%s\" failed: %s\n",
314+
report_status(PG_FATAL, "check for \"%s\" failed: %s",
315315
subDirName, strerror(errno));
316316
else if (!S_ISDIR(statBuf.st_mode))
317-
report_status(PG_FATAL, "\"%s\" is not a directory\n",
317+
report_status(PG_FATAL, "\"%s\" is not a directory",
318318
subDirName);
319319
}
320320

@@ -377,10 +377,10 @@ check_bin_dir(ClusterInfo *cluster, bool check_versions)
377377

378378
/* check bindir */
379379
if (stat(cluster->bindir, &statBuf) != 0)
380-
report_status(PG_FATAL, "check for \"%s\" failed: %s\n",
380+
report_status(PG_FATAL, "check for \"%s\" failed: %s",
381381
cluster->bindir, strerror(errno));
382382
else if (!S_ISDIR(statBuf.st_mode))
383-
report_status(PG_FATAL, "\"%s\" is not a directory\n",
383+
report_status(PG_FATAL, "\"%s\" is not a directory",
384384
cluster->bindir);
385385

386386
check_exec(cluster->bindir, "postgres", check_versions);
@@ -430,16 +430,16 @@ check_exec(const char *dir, const char *program, bool check_version)
430430
ret = validate_exec(path);
431431

432432
if (ret == -1)
433-
pg_fatal("check for \"%s\" failed: not a regular file\n",
433+
pg_fatal("check for \"%s\" failed: does not exist or cannot be executed",
434434
path);
435435
else if (ret == -2)
436-
pg_fatal("check for \"%s\" failed: cannot execute (permission denied)\n",
436+
pg_fatal("check for \"%s\" failed: cannot read (permission denied)",
437437
path);
438438

439439
snprintf(cmd, sizeof(cmd), "\"%s\" -V", path);
440440

441441
if (!pipe_read_line(cmd, line, sizeof(line)))
442-
pg_fatal("check for \"%s\" failed: cannot execute\n",
442+
pg_fatal("check for \"%s\" failed: cannot execute",
443443
path);
444444

445445
if (check_version)
@@ -449,7 +449,7 @@ check_exec(const char *dir, const char *program, bool check_version)
449449
snprintf(versionstr, sizeof(versionstr), "%s (PostgreSQL) " PG_VERSION, program);
450450

451451
if (strcmp(line, versionstr) != 0)
452-
pg_fatal("check for \"%s\" failed: incorrect version: found \"%s\", expected \"%s\"\n",
452+
pg_fatal("check for \"%s\" failed: incorrect version: found \"%s\", expected \"%s\"",
453453
path, line, versionstr);
454454
}
455455
}

‎src/bin/pg_upgrade/file.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,25 @@ cloneFile(const char *src, const char *dst,
4040
{
4141
#if defined(HAVE_COPYFILE) && defined(COPYFILE_CLONE_FORCE)
4242
if (copyfile(src, dst, NULL, COPYFILE_CLONE_FORCE) < 0)
43-
pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
43+
pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
4444
schemaName, relName, src, dst, strerror(errno));
4545
#elif defined(__linux__) && defined(FICLONE)
4646
int src_fd;
4747
int dest_fd;
4848

4949
if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
50-
pg_fatal("error while cloning relation \"%s.%s\": could not open file \"%s\": %s\n",
50+
pg_fatal("error while cloning relation \"%s.%s\": could not open file \"%s\": %s",
5151
schemaName, relName, src, strerror(errno));
5252

5353
if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
5454
pg_file_create_mode)) < 0)
55-
pg_fatal("error while cloning relation \"%s.%s\": could not create file \"%s\": %s\n",
55+
pg_fatal("error while cloning relation \"%s.%s\": could not create file \"%s\": %s",
5656
schemaName, relName, dst, strerror(errno));
5757

5858
if (ioctl(dest_fd, FICLONE, src_fd) < 0)
5959
{
6060
unlink(dst);
61-
pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
61+
pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
6262
schemaName, relName, src, dst, strerror(errno));
6363
}
6464

@@ -84,12 +84,12 @@ copyFile(const char *src, const char *dst,
8484
char *buffer;
8585

8686
if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
87-
pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s\n",
87+
pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s",
8888
schemaName, relName, src, strerror(errno));
8989

9090
if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
9191
pg_file_create_mode)) < 0)
92-
pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n",
92+
pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s",
9393
schemaName, relName, dst, strerror(errno));
9494

9595
/* copy in fairly large chunks for best efficiency */
@@ -103,7 +103,7 @@ copyFile(const char *src, const char *dst,
103103
ssize_t nbytes = read(src_fd, buffer, COPY_BUF_SIZE);
104104

105105
if (nbytes < 0)
106-
pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
106+
pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s",
107107
schemaName, relName, src, strerror(errno));
108108

109109
if (nbytes == 0)
@@ -115,7 +115,7 @@ copyFile(const char *src, const char *dst,
115115
/* if write didn't set errno, assume problem is no disk space */
116116
if (errno == 0)
117117
errno = ENOSPC;
118-
pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s\n",
118+
pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s",
119119
schemaName, relName, dst, strerror(errno));
120120
}
121121
}
@@ -129,7 +129,7 @@ copyFile(const char *src, const char *dst,
129129
if (CopyFile(src, dst, true) == 0)
130130
{
131131
_dosmaperr(GetLastError());
132-
pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
132+
pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
133133
schemaName, relName, src, dst, strerror(errno));
134134
}
135135

@@ -148,7 +148,7 @@ linkFile(const char *src, const char *dst,
148148
const char *schemaName, const char *relName)
149149
{
150150
if (link(src, dst) < 0)
151-
pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
151+
pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
152152
schemaName, relName, src, dst, strerror(errno));
153153
}
154154

@@ -187,16 +187,16 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
187187
rewriteVmBytesPerPage = (BLCKSZ - SizeOfPageHeaderData) / 2;
188188

189189
if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0)
190-
pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s\n",
190+
pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s",
191191
schemaName, relName, fromfile, strerror(errno));
192192

193193
if (fstat(src_fd, &statbuf) != 0)
194-
pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %s\n",
194+
pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %s",
195195
schemaName, relName, fromfile, strerror(errno));
196196

197197
if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
198198
pg_file_create_mode)) < 0)
199-
pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n",
199+
pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s",
200200
schemaName, relName, tofile, strerror(errno));
201201

202202
/* Save old file size */
@@ -220,10 +220,10 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
220220
if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ)
221221
{
222222
if (bytesRead < 0)
223-
pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
223+
pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s",
224224
schemaName, relName, fromfile, strerror(errno));
225225
else
226-
pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"\n",
226+
pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"",
227227
schemaName, relName, fromfile);
228228
}
229229

@@ -298,7 +298,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
298298
/* if write didn't set errno, assume problem is no disk space */
299299
if (errno == 0)
300300
errno = ENOSPC;
301-
pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s\n",
301+
pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s",
302302
schemaName, relName, tofile, strerror(errno));
303303
}
304304

@@ -325,31 +325,31 @@ check_file_clone(void)
325325

326326
#if defined(HAVE_COPYFILE) && defined(COPYFILE_CLONE_FORCE)
327327
if (copyfile(existing_file, new_link_file, NULL, COPYFILE_CLONE_FORCE) < 0)
328-
pg_fatal("could not clone file between old and new data directories: %s\n",
328+
pg_fatal("could not clone file between old and new data directories: %s",
329329
strerror(errno));
330330
#elif defined(__linux__) && defined(FICLONE)
331331
{
332332
int src_fd;
333333
int dest_fd;
334334

335335
if ((src_fd = open(existing_file, O_RDONLY | PG_BINARY, 0)) < 0)
336-
pg_fatal("could not open file \"%s\": %s\n",
336+
pg_fatal("could not open file \"%s\": %s",
337337
existing_file, strerror(errno));
338338

339339
if ((dest_fd = open(new_link_file, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
340340
pg_file_create_mode)) < 0)
341-
pg_fatal("could not create file \"%s\": %s\n",
341+
pg_fatal("could not create file \"%s\": %s",
342342
new_link_file, strerror(errno));
343343

344344
if (ioctl(dest_fd, FICLONE, src_fd) < 0)
345-
pg_fatal("could not clone file between old and new data directories: %s\n",
345+
pg_fatal("could not clone file between old and new data directories: %s",
346346
strerror(errno));
347347

348348
close(src_fd);
349349
close(dest_fd);
350350
}
351351
#else
352-
pg_fatal("file cloning not supported on this platform\n");
352+
pg_fatal("file cloning not supported on this platform");
353353
#endif
354354

355355
unlink(new_link_file);
@@ -367,7 +367,7 @@ check_hard_link(void)
367367

368368
if (link(existing_file, new_link_file) < 0)
369369
pg_fatal("could not create hard link between old and new data directories: %s\n"
370-
"In link mode the old and new data directories must be on the same file system.\n",
370+
"In link mode the old and new data directories must be on the same file system.",
371371
strerror(errno));
372372

373373
unlink(new_link_file);

‎src/bin/pg_upgrade/function.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ check_loadable_libraries(void)
162162
was_load_failure = true;
163163

164164
if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
165-
pg_fatal("could not open file \"%s\": %s\n",
165+
pg_fatal("could not open file \"%s\": %s",
166166
output_path, strerror(errno));
167167
fprintf(script, _("could not load library \"%s\": %s"),
168168
lib,
@@ -184,12 +184,12 @@ check_loadable_libraries(void)
184184
if (found)
185185
{
186186
fclose(script);
187-
pg_log(PG_REPORT, "fatal\n");
187+
pg_log(PG_REPORT, "fatal");
188188
pg_fatal("Your installation references loadable libraries that are missing from the\n"
189189
"new installation. You can add these libraries to the new installation,\n"
190190
"or remove the functions using them from the old installation. A list of\n"
191191
"problem libraries is in the file:\n"
192-
" %s\n\n", output_path);
192+
" %s", output_path);
193193
}
194194
else
195195
check_ok();

‎src/bin/pg_upgrade/info.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
123123
strcmp(old_rel->relname, new_rel->relname) != 0)
124124
{
125125
pg_log(PG_WARNING, "Relation names for OID %u in database \"%s\" do not match: "
126-
"old name \"%s.%s\", new name \"%s.%s\"\n",
126+
"old name \"%s.%s\", new name \"%s.%s\"",
127127
old_rel->reloid, old_db->db_name,
128128
old_rel->nspname, old_rel->relname,
129129
new_rel->nspname, new_rel->relname);
@@ -142,7 +142,7 @@ gen_db_file_maps(DbInfo *old_db, DbInfo *new_db,
142142
}
143143

144144
if (!all_matched)
145-
pg_fatal("Failed to match up old and new tables in database \"%s\"\n",
145+
pg_fatal("Failed to match up old and new tables in database \"%s\"",
146146
old_db->db_name);
147147

148148
*nmaps = num_maps;
@@ -257,10 +257,10 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
257257
}
258258

259259
if (is_new_db)
260-
pg_log(PG_WARNING, "No match found in old cluster for new relation with OID %u in database \"%s\": %s\n",
260+
pg_log(PG_WARNING, "No match found in old cluster for new relation with OID %u in database \"%s\": %s",
261261
reloid, db->db_name, reldesc);
262262
else
263-
pg_log(PG_WARNING, "No match found in new cluster for old relation with OID %u in database \"%s\": %s\n",
263+
pg_log(PG_WARNING, "No match found in new cluster for old relation with OID %u in database \"%s\": %s",
264264
reloid, db->db_name, reldesc);
265265
}
266266

@@ -284,9 +284,9 @@ get_db_and_rel_infos(ClusterInfo *cluster)
284284
get_rel_infos(cluster, &cluster->dbarr.dbs[dbnum]);
285285

286286
if (cluster == &old_cluster)
287-
pg_log(PG_VERBOSE, "\nsource databases:\n");
287+
pg_log(PG_VERBOSE, "\nsource databases:");
288288
else
289-
pg_log(PG_VERBOSE, "\ntarget databases:\n");
289+
pg_log(PG_VERBOSE, "\ntarget databases:");
290290

291291
if (log_opts.verbose)
292292
print_db_infos(&cluster->dbarr);
@@ -602,9 +602,8 @@ print_db_infos(DbInfoArr *db_arr)
602602

603603
for (dbnum = 0; dbnum < db_arr->ndbs; dbnum++)
604604
{
605-
pg_log(PG_VERBOSE, "Database: %s\n", db_arr->dbs[dbnum].db_name);
605+
pg_log(PG_VERBOSE, "Database: %s", db_arr->dbs[dbnum].db_name);
606606
print_rel_infos(&db_arr->dbs[dbnum].rel_arr);
607-
pg_log(PG_VERBOSE, "\n\n");
608607
}
609608
}
610609

@@ -615,7 +614,7 @@ print_rel_infos(RelInfoArr *rel_arr)
615614
int relnum;
616615

617616
for (relnum = 0; relnum < rel_arr->nrels; relnum++)
618-
pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: %s\n",
617+
pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: %s",
619618
rel_arr->rels[relnum].nspname,
620619
rel_arr->rels[relnum].relname,
621620
rel_arr->rels[relnum].reloid,

0 commit comments

Comments
 (0)