]> Kevux Git Server - fll/commitdiff
Bugfix: Fixes for f_file exposed by unit tests.
authorKevin Day <thekevinday@gmail.com>
Wed, 20 Apr 2022 03:10:20 +0000 (22:10 -0500)
committerKevin Day <thekevinday@gmail.com>
Wed, 20 Apr 2022 03:22:42 +0000 (22:22 -0500)
Restructure private_f_file_close() to be more consistent with the parameter ordering as is done with the rest of the project.

Have f_file_stream_close() not handle flush errors so that close is only to be called once.
The design of fclose() and close() state that even on error the descriptors are freed.
This means that file close doesn't fail, it always succeeds.
But the errors need to be propogated.
Ignore flush and always call close so that the behavior of never calling close twice can be guaranteed.
The private_f_file_close() will still
This also means that the stream and descriptor are to always be reset on close.

Return values need to be updated for several functions.

Rename f_file_stream_descriptor() to f_file_stream_open_descriptor() to make it clear that this is an open command.

level_0/f_file/c/file.c
level_0/f_file/c/file.h
level_0/f_file/c/private-file.c
level_0/f_file/c/private-file.h

index ecdd1eee60e63a6a9c4f8a70264a2e83a0c3dc47..3c0818634da716ff54a2f1903d03cd87ac6d42a3 100644 (file)
@@ -131,7 +131,7 @@ extern "C" {
       if (!id) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    return private_f_file_close(id, F_false);
+    return private_f_file_close(F_false, id);
   }
 #endif // _di_f_file_close_
 
@@ -141,7 +141,7 @@ extern "C" {
       if (!id) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    return private_f_file_close(id, F_true);
+    return private_f_file_close(F_true, id);
   }
 #endif // _di_f_file_close_flush_
 
