From: Kevin Day <thekevinday@gmail.com>
Date: Tue, 19 Apr 2022 03:27:58 +0000 (-0500)
Subject: Bugfix: Fix problems in f_file exposed by unit tests.
X-Git-Tag: 0.5.10~196
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=00bee85e3a0cbce2e1253a819860021542f6a76d;p=fll

Bugfix: Fix problems in f_file exposed by unit tests.

Add _f_file_rename_use_renameat2_ to enable support for renameat2() for systems in which this is available.
Add flags parameter to rename function to automatically support this.

Add missing "!path.used" checks.
Add missing parameter checks.
Move parameter checks that should instead be file closed checks that process normally.
Consistently apply file closed checks.

The function private_f_file_role_change_at() needs to check that result is not an error before processing group.
Failure to do this may result in hiding an error.

Minor clean ups.
---

diff --git a/build/level_0/settings b/build/level_0/settings
index 888f0ab..f8fbdde 100644
--- a/build/level_0/settings
+++ b/build/level_0/settings
@@ -100,6 +100,7 @@ search_shared yes
 search_static yes
 
 #defines -D_di_libcap_
+#defines -D_f_file_rename_use_renameat2_
 defines -D_libcap_legacy_only_ -D_pthread_attr_unsupported_ -D_pthread_sigqueue_unsupported_
 defines-clang -D_clang_not_a_compile_time_constant_workaround_
 defines-level_threadless -D_di_pthread_support_
diff --git a/build/monolithic/settings b/build/monolithic/settings
index 0f804d1..b06e664 100644
--- a/build/monolithic/settings
+++ b/build/monolithic/settings
@@ -151,6 +151,7 @@ search_shared yes
 search_static yes
 
 #defines -D_di_libcap_
+#defines -D_f_file_rename_use_renameat2_
 defines -D_libcap_legacy_only_ -D_pthread_attr_unsupported_ -D_pthread_sigqueue_unsupported_
 defines-clang -D_clang_not_a_compile_time_constant_workaround_
 defines-monolithic_threadless -D_di_pthread_support_
