From ac5442912f85386c92d9c7cd05d1ed5e8b1f0012 Mon Sep 17 00:00:00 2001
From: Kevin Day <thekevinday@gmail.com>
Date: Sat, 16 Apr 2022 00:01:06 -0500
Subject: [PATCH] Update: Improve the logic for the read link f_file functions.

The f_file_link_read() and f_file_link_read_at() functions have their dynamic allocation logic improved.
Use link_stat.st_size rather than target->used to pass to readlink() or readlinkat().

Initialize target->used to 0 and then update target->used on success while ensuring the string is always NULL terminated after the target->used.

The unit tests revealed that these can and should be improved.
---
 level_0/f_file/c/file.h         | 12 +++++++++++
 level_0/f_file/c/private-file.c | 44 +++++++++++++++++------------------------
 level_0/f_file/c/private-file.h |  4 ++++
 3 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/level_0/f_file/c/file.h b/level_0/f_file/c/file.h
index 80c314d..d1e20c3 100644
--- a/level_0/f_file/c/file.h
+++ b/level_0/f_file/c/file.h
@@ -979,7 +979,13 @@ extern "C" {
  *   F_string_too_large (with error bit) if link target path is too large for string buffer size.
  *   F_failure (with error bit) for any other error.
  *
+ *   Errors (with error bit) from: f_string_dynamic_increase_by()
+ *   Errors (with error bit) from: f_string_dynamic_terminate_after()
+ *
  * @see readlink()
+ *
+ * @see f_string_dynamic_increase_by()
+ * @see f_string_dynamic_terminate_after()
  */
 #ifndef _di_f_file_link_read_
   extern f_status_t f_file_link_read(const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target);
@@ -1018,7 +1024,13 @@ extern "C" {
  *   F_string_too_large (with error bit) if link target path is too large for string buffer size.
  *   F_failure (with error bit) for any other error.
  *
+ *   Errors (with error bit) from: f_string_dynamic_increase_by()
+ *   Errors (with error bit) from: f_string_dynamic_terminate_after()
+ *
  * @see readlinkat()
+ *
+ * @see f_string_dynamic_increase_by()
+ * @see f_string_dynamic_terminate_after()
  */
 #ifndef _di_f_file_link_read_at_
   extern f_status_t f_file_link_read_at(const int at_id, const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target);
diff --git a/level_0/f_file/c/private-file.c b/level_0/f_file/c/private-file.c
index a4f3e49..1850fc6 100644
--- a/level_0/f_file/c/private-file.c
+++ b/level_0/f_file/c/private-file.c
@@ -404,21 +404,12 @@ extern "C" {
 #if !defined(_di_f_file_link_read_) || !defined(_di_f_file_copy_)
   f_status_t private_f_file_link_read(const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target) {
 
-    // Create a NULL terminated string based on file stat.
-    if (link_stat.st_size + 1 > target->size) {
-      if (link_stat.st_size + 1 > F_array_length_t_size_d) {
-        return F_status_set_error(F_string_too_large);
-      }
-
-      const f_status_t status = f_string_dynamic_resize(link_stat.st_size + 1, target);
-      if (F_status_is_error(status)) return status;
-    }
+    target->used = 0;
 
-    memset(target->string, 0, sizeof(f_char_t) * (target->used + 1));
-
-    target->used = link_stat.st_size;
+    f_status_t status = f_string_dynamic_increase_by(link_stat.st_size + 1, target);
+    if (F_status_is_error(status)) return status;
 
-    if (readlink(path.string, target->string, target->used) < 0) {
+    if (readlink(path.string, target->string, link_stat.st_size) < 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);
@@ -432,6 +423,11 @@ extern "C" {
       return F_status_set_error(F_failure);
     }
 
+    target->used = link_stat.st_size;
+
+    status = f_string_dynamic_terminate_after(target);
+    if (F_status_is_error(status)) return status;
+
     return F_none;
   }
 #endif // !defined(_di_f_file_link_read_) || !defined(_di_f_file_copy_)
@@ -439,21 +435,12 @@ extern "C" {
 #if !defined(_di_f_file_link_read_at_) || !defined(_di_f_file_copy_at_)
   f_status_t private_f_file_link_read_at(const int at_id, const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target) {
 
-    // Create a NULL terminated string based on file stat.
-    if (link_stat.st_size + 1 > target->size) {
-      if (link_stat.st_size + 1 > F_array_length_t_size_d) {
-        return F_status_set_error(F_string_too_large);
-      }
-
-      const f_status_t status = f_string_dynamic_resize(link_stat.st_size + 1, target);
-      if (F_status_is_error(status)) return status;
-    }
+    target->used = 0;
 
-    memset(target->string, 0, sizeof(f_char_t) * (target->used + 1));
-
-    target->used = link_stat.st_size;
+    f_status_t status = f_string_dynamic_increase_by(link_stat.st_size + 1, target);
+    if (F_status_is_error(status)) return status;
 
-    if (readlinkat(at_id, path.string, target->string, target->used) < 0) {
+    if (readlinkat(at_id, path.string, target->string, link_stat.st_size) < 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 == EFAULT) return F_status_set_error(F_buffer);
@@ -468,6 +455,11 @@ extern "C" {
       return F_status_set_error(F_failure);
     }
 
+    target->used = link_stat.st_size;
+
+    status = f_string_dynamic_terminate_after(target);
+    if (F_status_is_error(status)) return status;
+
     return F_none;
   }
 #endif // !defined(_di_f_file_link_read_at_) || !defined(_di_f_file_copy_at_)
diff --git a/level_0/f_file/c/private-file.h b/level_0/f_file/c/private-file.h
index 59cf944..7e2b248 100644
--- a/level_0/f_file/c/private-file.h
+++ b/level_0/f_file/c/private-file.h
@@ -543,6 +543,8 @@ extern "C" {
  *
  * @see f_file_copy()
  * @see f_file_link_read()
+ * @see f_string_dynamic_increase_by()
+ * @see f_string_dynamic_terminate_after()
  */
 #if !defined(_di_f_file_link_read_) || !defined(_di_f_file_copy_)
   extern f_status_t private_f_file_link_read(const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target) F_attribute_visibility_internal_d;
@@ -581,6 +583,8 @@ extern "C" {
  *
  * @see f_file_copy_at()
  * @see f_file_link_read_at()
+ * @see f_string_dynamic_increase_by()
+ * @see f_string_dynamic_terminate_after()
  */
 #if !defined(_di_f_file_link_read_at_) || !defined(_di_f_file_copy_at_)
   extern f_status_t private_f_file_link_read_at(const int at_id, const f_string_static_t path, const struct stat link_stat, f_string_dynamic_t * const target) F_attribute_visibility_internal_d;
-- 
1.8.3.1