From c77f363384c3f4a6bbc73f501d2459e94382a30b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 26 Jan 2004 22:35:32 +0000 Subject: [PATCH] Ensure that close() and fclose() are checked for errors, at least in cases involving writes. Per recent discussion about the possibility of close-time failures on some filesystems. There is a TODO item for this, too. --- src/backend/access/transam/slru.c | 28 ++++++++++++++++++++++++---- src/backend/access/transam/xlog.c | 29 +++++++++++++++++++++-------- src/backend/commands/copy.c | 11 +++++++++-- src/backend/commands/user.c | 16 +++++++--------- src/backend/postmaster/postmaster.c | 16 ++++++++++------ src/backend/storage/file/fd.c | 16 +++++++++++----- src/backend/storage/freespace/freespace.c | 9 +++++++-- src/backend/utils/cache/relcache.c | 5 +++-- src/backend/utils/init/miscinit.c | 21 ++++++++++++++++++--- src/backend/utils/misc/guc.c | 13 +++++++++++-- src/bin/initdb/initdb.c | 10 +++++++--- src/bin/psql/command.c | 10 +++++++--- src/bin/psql/copy.c | 10 ++++++++-- src/include/storage/fd.h | 4 ++-- src/interfaces/libpq/fe-lobj.c | 13 ++++++++++--- 15 files changed, 155 insertions(+), 56 deletions(-) diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index a9bf0fcd2c..73c481c46b 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.8 2003/11/29 19:51:40 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.9 2004/01/26 22:35:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -144,7 +144,8 @@ typedef enum SLRU_CREATE_FAILED, SLRU_SEEK_FAILED, SLRU_READ_FAILED, - SLRU_WRITE_FAILED + SLRU_WRITE_FAILED, + SLRU_CLOSE_FAILED } SlruErrorCause; static SlruErrorCause slru_errcause; static int slru_errno; @@ -510,7 +511,13 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno) return false; } - close(fd); + if (close(fd)) + { + slru_errcause = SLRU_CLOSE_FAILED; + slru_errno = errno; + return false; + } + return true; } @@ -587,7 +594,13 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno) return false; } - close(fd); + if (close(fd)) + { + slru_errcause = SLRU_CLOSE_FAILED; + slru_errno = errno; + return false; + } + return true; } @@ -642,6 +655,13 @@ SlruReportIOError(SlruCtl ctl, int pageno, TransactionId xid) errdetail("could not write to file \"%s\" at offset %u: %m", path, offset))); break; + case SLRU_CLOSE_FAILED: + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not access status of transaction %u", xid), + errdetail("could not close file \"%s\": %m", + path))); + break; default: /* can't get here, we trust */ elog(ERROR, "unrecognized SimpleLru error cause: %d", diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 81ff16dd63..fe1ecd453c 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.132 2004/01/19 19:04:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.133 2004/01/26 22:35:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1044,7 +1044,7 @@ XLogWrite(XLogwrtRqst WriteRqst) */ if (openLogFile >= 0) { - if (close(openLogFile) != 0) + if (close(openLogFile)) ereport(PANIC, (errcode_for_file_access(), errmsg("could not close log file %u, segment %u: %m", @@ -1162,7 +1162,7 @@ XLogWrite(XLogwrtRqst WriteRqst) if (openLogFile >= 0 && !XLByteInPrevSeg(LogwrtResult.Write, openLogId, openLogSeg)) { - if (close(openLogFile) != 0) + if (close(openLogFile)) ereport(PANIC, (errcode_for_file_access(), errmsg("could not close log file %u, segment %u: %m", @@ -1427,7 +1427,10 @@ XLogFileInit(uint32 log, uint32 seg, (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", tmppath))); - close(fd); + if (close(fd)) + ereport(PANIC, + (errcode_for_file_access(), + errmsg("could not close file \"%s\": %m", tmppath))); /* * Now move the segment into place with its final name. @@ -2205,7 +2208,10 @@ WriteControlFile(void) (errcode_for_file_access(), errmsg("could not fsync control file: %m"))); - close(fd); + if (close(fd)) + ereport(PANIC, + (errcode_for_file_access(), + errmsg("could not close control file: %m"))); } static void @@ -2382,7 +2388,10 @@ UpdateControlFile(void) (errcode_for_file_access(), errmsg("could not fsync control file: %m"))); - close(fd); + if (close(fd)) + ereport(PANIC, + (errcode_for_file_access(), + errmsg("could not close control file: %m"))); } /* @@ -2535,7 +2544,11 @@ BootStrapXLOG(void) (errcode_for_file_access(), errmsg("could not fsync bootstrap transaction log file: %m"))); - close(openLogFile); + if (close(openLogFile)) + ereport(PANIC, + (errcode_for_file_access(), + errmsg("could not close bootstrap transaction log file: %m"))); + openLogFile = -1; memset(ControlFile, 0, sizeof(ControlFileData)); @@ -3577,7 +3590,7 @@ assign_xlog_sync_method(const char *method, bool doit, GucSource source) openLogId, openLogSeg))); if (open_sync_bit != new_sync_bit) { - if (close(openLogFile) != 0) + if (close(openLogFile)) ereport(PANIC, (errcode_for_file_access(), errmsg("could not close log file %u, segment %u: %m", diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index b3edfb2917..b5b3cfeefa 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.215 2004/01/18 02:15:29 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.216 2004/01/26 22:35:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -921,7 +921,14 @@ DoCopy(const CopyStmt *stmt) } if (!pipe) - FreeFile(copy_file); + { + /* we assume only the write case could fail here */ + if (FreeFile(copy_file)) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", + filename))); + } else if (IsUnderPostmaster && !is_from) SendCopyEnd(binary); pfree(attribute_buf.data); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index bd8c6b91b9..6a9ba6f529 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.132 2004/01/07 18:56:25 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.133 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -241,12 +241,11 @@ write_group_file(Relation grel) } heap_endscan(scan); - fflush(fp); - if (ferror(fp)) + if (FreeFile(fp)) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not write to temporary file \"%s\": %m", tempname))); - FreeFile(fp); + errmsg("could not write to temporary file \"%s\": %m", + tempname))); /* * Rename the temp file to its final name, deleting the old pg_pwd. We @@ -372,12 +371,11 @@ write_user_file(Relation urel) } heap_endscan(scan); - fflush(fp); - if (ferror(fp)) + if (FreeFile(fp)) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not write to temporary file \"%s\": %m", tempname))); - FreeFile(fp); + errmsg("could not write to temporary file \"%s\": %m", + tempname))); /* * Rename the temp file to its final name, deleting the old pg_pwd. We diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 56fec21acb..dd5f55c5ad 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -37,7 +37,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.358 2004/01/11 03:49:31 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.359 2004/01/26 22:35:32 tgl Exp $ * * NOTES * @@ -3171,15 +3171,12 @@ CreateOptsFile(int argc, char *argv[]) fprintf(fp, " '%s'", argv[i]); fputs("\n", fp); - fflush(fp); - if (ferror(fp)) + if (fclose(fp)) { elog(LOG, "could not write file \"%s\": %m", filename); - fclose(fp); return false; } - fclose(fp); return true; } @@ -3290,7 +3287,14 @@ write_backend_variables(Port *port) write_var(debug_flag,fp); /* Release file */ - FreeFile(fp); + if (FreeFile(fp)) + { + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", filename))); + return false; + } + return true; } diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 4875b7f2bc..df7d58d794 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.105 2003/12/20 17:31:21 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.106 2004/01/26 22:35:32 tgl Exp $ * * NOTES: * @@ -397,7 +397,7 @@ LruDelete(File file) /* close the file */ if (close(vfdP->fd)) - elog(LOG, "failed to close \"%s\": %m", + elog(ERROR, "failed to close \"%s\": %m", vfdP->fileName); --nfile; @@ -842,7 +842,7 @@ FileClose(File file) /* close the file */ if (close(vfdP->fd)) - elog(LOG, "failed to close \"%s\": %m", + elog(ERROR, "failed to close \"%s\": %m", vfdP->fileName); --nfile; @@ -1069,7 +1069,13 @@ TryAgain: return NULL; } -void +/* + * Close a file returned by AllocateFile. + * + * Note we do not check fclose's return value --- it is up to the caller + * to handle close errors. + */ +int FreeFile(FILE *file) { int i; @@ -1089,7 +1095,7 @@ FreeFile(FILE *file) if (i < 0) elog(WARNING, "file passed to FreeFile was not obtained from AllocateFile"); - fclose(file); + return fclose(file); } /* diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index 20f2186629..3c149c3510 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/freespace/freespace.c,v 1.29 2004/01/11 03:49:31 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/freespace/freespace.c,v 1.30 2004/01/26 22:35:32 tgl Exp $ * * * NOTES: @@ -793,7 +793,12 @@ DumpFreeSpaceMap(int code, Datum arg) /* Clean up */ LWLockRelease(FreeSpaceLock); - FreeFile(fp); + if (FreeFile(fp)) + { + elog(LOG, "could not write \"%s\": %m", cachefilename); + /* Remove busted cache file */ + unlink(cachefilename); + } return; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 797d96ca8e..fa8e2ac4d7 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.194 2003/12/28 21:57:37 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.195 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3330,7 +3330,8 @@ write_relcache_init_file(void) MemoryContextSwitchTo(oldcxt); } - FreeFile(fp); + if (FreeFile(fp)) + elog(FATAL, "could not write init file"); /* * Now we have to check whether the data we've so painstakingly diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index b2ed8a58d3..98de05c0b8 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.119 2004/01/08 06:01:21 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.120 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -622,7 +622,16 @@ CreateLockFile(const char *filename, bool amPostmaster, (errcode_for_file_access(), errmsg("could not write lock file \"%s\": %m", filename))); } - close(fd); + if (close(fd)) + { + int save_errno = errno; + + unlink(filename); + errno = save_errno; + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not write lock file \"%s\": %m", filename))); + } /* * Arrange for automatic removal of lockfile at proc_exit. @@ -776,7 +785,13 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2) close(fd); return; } - close(fd); + if (close(fd)) + { + ereport(LOG, + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", + directoryLockFile))); + } } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 7b3bde2d0c..33fd1e82ba 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.180 2004/01/24 20:00:45 wieck Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.181 2004/01/26 22:35:32 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -3971,7 +3971,16 @@ write_nondefault_variables(GucContext context) } } - FreeFile(fp); + if (FreeFile(fp)) + { + free(new_filename); + free(filename); + ereport(elevel, + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", CONFIG_EXEC_PARAMS))); + return; + } + /* Put new file in place, this could delay on Win32 */ rename(new_filename, filename); free(new_filename); diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index cd2df3acb2..b0f5c459c6 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -43,7 +43,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * Portions taken from FreeBSD. * - * $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.18 2003/12/23 21:50:38 tgl Exp $ + * $PostgreSQL: pgsql/src/bin/initdb/initdb.c,v 1.19 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1052,8 +1052,11 @@ set_short_version(char *short_version, char *extrapath) sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath); } version_file = fopen(path, PG_BINARY_W); + if (version_file == NULL) + exit_nicely(); fprintf(version_file, "%s\n", short_version); - fclose(version_file); + if (fclose(version_file)) + exit_nicely(); } /* @@ -1068,7 +1071,8 @@ set_null_conf(void) path = xmalloc(strlen(pg_data) + 17); sprintf(path, "%s/postgresql.conf", pg_data); conf_file = fopen(path, PG_BINARY_W); - fclose(conf_file); + if (conf_file == NULL || fclose(conf_file)) + exit_nicely(); } /* diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index a8e956530b..2b8fcb9dcd 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -3,7 +3,7 @@ * * Copyright (c) 2000-2003, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.111 2004/01/25 03:07:22 neilc Exp $ + * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.112 2004/01/26 22:35:32 tgl Exp $ */ #include "postgres_fe.h" #include "command.h" @@ -1601,8 +1601,12 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf) remove(fname); error = true; } - else - fclose(stream); + else if (fclose(stream) != 0) + { + psql_error("%s: %s\n", fname, strerror(errno)); + remove(fname); + error = true; + } } } diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index 9cbe6d2ffc..c2f89550c7 100644 --- a/src/bin/psql/copy.c +++ b/src/bin/psql/copy.c @@ -3,7 +3,7 @@ * * Copyright (c) 2000-2003, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/bin/psql/copy.c,v 1.39 2004/01/25 03:07:22 neilc Exp $ + * $PostgreSQL: pgsql/src/bin/psql/copy.c,v 1.40 2004/01/26 22:35:32 tgl Exp $ */ #include "postgres_fe.h" #include "copy.h" @@ -427,7 +427,13 @@ do_copy(const char *args) PQclear(result); if (options->file != NULL) - fclose(copystream); + { + if (fclose(copystream) != 0) + { + psql_error("%s: %s\n", options->file, strerror(errno)); + success = false; + } + } free_copy_options(options); return success; } diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 35a75a3671..26f30ffd53 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.41 2003/12/20 17:31:21 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.42 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -65,7 +65,7 @@ extern int FileTruncate(File file, long offset); /* Operations that allow use of regular stdio --- USE WITH CAUTION */ extern FILE *AllocateFile(char *name, char *mode); -extern void FreeFile(FILE *); +extern int FreeFile(FILE *); /* If you've really really gotta have a plain kernel FD, use this */ extern int BasicOpenFile(FileName fileName, int fileFlags, int fileMode); diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c index 822ccfe11d..16d4970d9e 100644 --- a/src/interfaces/libpq/fe-lobj.c +++ b/src/interfaces/libpq/fe-lobj.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-lobj.c,v 1.46 2004/01/07 18:56:29 neilc Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-lobj.c,v 1.47 2004/01/26 22:35:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -491,7 +491,7 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename) } /* - * read in from the Unix file and write to the inversion file + * read in from the inversion file and write to the Unix file */ while ((nbytes = lo_read(conn, lobj, buf, LO_BUFSIZE)) > 0) { @@ -508,7 +508,14 @@ lo_export(PGconn *conn, Oid lobjId, const char *filename) } (void) lo_close(conn, lobj); - (void) close(fd); + + if (close(fd)) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("error while writing to file \"%s\"\n"), + filename); + return -1; + } return 1; } -- 2.11.0