On 2025-04-07 Mo 6:13 AM, Álvaro Herrera wrote:
On 2025-Apr-07, Kyotaro Horiguchi wrote:
Hello.
While translating error messages related to pg_dumpall (1495eff7bdb),
I noticed that one message lacks double quotes around the file name:
could not open map file: %s
Since this placeholder appears standalone and not embedded in a
sentence, I initially thought it might fall outside the usual
convention of quoting file names.
Hello, I think the problem here is that the %s is not the file name, but
the string from strerror. So the lack of quotes there would seem to be
correct. The real problem is that the file name isn't mentioned in the
error message. A secondary issue might be that instead of using %s for
strerror(), maybe they should be using %m.
+ pg_fatal("could not open global.dat file: \"%s\"",
strerror(errno));
Maybe we should do something like
pg_fatal("could not open "%s" file: %m", map_file_path);
Yeah. Here's a more thorough error message cleanup.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index cdb8d84f064..18b544b0214 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -523,7 +523,7 @@ main(int argc, char *argv[])
OPF = fopen(global_path, PG_BINARY_W);
if (!OPF)
- pg_fatal("could not open global.dat file: %s", strerror(errno));
+ pg_fatal("could not open \"%s\": %m", global_path);
}
else if (filename)
{
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index b3dbde0d044..417317eb193 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -1035,7 +1035,7 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleOidStringList *dbn
*/
if (!file_exists_in_directory(dumpdirpath, "map.dat"))
{
- pg_log_info("databases restoring is skipped as map.dat file is not present in \"%s\"", dumpdirpath);
+ pg_log_info("database restoring is skipped as \"map.dat\" is not present in \"%s\"", dumpdirpath);
return 0;
}
@@ -1045,7 +1045,7 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleOidStringList *dbn
pfile = fopen(map_file_path, PG_BINARY_R);
if (pfile == NULL)
- pg_fatal("could not open map.dat file: \"%s\"", map_file_path);
+ pg_fatal("could not open \"%s\": %m", map_file_path);
/* Append all the dbname/db_oid combinations to the list. */
while ((fgets(line, MAXPGPATH, pfile)) != NULL)
@@ -1064,11 +1064,13 @@ get_dbname_oid_list_from_mfile(const char *dumpdirpath, SimpleOidStringList *dbn
/* Remove \n from dbname. */
dbname[strlen(dbname) - 1] = '\0';
- pg_log_info("found database \"%s\" (OID: %u) in map.dat file while restoring.", dbname, db_oid);
+ pg_log_info("found database \"%s\" (OID: %u) in \"%s\"",
+ dbname, db_oid, map_file_path);
/* Report error and exit if the file has any corrupted data. */
if (!OidIsValid(db_oid) || strlen(dbname) == 0)
- pg_fatal("invalid entry in map.dat file at line : %d", count + 1);
+ pg_fatal("invalid entry in \"%s\" at line : %d", map_file_path,
+ count + 1);
simple_oid_string_list_append(dbname_oid_list, db_oid, dbname);
count++;
@@ -1116,7 +1118,7 @@ restore_all_databases(PGconn *conn, const char *dumpdirpath,
if (dbname_oid_list.head == NULL)
return process_global_sql_commands(conn, dumpdirpath, opts->filename);
- pg_log_info("found total %d database names in map.dat file", num_total_db);
+ pg_log_info("found %d database names in \"map.dat\"", num_total_db);
if (!conn)
{
@@ -1288,7 +1290,7 @@ process_global_sql_commands(PGconn *conn, const char *dumpdirpath, const char *o
pfile = fopen(global_file_path, PG_BINARY_R);
if (pfile == NULL)
- pg_fatal("could not open global.dat file: \"%s\"", global_file_path);
+ pg_fatal("could not open \"%s\": %m", global_file_path);
/*
* If outfile is given, then just copy all global.dat file data into
@@ -1335,7 +1337,7 @@ process_global_sql_commands(PGconn *conn, const char *dumpdirpath, const char *o
/* Print a summary of ignored errors during global.dat. */
if (n_errors)
- pg_log_warning("errors ignored on global.dat file restore: %d", n_errors);
+ pg_log_warning("ignored %d errors in \"%s\"", n_errors, global_file_path);
fclose(pfile);
