164. [cleanup] Added functions isc_file_fopen(), isc_file_fclose(),

isc_file_fseek(), isc_file_fread(), isc_file_fwrite(),
                        isc_file_fflush(), isc_file_ffsync(), isc_file_remove()
                        to encapsulate nonportable usage of errno and fflush().

CHANGES

@@ -1,4 +1,9 @@
 
+ 164.   [cleanup]	Added functions isc_file_fopen(), isc_file_fclose(),
+			isc_file_fseek(), isc_file_fread(), isc_file_fwrite(),
+			isc_file_fflush(), isc_file_ffsync(), isc_file_remove()
+			to encapsulate nonportable usage of errno and fflush().
+
  163.   [func]		Added result codes ISC_R_FILENOTFOUND and ISC_R_FILEEXISTS.
 
  162.   [bug]           Ensure proper range for arguments to ctype.h functions.

lib/dns/journal.c

@@ -17,10 +17,10 @@
 
 #include <config.h>
 
-#include <errno.h>
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <isc/file.h>
 #include <isc/mem.h>
 #include <isc/net.h>		/* Required for ntohl. */
 #include <isc/string.h>
@@ -896,22 +896,14 @@ journal_header_encode(journal_header_t *cooked, journal_rawheader_t *raw) {
 
 /* Journal file I/O subroutines, with error checking and reporting. */
   
-/*
- * XXXRTH  You may not use UNIX I/O routines.  Use fread()/fwrite()/fseek().
- *	   Alternatively, we can create isc_file_t.  We may need to do this
- *	   anyway at some point, to provide "safe open" semantics (see
- *	   write_open() in BIND 8's ns_config.c).  We also need a portable
- *	   "fsync()", so isc_file_t is looking more and more probable.
- */
-
 static isc_result_t
 journal_seek(dns_journal_t *j, isc_uint32_t offset) {
-	int seek_result;
+	isc_result_t result;
-	seek_result = fseek(j->fp, (long) offset, SEEK_SET);
+	result = isc_file_fseek(j->fp, (long) offset, SEEK_SET);
-	if (seek_result != 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
-				 "%s: seek: %s",
+			      "%s: seek: %s", j->filename,
-				 j->filename, strerror(errno));
+			      isc_result_totext(result));
 		return (ISC_R_UNEXPECTED);
 	}
 	j->offset = offset;
@@ -920,16 +912,15 @@ journal_seek(dns_journal_t *j, isc_uint32_t offset) {
 
 static isc_result_t
 journal_read(dns_journal_t *j, void *mem, size_t nbytes) {
-	size_t nread;
+	isc_result_t result;
 
-	clearerr(j->fp);
+	result = isc_file_fread(mem, 1, nbytes, j->fp, NULL);
-	nread = fread(mem, 1, nbytes, j->fp);
+	if (result != ISC_R_SUCCESS) {
-	if (nread != nbytes) {
+		if (result == ISC_R_EOF)
-		if (feof(j->fp))
 			return (ISC_R_NOMORE);
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 			      "%s: read: %s",
-				 j->filename, strerror(errno));
+			      j->filename, isc_result_totext(result));
 		return (ISC_R_UNEXPECTED);
 	}
 	j->offset += nbytes;
@@ -938,14 +929,13 @@ journal_read(dns_journal_t *j, void *mem, size_t nbytes) {
 
 static isc_result_t
 journal_write(dns_journal_t *j, void *mem, size_t nbytes) {
-	size_t nwritten;
+	isc_result_t result;
 
-	clearerr(j->fp);	
+	result = isc_file_fwrite(mem, 1, nbytes, j->fp, NULL);
-	nwritten = fwrite(mem, 1, nbytes, j->fp);
+	if (result != ISC_R_SUCCESS) {
-	if (nwritten != nbytes) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 			      "%s: write: %s",
-				 j->filename, strerror(errno));
+			      j->filename, isc_result_totext(result));
 		return (ISC_R_UNEXPECTED);
 	}
 	j->offset += nbytes;	
@@ -954,21 +944,19 @@ journal_write(dns_journal_t *j, void *mem, size_t nbytes) {
 
 static isc_result_t
 journal_fsync(dns_journal_t *j) {
-	int r;
+	isc_result_t result;
-	r = fflush(j->fp);
+	result = isc_file_fflush(j->fp);
-	if (r < 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
-				 "%s: fflush: %s",
+			      "%s: flush: %s",
-				 j->filename,
+			      j->filename, isc_result_totext(result));
-				 strerror(errno));
 		return (ISC_R_UNEXPECTED);
 	}
-	r = fsync(fileno(j->fp));
+	result = isc_file_ffsync(j->fp);
-	if (r < 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 			      "%s: fsync: %s",
-				 j->filename,
+			      j->filename, isc_result_totext(result));
-				 strerror(errno));
 		return (ISC_R_UNEXPECTED);
 	}
 	return (ISC_R_SUCCESS);
