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.

15 files changed, 155 insertions, 56 deletions

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index a9bf0fcd2cb..73c481c46b7 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 81ff16dd633..fe1ecd453c5 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 b3edfb2917c..b5b3cfeefac 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 bd8c6b91b9e..6a9ba6f529a 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 56fec21acb3..dd5f55c5adb 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 4875b7f2bcf..df7d58d7940 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 20f21866295..3c149c3510b 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 797d96ca8ec..fa8e2ac4d7b 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 b2ed8a58d32..98de05c0b8f 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 7b3bde2d0c2..33fd1e82ba6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * 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 cd2df3acb23..b0f5c459c6e 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 a8e956530b2..2b8fcb9dcde 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 9cbe6d2ffc3..c2f89550c7f 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 35a75a36719..26f30ffd537 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 822ccfe11d6..16d4970d9eb 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;
 }