From 9173313ee399247384abd9724c64fdf378dd49e5 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Mon, 23 Mar 2026 09:49:59 +0800 Subject: [PATCH v1] Check fclose() failures in more places Commit 860359ea0 fixed archive_waldump.c to check fclose(), noting that write errors such as ENOSPC may not be reported until close time. Do the same in several other file-writing paths that currently ignore fclose() failures. In postmaster.c, if writing the external PID file fails at fclose() time, report the error, avoid chmod() on the incomplete file, and unlink it so that a partial file is not left behind. Author: Chao Li Reviewed-by: Discussion: --- src/backend/postmaster/postmaster.c | 19 +++++++++++++------ src/bin/pg_basebackup/pg_createsubscriber.c | 3 ++- src/bin/pg_dump/pg_dumpall.c | 11 ++++++++--- src/bin/pg_upgrade/pg_upgrade.c | 3 ++- src/fe_utils/astreamer_file.c | 3 ++- src/fe_utils/recovery_gen.c | 6 ++++-- 6 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 3fac46c402b..95f20f35f31 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1300,12 +1300,19 @@ PostmasterMain(int argc, char *argv[]) if (fpidfile) { fprintf(fpidfile, "%d\n", MyProcPid); - fclose(fpidfile); - - /* Make PID file world readable */ - if (chmod(external_pid_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0) - write_stderr("%s: could not change permissions of external PID file \"%s\": %m\n", + if (fclose(fpidfile) != 0) + { + write_stderr("%s: could not write external PID file \"%s\": %m\n", progname, external_pid_file); + unlink(external_pid_file); + } + else + { + /* Make PID file world readable */ + if (chmod(external_pid_file, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0) + write_stderr("%s: could not change permissions of external PID file \"%s\": %m\n", + progname, external_pid_file); + } } else write_stderr("%s: could not write external PID file \"%s\": %m\n", @@ -4117,7 +4124,7 @@ CreateOptsFile(int argc, char *argv[], char *fullprogname) fprintf(fp, " \"%s\"", argv[i]); fputs("\n", fp); - if (fclose(fp)) + if (fclose(fp) != 0) { ereport(LOG, (errcode_for_file_access(), diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index 2bc84505aab..27d2c830d4d 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -1390,7 +1390,8 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c if (fwrite(recoveryconfcontents->data, recoveryconfcontents->len, 1, fd) != 1) pg_fatal("could not write to file \"%s\": %m", conf_filename); - fclose(fd); + if (fclose(fd) != 0) + pg_fatal("could not close file \"%s\": %m", conf_filename); recovery_params_set = true; /* Include conditionally the recovery parameters. */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 20cdd2d92f0..ba4831181c5 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -842,7 +842,8 @@ main(int argc, char *argv[]) if (filename) { - fclose(OPF); + if (fclose(OPF) != 0) + pg_fatal("could not close output file \"%s\": %m", filename); /* sync the resulting file, errors are not fatal */ if (dosync) @@ -2099,7 +2100,8 @@ dumpDatabases(PGconn *conn) create_opts = "--create"; if (filename && archDumpFormat == archNull) - fclose(OPF); + if (fclose(OPF) != 0) + pg_fatal("could not close output file \"%s\": %m", filename); /* * If this is not a plain format dump, then append dboid and dbname to @@ -2133,7 +2135,10 @@ dumpDatabases(PGconn *conn) /* Close map file */ if (archDumpFormat != archNull) - fclose(map_file); + { + if (fclose(map_file) != 0) + pg_fatal("could not close map file: %m"); + } PQclear(res); } diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 2127d297bfe..ee5dd654e25 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -353,7 +353,8 @@ make_outputdirs(char *pgdata) " pg_upgrade run on %s" "-----------------------------------------------------------------\n\n", ctime(&run_time)); - fclose(fp); + if (fclose(fp) != 0) + pg_fatal("could not close log file \"%s\": %m", filename_path); } } diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c index 6e63a41af0d..33da0bc980b 100644 --- a/src/fe_utils/astreamer_file.c +++ b/src/fe_utils/astreamer_file.c @@ -266,7 +266,8 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member, case ASTREAMER_MEMBER_TRAILER: if (mystreamer->file == NULL) break; - fclose(mystreamer->file); + if (fclose(mystreamer->file) != 0) + pg_fatal("could not close file \"%s\": %m", mystreamer->filename); mystreamer->file = NULL; break; diff --git a/src/fe_utils/recovery_gen.c b/src/fe_utils/recovery_gen.c index f352652c785..f6da9e0d152 100644 --- a/src/fe_utils/recovery_gen.c +++ b/src/fe_utils/recovery_gen.c @@ -143,7 +143,8 @@ WriteRecoveryConfig(PGconn *pgconn, const char *target_dir, PQExpBuffer contents if (fwrite(contents->data, contents->len, 1, cf) != 1) pg_fatal("could not write to file \"%s\": %m", filename); - fclose(cf); + if (fclose(cf) != 0) + pg_fatal("could not close file \"%s\": %m", filename); if (!use_recovery_conf) { @@ -152,7 +153,8 @@ WriteRecoveryConfig(PGconn *pgconn, const char *target_dir, PQExpBuffer contents if (cf == NULL) pg_fatal("could not create file \"%s\": %m", filename); - fclose(cf); + if (fclose(cf) != 0) + pg_fatal("could not close file \"%s\": %m", filename); } } -- 2.50.1 (Apple Git-155)