@@ -1020,9 +1008,8 @@ journal_read_rrhdr(dns_journal_t *j, journal_rrhdr_t *rrhdr) {
 
 static isc_result_t
 journal_file_create(isc_mem_t *mctx, const char *filename) {
-	FILE *fp;
+	FILE *fp = NULL;
-	int r;
+	isc_result_t result;
-	size_t nwritten;
 	journal_header_t header;
 	journal_rawheader_t rawheader;
 	int index_size = 56; /* XXX configurable */
@@ -1031,11 +1018,11 @@ journal_file_create(isc_mem_t *mctx, const char *filename) {
 
 	INSIST(sizeof(journal_rawheader_t) == JOURNAL_HEADER_SIZE);
 	
-	fp = fopen(filename, "w");
+	result = isc_file_fopen(filename, "w", &fp);
-	if (fp == 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 			      "%s: create: %s",
-				 filename, strerror(errno));
+			      filename, isc_result_totext(result));
 		return (ISC_R_UNEXPECTED);
 	}
 
@@ -1048,31 +1035,31 @@ journal_file_create(isc_mem_t *mctx, const char *filename) {
 
 	mem = isc_mem_get(mctx, size);
 	if (mem == NULL) {
-		(void) fclose(fp);
+		(void)isc_file_fclose(fp);
-		(void) unlink(filename);		
+		(void)isc_file_remove(filename);
 		return (ISC_R_NOMEMORY);
 	}
 	memset(mem, 0, size);
 	memcpy(mem, &rawheader, sizeof(rawheader));
 
-	nwritten = fwrite(mem, 1, (size_t) size, fp);
+	result = isc_file_fwrite(mem, 1, (size_t) size, fp, NULL);
-	if (nwritten != (size_t) size) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 				 "%s: write: %s",
-				 filename, strerror(errno));
+				 filename, isc_result_totext(result));
-		(void) fclose(fp);
+		(void)isc_file_fclose(fp);
-		(void) unlink(filename);		
+		(void)isc_file_remove(filename);		
 		isc_mem_put(mctx, mem, size); 
 		return (ISC_R_UNEXPECTED);
 	}
 	isc_mem_put(mctx, mem, size); 	
 	
-	r = fclose(fp);
+	result = isc_file_fclose(fp);
-	if (r != 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,		
 				 "%s: close: %s",
-				 filename, strerror(errno));
+				 filename, isc_result_totext(result));
-		(void) unlink(filename);
+		(void)isc_file_remove(filename);
 		return (ISC_R_UNEXPECTED);
 	}
 