diff --git a/level_0/f_file/c/file.c b/level_0/f_file/c/file.c
index 3226d92..ecdd1ee 100644
--- a/level_0/f_file/c/file.c
+++ b/level_0/f_file/c/file.c
@@ -1742,11 +1742,11 @@ extern "C" {
     if (unlink(path.string) < 0) {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
       if (errno == EBUSY) return F_status_set_error(F_busy);
+      if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == EIO) return F_status_set_error(F_input_output);
       if (errno == EISDIR) return F_status_set_error(F_file_type_directory);
       if (errno == ELOOP) return F_status_set_error(F_loop);
       if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
-      if (errno == EFAULT) return F_status_set_error(F_buffer);
       if (errno == ENOENT) return F_status_set_error(F_file_found_not);
       if (errno == ENOMEM) return F_status_set_error(F_memory_not);
       if (errno == ENOTDIR) return F_status_set_error(F_directory_not);
@@ -1824,35 +1824,39 @@ extern "C" {
 #endif // _di_f_file_rename_
 
 #ifndef _di_f_file_rename_at_
-  f_status_t f_file_rename_at(const int at_id, const int to_id, const f_string_static_t source, const f_string_static_t destination) {
+  f_status_t f_file_rename_at(const int at_id, const int to_id, const f_string_static_t source, const f_string_static_t destination, const unsigned int flag) {
 
     if (!source.used || !destination.used) {
       return F_data_not;
     }
 
-    if (renameat(at_id, source.string, to_id, destination.string) < 0) {
-      if (errno == EACCES) return F_status_set_error(F_access_denied);
-      if (errno == EBADF) return F_status_set_error(F_directory_descriptor);
-      if (errno == EBUSY) return F_status_set_error(F_busy);
-      if (errno == EDQUOT) return F_status_set_error(F_filesystem_quota_block);
-      if (errno == EFAULT) return F_status_set_error(F_buffer);
-      if (errno == EINVAL) return F_status_set_error(F_parameter);
-      if (errno == EISDIR) return F_status_set_error(F_file_type_directory);
-      if (errno == ELOOP) return F_status_set_error(F_loop);
-      if (errno == EMLINK) return F_status_set_error(F_link);
-      if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
-      if (errno == ENOENT) return F_status_set_error(F_file_found_not);
-      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_directory_not);
-      if (errno == ENOTEMPTY) return F_status_set_error(F_directory_empty_not);
-      if (errno == EEXIST) return F_status_set_error(F_directory_empty_not);
-      if (errno == EPERM) return F_status_set_error(F_prohibited);
-      if (errno == EROFS) return F_status_set_error(F_read_only);
-      if (errno == EXDEV) return F_status_set_error(F_mount);
+    #ifdef _f_file_rename_use_renameat2_
+      if (renameat2(at_id, source.string, to_id, destination.string, flag) < 0) {
+    #else
+      if (renameat(at_id, source.string, to_id, destination.string) < 0) {
+    #endif // _f_file_rename_use_renameat2_
+        if (errno == EACCES) return F_status_set_error(F_access_denied);
+        if (errno == EBADF) return F_status_set_error(F_directory_descriptor);
+        if (errno == EBUSY) return F_status_set_error(F_busy);
+        if (errno == EDQUOT) return F_status_set_error(F_filesystem_quota_block);
+        if (errno == EFAULT) return F_status_set_error(F_buffer);
+        if (errno == EINVAL) return F_status_set_error(F_parameter);
+        if (errno == EISDIR) return F_status_set_error(F_file_type_directory);
+        if (errno == ELOOP) return F_status_set_error(F_loop);
+        if (errno == EMLINK) return F_status_set_error(F_link);
+        if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
+        if (errno == ENOENT) return F_status_set_error(F_file_found_not);
+        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_directory_not);
+        if (errno == ENOTEMPTY) return F_status_set_error(F_directory_empty_not);
+        if (errno == EEXIST) return F_status_set_error(F_directory_empty_not);
+        if (errno == EPERM) return F_status_set_error(F_prohibited);
+        if (errno == EROFS) return F_status_set_error(F_read_only);
+        if (errno == EXDEV) return F_status_set_error(F_mount);
 
-      return F_status_set_error(F_failure);
-    }
+        return F_status_set_error(F_failure);
+      }
 
     return F_none;
   }
@@ -1872,7 +1876,7 @@ extern "C" {
 #ifndef _di_f_file_role_change_at_
   f_status_t f_file_role_change_at(const int at_id, const f_string_static_t path, const uid_t uid, const gid_t gid, const int flag) {
 
-    if (uid == -1 && gid == -1) {
+    if (uid == -1 && gid == -1 || !path.used) {
       return F_data_not;
     }
 
@@ -1883,8 +1887,8 @@ extern "C" {
 #ifndef _di_f_file_seek_
   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);
+      if (!seeked) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
     *seeked = lseek(id, offset, whence);
@@ -1956,10 +1960,13 @@ extern "C" {
 #ifndef _di_f_file_size_by_id_
   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);
     #endif // _di_level_0_parameter_checking_
 
+    if (id == -1) {
+      return F_status_set_error(F_file_closed);
+    }
+
     struct stat stat_file;
 
     memset(&stat_file, 0, sizeof(struct stat));
@@ -2006,10 +2013,13 @@ extern "C" {
 #ifndef _di_f_file_stat_by_id_
   f_status_t f_file_stat_by_id(const int id, struct stat * const stat_file) {
     #ifndef _di_level_0_parameter_checking_
-      if (id <= 0) return F_status_set_error(F_parameter);
       if (!stat_file) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
+    if (id == -1) {
+      return F_status_set_error(F_file_closed);
+    }
+
     return private_f_file_stat_by_id(id, stat_file);
   }
 #endif // _di_f_file_stat_by_id_
@@ -2066,9 +2076,12 @@ extern "C" {
   f_status_t f_file_stream_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);
-      if (file->id == -1) 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);
+    }
+
     if (mode.string) {
       file->stream = fdopen(file->id, mode.string);
     }
@@ -2214,7 +2227,9 @@ extern "C" {
       if (!buffer) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (!file.stream) return F_status_set_error(F_file_closed);
+    if (!file.stream) {
+      return F_status_set_error(F_file_closed);
+    }
 
     flockfile(file.stream);
 
@@ -2279,7 +2294,9 @@ extern "C" {
       if (!buffer) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    if (!file.stream) return F_status_set_error(F_file_closed);
+    if (!file.stream) {
+      return F_status_set_error(F_file_closed);
+    }
 
     flockfile(file.stream);
 
diff --git a/level_0/f_file/c/file.h b/level_0/f_file/c/file.h
index 3db20e9..7bdcc09 100644
--- a/level_0/f_file/c/file.h
+++ b/level_0/f_file/c/file.h
@@ -1722,6 +1722,10 @@ extern "C" {
  *   The path to the file to copy from.
  * @param destination
  *   The path to copy to.
+ * @param flag
+ *   Any valid flag, such as F_file_at_path_empty_d, F_file_at_automount_no_d, or F_file_at_symlink_follow_no_d.
+ *   The POSIX renameat() doesn't support flag but Linux has a renameat2() that does.
+ *   If this is compiled with renameat2 support (), then flag is used otherwise flag is always 0 regardless of this property.
  *
  * @return
  *   F_none on success.
@@ -1749,7 +1753,7 @@ extern "C" {
  * @see renameat()
  */
 #ifndef _di_f_file_rename_at_
-  extern f_status_t f_file_rename_at(const int at_id, const int to_id, const f_string_static_t source, const f_string_static_t destination);
+  extern f_status_t f_file_rename_at(const int at_id, const int to_id, const f_string_static_t source, const f_string_static_t destination, const unsigned int flag);
 #endif // _di_f_file_rename_at_
 
 /**
@@ -1936,6 +1940,8 @@ extern "C" {
  *
  *   F_access_denied (with error bit) if access to the file was denied.
  *   F_directory_not (with error bit) on invalid directory.
+ *   F_file_closed (with error bit) if file is not open.
+ *   F_file_descriptor (with error bit) if the file descriptor is invalid.
  *   F_file_found_not (with error bit) if the file was not found.
  *   F_loop (with error bit) on loop error.
  *   F_memory_not (with error bit) if out of memory.
@@ -2093,6 +2099,7 @@ extern "C" {
  *   F_access_denied (with error bit) on access denied.
  *   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_type_not_directory (with error bit) if F_NOTIFY was specified and file.id is not a directory.
diff --git a/level_0/f_file/c/private-file.c b/level_0/f_file/c/private-file.c
index fa2ee95..00e2183 100644
--- a/level_0/f_file/c/private-file.c
+++ b/level_0/f_file/c/private-file.c
@@ -642,7 +642,7 @@ extern "C" {
       }
     }
 
-    if (gid != -1) {
+    if (result == 0 && gid != -1) {
       result = fchownat(at_id, path.string, -1, gid, flag);
 
       if (result < 0 && errno == EPERM) {
@@ -675,6 +675,7 @@ extern "C" {
     if ((dereference ? stat(path.string, file_stat) : lstat(path.string, file_stat)) < 0) {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
       if (errno == EFAULT) return F_status_set_error(F_buffer);
+      if (errno == EINVAL) return F_status_set_error(F_parameter);
       if (errno == ELOOP) return F_status_set_error(F_loop);
       if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
       if (errno == ENOENT) return F_status_set_error(F_file_found_not);
@@ -696,6 +697,7 @@ extern "C" {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
       if (errno == EBADF) return F_status_set_error(F_directory_descriptor);
       if (errno == EFAULT) return F_status_set_error(F_buffer);
+      if (errno == EINVAL) return F_status_set_error(F_parameter);
       if (errno == ELOOP) return F_status_set_error(F_loop);
       if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
       if (errno == ENOENT) return F_status_set_error(F_file_found_not);
@@ -715,10 +717,12 @@ extern "C" {
 
     if (fstat(id, file_stat) < 0) {
       if (errno == EACCES) return F_status_set_error(F_access_denied);
+      if (errno == EBADF) return F_status_set_error(F_file_descriptor);
       if (errno == EFAULT) return F_status_set_error(F_buffer);
+      if (errno == EINVAL) return F_status_set_error(F_parameter);
       if (errno == ELOOP) return F_status_set_error(F_loop);
       if (errno == ENAMETOOLONG) return F_status_set_error(F_name);
-      if (errno == ENOENT) return F_file_found_not;
+      if (errno == ENOENT) return F_status_set_error(F_file_found_not);
       if (errno == ENOMEM) return F_status_set_error(F_memory_not);
       if (errno == ENOTDIR) return F_status_set_error(F_directory_not);
       if (errno == EOVERFLOW) return F_status_set_error(F_number_overflow);
diff --git a/level_0/f_file/data/build/defines b/level_0/f_file/data/build/defines
index 4f13080..04d7dfd 100644
--- a/level_0/f_file/data/build/defines
+++ b/level_0/f_file/data/build/defines
@@ -1 +1,2 @@
 # fss-0000
+_f_file_rename_use_renameat2_ Use renameat2() instead of rename(), which allows for a flag value other than 0.
diff --git a/level_0/f_file/data/build/settings b/level_0/f_file/data/build/settings
index a32b56a..2bb01c5 100644
--- a/level_0/f_file/data/build/settings
+++ b/level_0/f_file/data/build/settings
@@ -50,6 +50,8 @@ flags -O2 -z now -g -fdiagnostics-color=always -Wno-logical-not-parentheses -Wno
 flags-clang -Wno-logical-op-parentheses
 flags-test -fstack-protector
 
+#defines -D_f_file_rename_use_renameat2_
+
 flags_library -fPIC
 flags_object -fPIC
 flags_program -fPIE