diff --git a/CHANGES b/CHANGES index 3881d3d337..9e29a9fd68 100644 --- a/CHANGES +++ b/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. diff --git a/lib/dns/journal.c b/lib/dns/journal.c index e762406e06..72b4d61f6c 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -17,10 +17,10 @@ #include -#include #include #include +#include #include #include /* Required for ntohl. */ #include @@ -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; - seek_result = fseek(j->fp, (long) offset, SEEK_SET); - if (seek_result != 0) { + isc_result_t result; + result = isc_file_fseek(j->fp, (long) offset, SEEK_SET); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: seek: %s", - j->filename, strerror(errno)); + "%s: seek: %s", j->filename, + 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); - nread = fread(mem, 1, nbytes, j->fp); - if (nread != nbytes) { - if (feof(j->fp)) + result = isc_file_fread(mem, 1, nbytes, j->fp, NULL); + if (result != ISC_R_SUCCESS) { + if (result == ISC_R_EOF) return (ISC_R_NOMORE); isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: read: %s", - j->filename, strerror(errno)); + "%s: read: %s", + 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); - nwritten = fwrite(mem, 1, nbytes, j->fp); - if (nwritten != nbytes) { + result = isc_file_fwrite(mem, 1, nbytes, j->fp, NULL); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: write: %s", - j->filename, strerror(errno)); + "%s: write: %s", + 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; - r = fflush(j->fp); - if (r < 0) { + isc_result_t result; + result = isc_file_fflush(j->fp); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: fflush: %s", - j->filename, - strerror(errno)); + "%s: flush: %s", + j->filename, isc_result_totext(result)); return (ISC_R_UNEXPECTED); } - r = fsync(fileno(j->fp)); - if (r < 0) { + result = isc_file_ffsync(j->fp); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: fsync: %s", - j->filename, - strerror(errno)); + "%s: fsync: %s", + j->filename, isc_result_totext(result)); 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; - int r; - size_t nwritten; + FILE *fp = NULL; + isc_result_t result; journal_header_t header; journal_rawheader_t rawheader; int index_size = 56; /* XXX configurable */ @@ -1030,12 +1017,12 @@ journal_file_create(isc_mem_t *mctx, const char *filename) { void *mem; /* Memory for temporary index image. */ INSIST(sizeof(journal_rawheader_t) == JOURNAL_HEADER_SIZE); - - fp = fopen(filename, "w"); - if (fp == 0) { + + result = isc_file_fopen(filename, "w", &fp); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, - "%s: create: %s", - filename, strerror(errno)); + "%s: create: %s", + 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) unlink(filename); + (void)isc_file_fclose(fp); + (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); - if (nwritten != (size_t) size) { + result = isc_file_fwrite(mem, 1, (size_t) size, fp, NULL); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, "%s: write: %s", - filename, strerror(errno)); - (void) fclose(fp); - (void) unlink(filename); + filename, isc_result_totext(result)); + (void)isc_file_fclose(fp); + (void)isc_file_remove(filename); isc_mem_put(mctx, mem, size); return (ISC_R_UNEXPECTED); } isc_mem_put(mctx, mem, size); - r = fclose(fp); - if (r != 0) { + result = isc_file_fclose(fp); + if (result != ISC_R_SUCCESS) { isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, "%s: close: %s", - filename, strerror(errno)); - (void) unlink(filename); + filename, isc_result_totext(result)); + (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 */ - fp = fopen(j->filename, write ? "r+" : "r"); - if (fp == 0 && errno == ENOENT) { + /* XXX isc_file_fopen() may need "b" (binary) on some platforms */ + result = isc_file_fopen(j->filename, write ? "r+" : "r", &fp); + + 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; diff --git a/lib/isc/include/isc/file.h b/lib/isc/include/isc/file.h index 556c8f957d..c4e9515a78 100644 --- a/lib/isc/include/isc/file.h +++ b/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 */ diff --git a/lib/isc/lex.c b/lib/isc/lex.c index 2f1b3ee34d..2a0b081c5c 100644 --- a/lib/isc/lex.c +++ b/lib/isc/lex.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -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)); - /* - * XXX we should really call something like isc_file_open() to - * get maximally safe file opening. - */ - 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); + result = isc_file_fopen(filename, "r", &stream); + if (result != ISC_R_SUCCESS) + return (result); + flockfile(stream); return (new_source(lex, ISC_TRUE, ISC_TRUE, stream, filename)); diff --git a/lib/isc/unix/file.c b/lib/isc/unix/file.c index fd0899f12c..3f05739490 100644 --- a/lib/isc/unix/file.c +++ b/lib/isc/unix/file.c @@ -19,7 +19,7 @@ #include #include -#include /* Required for mkstemp on NetBSD. */ +#include /* Required for mkstemp on NetBSD. */ #include @@ -28,6 +28,36 @@ #include #include +/* + * 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 @@ -39,31 +69,10 @@ static isc_result_t file_stats(const char *file, struct stat *stats) { isc_result_t result = ISC_R_SUCCESS; - - if (stat(file, stats) != 0) { - switch (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; - } - } - + + if (stat(file, stats) != 0) + result = posix_result(errno); + return (result); } @@ -100,21 +109,20 @@ isc_file_mktemplate(const char *path, char *buf, size_t buflen) { if (s != NULL) { if ((s - path + 1 + sizeof(TEMPLATE)) > buflen) return (ISC_R_NOSPACE); - + strncpy(buf, path, s - path + 1); buf[s - path + 1] = '\0'; strcat(buf, TEMPLATE); - } else { if (sizeof(TEMPLATE) > buflen) return (ISC_R_NOSPACE); - + strcpy(buf, TEMPLATE); } - + return (ISC_R_SUCCESS); } - + isc_result_t isc_file_openunique(char *templet, FILE **fp) { int fd; @@ -130,34 +138,11 @@ isc_file_openunique(char *templet, FILE **fp) { fd = mkstemp(templet); if (fd == -1) - switch (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; - } - + result = posix_result(errno); if (result == ISC_R_SUCCESS) { f = fdopen(fd, "w+"); if (f == NULL) { - if (errno == ENOMEM) - result = ISC_R_NOMEMORY; - else - result = ISC_R_UNEXPECTED; - + result = posix_result(errno); (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)); +}