@@ -1083,7 +1070,7 @@ journal_file_create(isc_mem_t *mctx, const char *filename) {
 isc_result_t
 dns_journal_open(isc_mem_t *mctx, const char *filename, isc_boolean_t write,
 		 dns_journal_t **journalp) {
-	FILE *fp;
+	FILE *fp = NULL;
 	isc_result_t result;
 	journal_rawheader_t rawheader;
 	dns_journal_t *j;
@@ -1095,13 +1082,14 @@ dns_journal_open(isc_mem_t *mctx, const char *filename, isc_boolean_t write,
 	
 	j->mctx = mctx;
 	j->state = JOURNAL_STATE_INVALID;
-	j->fp = 0;
+	j->fp = NULL;
 	j->filename = filename;
 	j->index = NULL;
 
-	/* XXX fopen() will need "b" (binary) on some platforms */
+	/* XXX isc_file_fopen() may need "b" (binary) on some platforms */
-	fp = fopen(j->filename, write ? "r+" : "r");
+	result = isc_file_fopen(j->filename, write ? "r+" : "r", &fp);
-	if (fp == 0 && errno == ENOENT) {
+
+	if (result == ISC_R_FILENOTFOUND) {
 		if (write) {
 			isc_log_write(JOURNAL_COMMON_LOGARGS,
 				      ISC_LOG_INFO,
@@ -1110,15 +1098,15 @@ dns_journal_open(isc_mem_t *mctx, const char *filename, isc_boolean_t write,
 				      j->filename);
 			CHECK(journal_file_create(mctx, filename));
 			/* Retry. */
-			fp = fopen(j->filename, "r+");
+			result = isc_file_fopen(j->filename, "r+", &fp);
 		} else {
 			FAIL(ISC_R_NOTFOUND);
 		}
 	}
-	if (fp == 0) {
+	if (result != ISC_R_SUCCESS) {
 		isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR,
 			      "%s: open: %s",
-			      j->filename, strerror(errno));
+			      j->filename, isc_result_totext(result));
 		FAIL(ISC_R_UNEXPECTED);
 	}
 
@@ -1203,7 +1191,7 @@ dns_journal_open(isc_mem_t *mctx, const char *filename, isc_boolean_t write,
 		j->index = NULL;
 	}
 	if (j->fp != NULL)
-		(void) fclose(j->fp);
+		(void)isc_file_fclose(j->fp);
 	isc_mem_put(j->mctx, j, sizeof(*j));		
 	return (result);
 }
@@ -1703,7 +1691,7 @@ dns_journal_destroy(dns_journal_t **journalp) {
 		isc_mem_put(j->mctx, j->it.source.base, j->it.source.length);
 
 	if (j->fp != NULL)
-		(void) fclose(j->fp);
+		(void)isc_file_fclose(j->fp);
 	j->magic = 0;
 	isc_mem_put(j->mctx, j, sizeof(*j));
 	*journalp = NULL;

lib/isc/include/isc/file.h

@@ -90,6 +90,7 @@ isc_file_mktemplate(const char *path, char *buf, size_t buflen);
  *				of the path with the internal template string.
  */
 
+
 isc_result_t
 isc_file_openunique(char *templet, FILE **fp);
 /*
@@ -147,6 +148,58 @@ isc_file_openunique(char *templet, FILE **fp);
  *		Something totally unexpected happened.
  */
 
+
+isc_result_t
+isc_file_fopen(const char *filename, const char *mode, FILE **fp);
+
+isc_result_t
+isc_file_fclose(FILE *f);
+
+isc_result_t
+isc_file_fseek(FILE *f, long offset, int whence);
+
+isc_result_t
+isc_file_fread(void *ptr, size_t size, size_t nmemb, FILE *f,
+	       size_t *nret);
+
+isc_result_t
+isc_file_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *f,
+		size_t *nret);
+
+isc_result_t
+isc_file_fflush(FILE *f);
+
+/*
+ * These functions are wrappers around the corresponding
+ * stdio functions, returning a detailed error code in the
+ * form of an an isc_result_t.  ANSI C does not guarantee
+ * that stdio functions set errno, hence these functions 
+ * must use platform dependent methods (e.g., the POSIX errno)
+ * to construct the error code.
+ */
+
+isc_result_t
+isc_file_ffsync(FILE *f);
+/*
+ * Invoke fsync() on the file descriptor underlying
+ * an stdio stream, or an equivalent system-dependent
+ * operation.  Note that this function has no direct
+ * counterpart in the stdio library.
+ */
+
+isc_result_t
+isc_file_remove(const char *filename);
+/*
+ * Remove the file named by 'filename'.
+ */
+
+/*
+ * XXX We should also have a isc_file_writeeopen() function
+ * for safely open a file in a publicly writable directory
+ * (see write_open() in BIND 8's ns_config.c).
+ */
+
+
 ISC_LANG_ENDDECLS
 
 #endif /* ISC_FILE_H */

lib/isc/lex.c

@@ -21,6 +21,7 @@
 #include <stdlib.h>
 
 #include <isc/buffer.h>
+#include <isc/file.h>
 #include <isc/lex.h>
 #include <isc/mem.h>
 #include <isc/string.h>
@@ -210,7 +211,8 @@ new_source(isc_lex_t *lex, isc_boolean_t is_file, isc_boolean_t need_close,
 
 isc_result_t
 isc_lex_openfile(isc_lex_t *lex, const char *filename) {
-	FILE *stream;
+	isc_result_t result;
+	FILE *stream = NULL;
 
 	/*
 	 * Open 'filename' and make it the current input source for 'lex'.
@@ -218,17 +220,10 @@ isc_lex_openfile(isc_lex_t *lex, const char *filename) {
 
 	REQUIRE(VALID_LEX(lex));
 
-	/*
+	result = isc_file_fopen(filename, "r", &stream);
-	 * XXX we should really call something like isc_file_open() to
+	if (result != ISC_R_SUCCESS)
-	 * get maximally safe file opening.
+		return (result);
-	 */
+
-	stream = fopen(filename, "r");
-	/*
-	 * The C standard doesn't say that errno is set by fopen(), so
-	 * we just return a generic error.
-	 */
-	if (stream == NULL)
-		return (ISC_R_FAILURE);
 	flockfile(stream);
 	
 	return (new_source(lex, ISC_TRUE, ISC_TRUE, stream, filename));

lib/isc/unix/file.c

@@ -28,6 +28,36 @@
 #include <isc/time.h>
 #include <isc/util.h>
 
+/*
+ * Convert a POSIX errno value into an isc_result_t.  The
+ * list of supported errno values is not complete; new users
+ * of this function should add any expected errors that are
+ * not already there.
+ */
+static isc_result_t
+posix_result(int posixerrno) {
+	switch (posixerrno) {
+	case ENOTDIR:
+	case ELOOP:
+	case EINVAL:
+	case ENAMETOOLONG:
+	case EBADF:
+		return (ISC_R_INVALIDFILE);
+	case ENOENT:
+		return (ISC_R_FILENOTFOUND);
+	case EACCES:
+		return (ISC_R_NOPERM);
+	case EEXIST:
+		return (ISC_R_FILEEXISTS);
+	case EIO:
+		return (ISC_R_IOERROR);
+	case ENOMEM:
+		return (ISC_R_NOMEMORY);
+	default:
+		return (ISC_R_UNEXPECTED);
+	}
+}
+
 /*
  * XXXDCL As the API for accessing file statistics undoubtedly gets expanded,
  * it might be good to provide a mechanism that allows for the results
@@ -40,29 +70,8 @@ static isc_result_t
 file_stats(const char *file, struct stat *stats) {
 	isc_result_t result = ISC_R_SUCCESS;
 	
-	if (stat(file, stats) != 0) {
+	if (stat(file, stats) != 0)
-		switch (errno) {
+		result = posix_result(errno);
-		case ENOTDIR:
-		case ENOENT:
-			result = ISC_R_NOTFOUND;
-			break;
-		case ELOOP:
-		case EINVAL:
-		case ENAMETOOLONG:
-			result = ISC_R_INVALIDFILE;
-			break;
-		case EACCES:
-			result = ISC_R_NOPERM;
-			break;
-		case EIO:
-			result = ISC_R_IOERROR;
-			break;
-		case EFAULT:
-		default:
-			result = ISC_R_UNEXPECTED;
-			break;
-		}
-	}
 		
 	return (result);
 }
@@ -104,7 +113,6 @@ isc_file_mktemplate(const char *path, char *buf, size_t buflen) {
 		strncpy(buf, path, s - path + 1);
 		buf[s - path + 1] = '\0';
 		strcat(buf, TEMPLATE);
-
 	} else {
 		if (sizeof(TEMPLATE) > buflen)
 			return (ISC_R_NOSPACE);
@@ -130,34 +138,11 @@ isc_file_openunique(char *templet, FILE **fp) {
 	fd = mkstemp(templet);
 
 	if (fd == -1)
-		switch (errno) {
+		result = posix_result(errno);
-		case ENOTDIR:
-		case ELOOP:
-		case EINVAL:
-		case ENAMETOOLONG:
-			result = ISC_R_INVALIDFILE;
-			break;
-		case EACCES:
-			result = ISC_R_NOPERM;
-			break;
-		case EEXIST:
-			result = ISC_R_EXISTS;
-			break;
-		case EIO:
-			result = ISC_R_IOERROR;
-			break;
-		default:
-			result = ISC_R_UNEXPECTED;
-		}
-
 	if (result == ISC_R_SUCCESS) {
 		f = fdopen(fd, "w+");
 		if (f == NULL) {
-			if (errno == ENOMEM)
+			result = posix_result(errno);
-				result = ISC_R_NOMEMORY;
-			else
-				result = ISC_R_UNEXPECTED;
-
 			(void)remove(templet);
 			(void)close(fd);
 
@@ -167,3 +152,101 @@ isc_file_openunique(char *templet, FILE **fp) {
 
 	return (result);
 }
+
+isc_result_t
+isc_file_fopen(const char *filename, const char *mode, FILE **fp) {
+	FILE *f;
+	
+	f = fopen(filename, mode);
+	if (f == NULL)
+		return (posix_result(errno));
+	*fp = f;
+	return (ISC_R_SUCCESS);
+}
+
+isc_result_t
+isc_file_fclose(FILE *f) {
+	int r;
+
+	r = fclose(f);
+	if (r == 0)
+		return (ISC_R_SUCCESS);
+	else
+		return (posix_result(errno));
+}
+
+isc_result_t
+isc_file_fseek(FILE *f, long offset, int whence) {
+	int r;
+
+	r = fseek(f, offset, whence);
+	if (r == 0)
+		return (ISC_R_SUCCESS);
+	else
+		return (posix_result(errno));
+}
+
+isc_result_t
+isc_file_fread(void *ptr, size_t size, size_t nmemb, FILE *f, size_t *nret) {
+	isc_result_t result = ISC_R_SUCCESS;
+	size_t r;
+	
+	clearerr(f);
+	r = fread(ptr, size, nmemb, f);
+	if (r != nmemb) {
+		if (feof(f))
+			result = ISC_R_EOF;
+		else
+			result = posix_result(errno);
+	}
+	if (nret != NULL)
+		*nret = r;
+	return (result);
+}
+
+isc_result_t
+isc_file_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *f, size_t *nret) {
+	isc_result_t result = ISC_R_SUCCESS;
+	size_t r;
+	
+	clearerr(f);
+	r = fwrite(ptr, size, nmemb, f);
+	if (r != nmemb)
+		result = posix_result(errno);
+	if (nret != NULL)
+		*nret = r;
+	return (result);
+}
+
+isc_result_t
+isc_file_fflush(FILE *f) {
+	int r;
+	
+	r = fflush(f);
+	if (r == 0)
+		return (ISC_R_SUCCESS);
+	else
+		return (posix_result(errno));
+}
+
+isc_result_t
+isc_file_ffsync(FILE *f) {
+	int r;
+	
+	r = fsync(fileno(f));
+	if (r == 0)
+		return (ISC_R_SUCCESS);
+	else
+		return (posix_result(errno));
+}
+
+isc_result_t
+isc_file_remove(const char *filename) {
+	int r;
+	
+	r = unlink(filename);
+	if (r == 0)
+		return (ISC_R_SUCCESS);
+	else
+		return (posix_result(errno));
+}