@@ -2025,15 +2025,24 @@ extern "C" {
 #endif // _di_f_file_stat_by_id_
 
 #ifndef _di_f_file_stream_close_
-  f_status_t f_file_stream_close(const bool complete, f_file_t * const file) {
+  f_status_t f_file_stream_close(const bool flush, f_file_t * const file) {
     #ifndef _di_level_0_parameter_checking_
       if (!file) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
     if (file->stream) {
+      if (flush) {
+        fflush(file->stream);
+      }
+
       if (fclose(file->stream) == EOF) {
+
+        // According to man pages, further access to a stream on error results in undefined behavior.
+        file->stream = 0;
+        file->id = -1;
+
         if (errno == EACCES) return F_status_set_error(F_access_denied);
-        if (errno == EAGAIN) return F_status_set_error(F_prohibited);
+        if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block);
         if (errno == EBADF) return F_status_set_error(F_file_descriptor);
         if (errno == EFBIG) return F_status_set_error(F_file_overflow);
         if (errno == EDEADLK) return F_status_set_error(F_deadlock);
@@ -2049,66 +2058,69 @@ extern "C" {
         if (errno == ENOTDIR) return F_status_set_error(F_file_type_not_directory);
         if (errno == EPERM) return F_status_set_error(F_prohibited);
         if (errno == EPIPE) return F_status_set_error(F_pipe_not);
-        if (errno == EWOULDBLOCK) return F_status_set_error(F_block);
 
-        return F_status_set_error(F_failure);
+        return F_status_set_error(F_file_close);
       }
 
       file->stream = 0;
-
-      // File stream will result in the file descriptor being invalid because it is already closed.
-      if (complete) {
-        file->id = -1;
-      }
+      file->id = -1;
 
       return F_none;
     }
 
-    if (complete) {
-      return private_f_file_close(&file->id, F_true);
-    }
+    const f_status_t status = private_f_file_close(flush, &file->id);
 
-    return F_none;
+    file->stream = 0;
+    file->id = -1;
+
+    return status;
   }
 #endif // _di_f_file_stream_close_
 
-#ifndef _di_f_file_stream_descriptor_
-  f_status_t f_file_stream_descriptor(const f_string_static_t mode, f_file_t * const file) {
+#ifndef _di_f_file_stream_open_descriptor_
+  f_status_t f_file_stream_open_descriptor(const f_string_static_t mode, f_file_t * const file) {
     #ifndef _di_level_0_parameter_checking_
       if (!file) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
     if (file->id == -1) {
-      return F_status_set_error(F_file_closed);
+      return F_status_set_error(F_file_descriptor);
     }
 
-    if (mode.string) {
-      file->stream = fdopen(file->id, mode.string);
+    if (mode.used) {
+      file->stream = fdopen(file->id, private_f_file_stream_open_mode_determine(file->flag));
     }
     else {
-      file->stream = fdopen(file->id, private_f_file_stream_open_mode_determine(file->flag));
+      file->stream = fdopen(file->id, mode.string);
     }
 
     if (!file->stream) {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
-      if (errno == EAGAIN) return F_status_set_error(F_prohibited);
+      if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block);
       if (errno == EBADF) return F_status_set_error(F_file_descriptor);
+      if (errno == EFBIG) return F_status_set_error(F_file_overflow);
       if (errno == EDEADLK) return F_status_set_error(F_deadlock);
+      if (errno == EDESTADDRREQ) return F_status_set_error(F_socket_not);
+      if (errno == EDQUOT) return F_status_set_error(F_space_not);
       if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == EINTR) return F_status_set_error(F_interrupt);
       if (errno == EINVAL) return F_status_set_error(F_parameter);
+      if (errno == EIO) return F_status_set_error(F_input_output);
       if (errno == EMFILE) return F_status_set_error(F_file_descriptor_max);
+      if (errno == ENODEV) return F_status_set_error(F_device_not);
       if (errno == ENOLCK) return F_status_set_error(F_lock);
+      if (errno == ENOMEM) return F_status_set_error(F_memory_not);
+      if (errno == ENOSPC) return F_status_set_error(F_space_not);
       if (errno == ENOTDIR) return F_status_set_error(F_file_type_not_directory);
       if (errno == EPERM) return F_status_set_error(F_prohibited);
+      if (errno == EPIPE) return F_status_set_error(F_pipe_not);
 
       return F_status_set_error(F_failure);
     }
 
     return F_none;
   }
-#endif // _di_f_file_stream_descriptor_
-
+#endif // _di_f_file_stream_open_descriptor_
 
 #ifndef _di_f_file_stream_open_
   f_status_t f_file_stream_open(const f_string_static_t path, const f_string_static_t mode, f_file_t * const file) {
@@ -2389,16 +2401,22 @@ extern "C" {
 
     if (!result) {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
-      if (errno == EAGAIN) return F_status_set_error(F_prohibited);
+      if (errno == EAGAIN || errno == EWOULDBLOCK) return F_status_set_error(F_block);
       if (errno == EBADF) return F_status_set_error(F_file_descriptor);
+      if (errno == EFBIG) return F_status_set_error(F_file_overflow);
       if (errno == EDEADLK) return F_status_set_error(F_deadlock);
+      if (errno == EDESTADDRREQ) return F_status_set_error(F_socket_not);
+      if (errno == EDQUOT) return F_status_set_error(F_space_not);
       if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == EINTR) return F_status_set_error(F_interrupt);
       if (errno == EINVAL) return F_status_set_error(F_parameter);
+      if (errno == EIO) return F_status_set_error(F_input_output);
       if (errno == EMFILE) return F_status_set_error(F_file_descriptor_max);
       if (errno == ENOLCK) return F_status_set_error(F_lock);
+      if (errno == ENOSPC) return F_status_set_error(F_space_not);
       if (errno == ENOTDIR) return F_status_set_error(F_file_type_not_directory);
       if (errno == EPERM) return F_status_set_error(F_prohibited);
+      if (errno == EPIPE) return F_status_set_error(F_pipe_not);
 
       return F_status_set_error(F_failure);
     }
index 7bdcc098384c81c2970a9474d88fa9a9809843d8..f5b08a34e5fea882a67f478cb78136430a9304a4 100644 (file)
@@ -2044,12 +2044,15 @@ extern "C" {
 /**
  * Close an open file stream.
  *
- * @param complete
- *   If TRUE, will close the file descriptor as well, setting file.id is reset to -1, on success.
- *   If FALSE, will do nothing in regards to the file descriptor.
+ * @param flush
+ *   If TRUE, will explicitly flush all unwritten data in any buffers to the file.
+ *   If FALSE, will not explicitly flush unwritten data.
+ *
+ *   If TRUE and flush fails, then this will still attempt to close the stream.
  * @param file
  *   The file information.
- *   The file.stream is set to 0, on success.
+ *   The file.stream is set to 0, on success or on failure.
+ *   The file.id is set to 0, on success or on failure.
  *
  * @return
  *   F_none is returned on success.
@@ -2070,15 +2073,15 @@ extern "C" {
  *   F_lock (with error bit) if failed to lock, such as lock table is full or too many open segments.
  *   F_parameter (with error bit) if a parameter is invalid.
  *   F_pipe_not (with error bit) if the stream is a pipe or a socket but the pipe or socket is already closed.
- *   F_prohibited (with error bit) if file system does not allow for making changes.
  *   F_space_not (with error bit) if file system is out of space (or file system quota is reached).
  *   F_socket_not (with error bit) if the datagram socket in which a peer has not been set (for socket related streams).
  *
  * @see close()
  * @see fclose()
+ * @see fflush()
  */
 #ifndef _di_f_file_stream_close_
-  extern f_status_t f_file_stream_close(const bool complete, f_file_t * const file);
+  extern f_status_t f_file_stream_close(const bool flush, f_file_t * const file);
 #endif // _di_f_file_stream_close_
 
 /**
@@ -2097,22 +2100,26 @@ extern "C" {
  *   F_none is returned on success.
  *
  *   F_access_denied (with error bit) on access denied.
+ *   F_block (with error bit) if the action would block and non-blocking is set on the stream.
  *   F_buffer (with error bit) if the buffer is invalid.
  *   F_deadlock (with error bit) if operation would cause a deadlock.
- *   F_file_closed (with error bit) if file is not open.
  *   F_file_descriptor (with error bit) if file descriptor is invalid.
  *   F_file_descriptor_max (with error bit) if max file descriptors is reached.
+ *   F_file_overflow (with error bit) if the write exceeds some implementation defined maximum file size.
  *   F_file_type_not_directory (with error bit) if F_NOTIFY was specified and file.id is not a directory.
  *   F_interrupt (with error bit) when program received an interrupt signal, halting operation.
  *   F_lock (with error bit) if failed to lock, such as lock table is full or too many open segments.
  *   F_parameter (with error bit) if a parameter is invalid.
+ *   F_pipe_not (with error bit) if the stream is a pipe or a socket but the pipe or socket is already closed.
  *   F_prohibited (with error bit) if file system does not allow for making changes.
+ *   F_socket_not (with error bit) if socket is not connected.
+ *   F_space_not (with error bit) if the file system is out of space (or file system quota is reached).
  *
  * @see fdopen()
  */
-#ifndef _di_f_file_stream_descriptor_
-  extern f_status_t f_file_stream_descriptor(const f_string_static_t mode, f_file_t * const file);
-#endif // _di_f_file_stream_descriptor_
+#ifndef _di_f_file_stream_open_descriptor_
+  extern f_status_t f_file_stream_open_descriptor(const f_string_static_t mode, f_file_t * const file);
+#endif // _di_f_file_stream_open_descriptor_
 
 /**
  * Open a file stream.
index 00e2183ed5b10efc55371c2ff0af29dab1635f29..5f69be9578054e946a21a6ea781fe1358f58e595 100644 (file)
@@ -6,14 +6,14 @@ extern "C" {
 #endif
 
 #if !defined(_di_f_file_close_) || !defined(_di_f_file_copy_) || !defined(_di_f_file_stream_close_)
-  f_status_t private_f_file_close(int * const id, const bool flush) {
+  f_status_t private_f_file_close(const bool flush, int * const id) {
 
     if (*id == -1) {
       return F_none;
     }
 
-    if (flush && F_status_is_error(private_f_file_flush(*id))) {
-      return F_status_set_error(F_file_synchronize);
+    if (flush) {
+      private_f_file_flush(*id);
     }
 
     if (close(*id) < 0) {
@@ -53,7 +53,7 @@ extern "C" {
     status = private_f_file_open(destination, 0, &file_destination);
 
     if (F_status_is_error(status)) {
-      private_f_file_close(&file_source.id, F_true);
+      private_f_file_close(F_true, &file_source.id);
 
       return status;
     }
@@ -69,15 +69,15 @@ extern "C" {
       size_write = write(file_destination.id, buffer, size_read);
 
       if (size_write < 0 || size_write != size_read) {
-        private_f_file_close(&file_destination.id, F_true);
-        private_f_file_close(&file_source.id, F_true);
+        private_f_file_close(F_true, &file_destination.id);
+        private_f_file_close(F_true, &file_source.id);
 
         return F_status_set_error(F_file_write);
       }
     } // while
 
-    private_f_file_close(&file_destination.id, F_true);
-    private_f_file_close(&file_source.id, F_true);
+    private_f_file_close(F_true, &file_destination.id);
+    private_f_file_close(F_true, &file_source.id);
 
     if (size_read < 0) {
       return F_status_set_error(F_file_read);
@@ -102,7 +102,7 @@ extern "C" {
     status = private_f_file_open_at(at_id, destination, 0, &file_destination);
 
     if (F_status_is_error(status)) {
-      private_f_file_close(&file_source.id, F_true);
+      private_f_file_close(F_true, &file_source.id);
 
       return status;
     }
@@ -118,15 +118,15 @@ extern "C" {
       size_write = write(file_destination.id, buffer, size_read);
 
       if (size_write < 0 || size_write != size_read) {
-        private_f_file_close(&file_destination.id, F_true);
-        private_f_file_close(&file_source.id, F_true);
+        private_f_file_close(F_true, &file_destination.id);
+        private_f_file_close(F_true, &file_source.id);
 
         return F_status_set_error(F_file_write);
       }
     } // while
 
-    private_f_file_close(&file_destination.id, F_true);
-    private_f_file_close(&file_source.id, F_true);
+    private_f_file_close(F_true, &file_destination.id);
+    private_f_file_close(F_true, &file_source.id);
 
     if (size_read < 0) {
       return F_status_set_error(F_file_read);
@@ -150,7 +150,7 @@ extern "C" {
     const f_status_t status = private_f_file_open(path, mode, &file);
 
     if (file.id != -1) {
-      return private_f_file_close(&file.id, F_true);
+      return private_f_file_close(F_true, &file.id);
     }
 
     return status;
@@ -171,7 +171,7 @@ extern "C" {
     const f_status_t status = private_f_file_open_at(at_id, path, mode, &file);
 
     if (file.id != -1) {
-      return private_f_file_close(&file.id, F_true);
+      return private_f_file_close(F_true, &file.id);
     }
 
     return status;
@@ -333,11 +333,12 @@ extern "C" {
     if (fsync(id) < 0) {
       if (errno == EBADF) return F_status_set_error(F_file_descriptor);
       if (errno == EDQUOT) return F_status_set_error(F_filesystem_quota_block);
+      if (errno == EINVAL) return F_status_set_error(F_supported_not);
       if (errno == EIO) return F_status_set_error(F_input_output);
       if (errno == ENOSPC) return F_status_set_error(F_space_not);
       if (errno == EROFS) return F_status_set_error(F_supported_not);
 
-      return F_status_set_error(F_failure);
+      return F_status_set_error(F_file_synchronize);
     }
 
     return F_none;
@@ -734,7 +735,7 @@ extern "C" {
   }
 #endif // !defined(_di_f_file_stat_by_id_) || !defined(_di_f_file_size_by_id_)
 
-#if !defined(_di_f_file_stream_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
+#if !defined(_di_f_file_stream_open_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
   const f_string_t private_f_file_stream_open_mode_determine(const int flag) {
 
     if (flag & F_file_flag_read_write_d) {
@@ -761,7 +762,7 @@ extern "C" {
     // Failsafe to read only.
     return f_file_open_mode_read_s.string;
   }
-#endif // !defined(_di_f_file_stream_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
+#endif // !defined(_di_f_file_stream_open_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
 
 #if !defined(f_file_stream_write) || !defined(_di_f_file_stream_write_block_) || !defined(f_file_stream_write_until) || !defined(f_file_stream_write_range)
   f_status_t private_f_file_stream_write_until(const f_file_t file, const f_string_static_t buffer, const f_array_length_t total, f_array_length_t * const written) {
index 2d798cdc05a4edb39d709b192c3e1e76699475ab..ae6edbe650a5b82e1b33c810f5b19a67f39e1cb4 100644 (file)
@@ -20,12 +20,12 @@ extern "C" {
  *
  * Intended to be shared to each of the different implementation variations.
  *
- * @param id
- *   The file descriptor.
- *   The value gets set to -1.
  * @param flush
  *   If TRUE, then perform flush before closing.
  *   If FALSE, then do not perform flush before closing.
+ * @param id
+ *   The file descriptor.
+ *   The value gets set to -1.
  *
  * @return
  *   F_none on success.
@@ -44,7 +44,7 @@ extern "C" {
  * @see f_file_stream_close()
  */
 #if !defined(_di_f_file_close_) || !defined(_di_f_file_copy_) || !defined(_di_f_file_stream_close_)
-  extern f_status_t private_f_file_close(int * const id, const bool flush) F_attribute_visibility_internal_d;
+  extern f_status_t private_f_file_close(const bool flush, int * const id) F_attribute_visibility_internal_d;
 #endif // !defined(_di_f_file_close_) || !defined(_di_f_file_copy_) || !defined(_di_f_file_stream_close_)
 
 /**
@@ -427,7 +427,7 @@ extern "C" {
  *   F_parameter (with error bit) if a parameter is invalid.
  *   F_space_not (with error bit) if file system is out of space (or file system quota is reached).
  *   F_supported_not (with error bit) if the file system or file type does not support flushing.
- *   F_failure (with error bit) on any other failure.
+ *   F_file_synchronize (with error bit) on any other failure.
  *
  * @see f_file_close()
  * @see f_file_copy()
@@ -902,13 +902,13 @@ extern "C" {
  * @return
  *   A string representing the file mode for use in fopen(), fdopen(), or freopen().
  *
- * @see f_file_stream_descriptor()
+ * @see f_file_stream_open_descriptor()
  * @see f_file_stream_open()
  * @see f_file_stream_reopen()
  */
-#if !defined(_di_f_file_stream_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
+#if !defined(_di_f_file_stream_open_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
   extern const f_string_t private_f_file_stream_open_mode_determine(const int flag) F_attribute_visibility_internal_d;
-#endif // !defined(_di_f_file_stream_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
+#endif // !defined(_di_f_file_stream_open_descriptor_) || !defined(_di_f_file_stream_open_) || !defined(_di_f_file_stream_reopen_)
 
 /**
  * Private implementation of f_file_stream_write_until().