From 9c8e5a96825241562809396cbd82e44bb909c298 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Thu, 7 Apr 2022 00:31:34 -0500 Subject: [PATCH] Bugfix: For f_file, use off_t, pre-test for EOF or error, use sizeof(f_char_t), and use size_t rather than ssize_t. The fread() and similar stream functions actually use size_t rather than ssize_t. Fix this bug by changing ssize_to to size_to for the affected functions. Use off_t rather than f_array_length_t to be consistent with the types used in the libc/POSIX API. Add additional checks for feof() and ferror() and the start of the stream read functions. There needs to be a few more error codes, add a TODO comment to address this via a separate commit. Use the size of f_char_t rather than 1 for reading. Use >= in buffer read check rather than ==, just in case. --- level_0/f_file/c/file.c | 67 +++++++++++++++++++++++++++++++++++++------------ level_0/f_file/c/file.h | 8 +++--- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/level_0/f_file/c/file.c b/level_0/f_file/c/file.c index 4f427de..718a4fa 100644 --- a/level_0/f_file/c/file.c +++ b/level_0/f_file/c/file.c @@ -1791,7 +1791,7 @@ extern "C" { #endif // _di_f_file_role_change_at_ #ifndef _di_f_file_seek_ - f_status_t f_file_seek(const int id, const int whence, const f_array_length_t offset, f_array_length_t * const seeked) { + f_status_t f_file_seek(const int id, const int whence, const off_t offset, off_t * const seeked) { #ifndef _di_level_0_parameter_checking_ if (id <= 0) return F_status_set_error(F_parameter); if (whence < 0) return F_status_set_error(F_parameter); @@ -1814,7 +1814,7 @@ extern "C" { #endif // _di_f_file_seek_ #ifndef _di_f_file_size_ - f_status_t f_file_size(const f_string_static_t path, const bool dereference, f_array_length_t * const size) { + f_status_t f_file_size(const f_string_static_t path, const bool dereference, off_t * const size) { #ifndef _di_level_0_parameter_checking_ if (!size) return F_status_set_error(F_parameter); #endif // _di_level_0_parameter_checking_ @@ -1839,7 +1839,7 @@ extern "C" { #endif // _di_f_file_size_ #ifndef _di_f_file_size_at_ - f_status_t f_file_size_at(const int at_id, const f_string_static_t path, const bool dereference, f_array_length_t * const size) { + f_status_t f_file_size_at(const int at_id, const f_string_static_t path, const bool dereference, off_t * const size) { #ifndef _di_level_0_parameter_checking_ if (!size) return F_status_set_error(F_parameter); #endif // _di_level_0_parameter_checking_ @@ -1864,7 +1864,7 @@ extern "C" { #endif // _di_f_file_size_at_ #ifndef _di_f_file_size_by_id_ - f_status_t f_file_size_by_id(const int id, f_array_length_t * const size) { + f_status_t f_file_size_by_id(const int id, off_t * const size) { #ifndef _di_level_0_parameter_checking_ if (id <= 0) return F_status_set_error(F_parameter); if (!size) return F_status_set_error(F_parameter); @@ -2064,11 +2064,23 @@ extern "C" { return F_status_set_error(F_file_closed); } - f_status_t status = F_none; - ssize_t size_read = 0; - flockfile(file.stream); + if (feof(file.stream)) { + funlockfile(file.stream); + + return F_none_eof; + } + + if (ferror(file.stream)) { + funlockfile(file.stream); + + return F_status_set_error(F_failure); // @todo create F_error and return F_error. + } + + f_status_t status = F_none; + size_t size_read = 0; + do { status = f_string_dynamic_increase_by(file.size_read, buffer); @@ -2078,7 +2090,7 @@ extern "C" { return status; } - size_read = fread_unlocked(buffer->string + buffer->used, 1, file.size_read, file.stream); + size_read = fread_unlocked(buffer->string + buffer->used, sizeof(f_char_t), file.size_read, file.stream); if (ferror(file.stream)) { funlockfile(file.stream); @@ -2112,14 +2124,20 @@ extern "C" { if (!file.stream) return F_status_set_error(F_file_closed); - ssize_t size_read = 0; + if (feof(file.stream)) { + return F_none_eof; + } + + if (ferror(file.stream)) { + return F_status_set_error(F_failure); // @todo create F_error and return F_error. + } { const f_status_t status = f_string_dynamic_increase_by(file.size_read, buffer); if (F_status_is_error(status)) return status; } - size_read = fread(buffer->string + buffer->used, 1, file.size_read, file.stream); + const size_t size_read = fread(buffer->string + buffer->used, sizeof(f_char_t), file.size_read, file.stream); if (ferror(file.stream)) { if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block); @@ -2153,17 +2171,34 @@ extern "C" { if (!file.stream) return F_status_set_error(F_file_closed); + flockfile(file.stream); + + if (feof(file.stream)) { + funlockfile(file.stream); + + return F_none_eof; + } + + if (ferror(file.stream)) { + funlockfile(file.stream); + + return F_status_set_error(F_failure); // @todo create F_error and return F_error. + } + { const f_status_t status = f_string_dynamic_increase_by(total, buffer); - if (F_status_is_error(status)) return status; + + if (F_status_is_error(status)) { + funlockfile(file.stream); + + return F_none_eof; + } } f_array_length_t buffer_size = file.size_read; f_array_length_t buffer_count = 0; - ssize_t size_read = 0; - - flockfile(file.stream); + size_t size_read = 0; for (;;) { @@ -2171,7 +2206,7 @@ extern "C" { buffer_size = total - buffer_count; } - size_read = fread(buffer->string + buffer->used, 1, file.size_read, file.stream); + size_read = fread(buffer->string + buffer->used, sizeof(f_char_t), file.size_read, file.stream); if (ferror(file.stream)) { funlockfile(file.stream); @@ -2197,7 +2232,7 @@ extern "C" { buffer_count += size_read; - if (buffer_count == total) break; + if (buffer_count >= total) break; } // for funlockfile(file.stream); diff --git a/level_0/f_file/c/file.h b/level_0/f_file/c/file.h index 73c4b65..88147a0 100644 --- a/level_0/f_file/c/file.h +++ b/level_0/f_file/c/file.h @@ -1752,7 +1752,7 @@ extern "C" { * @see lseek */ #ifndef _di_f_file_seek_ - extern f_status_t f_file_seek(const int id, const int whence, const f_array_length_t offset, f_array_length_t * const seeked); + extern f_status_t f_file_seek(const int id, const int whence, const off_t offset, off_t * const seeked); #endif // _di_f_file_seek_ /** @@ -1781,7 +1781,7 @@ extern "C" { * @see f_file_stat() */ #ifndef _di_f_file_size_ - extern f_status_t f_file_size(const f_string_static_t path, const bool dereference, f_array_length_t * const size); + extern f_status_t f_file_size(const f_string_static_t path, const bool dereference, off_t * const size); #endif // _di_f_file_size_ /** @@ -1812,7 +1812,7 @@ extern "C" { * @see f_file_stat_at() */ #ifndef _di_f_file_size_at_ - extern f_status_t f_file_size_at(const int at_id, const f_string_static_t path, const bool dereference, f_array_length_t * const size); + extern f_status_t f_file_size_at(const int at_id, const f_string_static_t path, const bool dereference, off_t * const size); #endif // _di_f_file_size_at_ /** @@ -1838,7 +1838,7 @@ extern "C" { * @see f_file_stat_by_id() */ #ifndef _di_f_file_size_by_id_ - extern f_status_t f_file_size_by_id(const int id, f_array_length_t * const size); + extern f_status_t f_file_size_by_id(const int id, off_t * const size); #endif // _di_f_file_size_by_id_ /** -- 1.8.3.1