From be178de34f0bc09cebeb6a11a50f754feab4892f Mon Sep 17 00:00:00 2001
From: PengZheng <howtofly@gmail.com>
Date: Fri, 29 Mar 2024 17:08:20 +0800
Subject: [PATCH] gh-674: Optimize memstream usage to reduce dynamic allocation
 and handle CELIX_ILLEGAL_ARGUMENT of addEntry callback.

---
 .../ConvertUtilsErrorInjectionTestSuite.cc    |  2 +-
 libs/utils/gtest/src/ConvertUtilsTestSuite.cc |  9 ++++-
 libs/utils/src/celix_convert_utils.c          | 38 +++++++++----------
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc b/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc
index c74c70199..63d6a8ff3 100644
--- a/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc
+++ b/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc
@@ -122,7 +122,7 @@ TEST_F(ConvertUtilsWithErrorInjectionTestSuite, LongArrayToStringTest) {
 
 TEST_F(ConvertUtilsWithErrorInjectionTestSuite, StringToStringArrayTest) {
     // Given an error injection for open_memstream (on the second call)
-    celix_ei_expect_open_memstream((void*)celix_utils_convertStringToStringArrayList, 1, nullptr, 2);
+    celix_ei_expect_open_memstream((void*)celix_utils_convertStringToStringArrayList, 1, nullptr);
     // When calling celix_utils_convertStringToStringArrayList
     celix_array_list_t* result;
     celix_status_t status = celix_utils_convertStringToStringArrayList("a,b,c", nullptr, &result);
diff --git a/libs/utils/gtest/src/ConvertUtilsTestSuite.cc b/libs/utils/gtest/src/ConvertUtilsTestSuite.cc
index f04144630..d0b402ba2 100644
--- a/libs/utils/gtest/src/ConvertUtilsTestSuite.cc
+++ b/libs/utils/gtest/src/ConvertUtilsTestSuite.cc
@@ -328,7 +328,14 @@ TEST_F(ConvertUtilsTestSuite, ConvertToLongArrayTest) {
     EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, convertState);
     EXPECT_TRUE(result != nullptr); //copy of default list
     EXPECT_EQ(1, celix_arrayList_size(result));
-    EXPECT_EQ(42L, celix_arrayList_getLong(result, 0));
+    EXPECT_TRUE(celix_arrayList_equals(defaultList, result));
+    celix_arrayList_destroy(result);
+
+    convertState = celix_utils_convertStringToLongArrayList("1,3,invalid,2", defaultList, &result);
+    EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, convertState);
+    EXPECT_TRUE(result != nullptr); //copy of default list
+    EXPECT_EQ(1, celix_arrayList_size(result));
+    EXPECT_TRUE(celix_arrayList_equals(defaultList, result));
     celix_arrayList_destroy(result);
 
     convertState = celix_utils_convertStringToLongArrayList("  1  ,  2  ,  3   ", nullptr, &result);
diff --git a/libs/utils/src/celix_convert_utils.c b/libs/utils/src/celix_convert_utils.c
index 35082ac74..32bbbcbfd 100644
--- a/libs/utils/src/celix_convert_utils.c
+++ b/libs/utils/src/celix_convert_utils.c
@@ -157,18 +157,16 @@ static celix_status_t celix_utils_convertStringToArrayList(const char* val,
         return CELIX_ILLEGAL_ARGUMENT;
     }
 
-    celix_autofree char* buf = NULL;
+    char* buf = NULL;
     size_t bufSize = 0;
-    celix_autoptr(FILE) entryStream = NULL;
+    FILE* entryStream = NULL;
     celix_status_t status = CELIX_SUCCESS;
     size_t max = strlen(val);
-    for (size_t i = 0; i <= max; ++i) {
-        if (!entryStream) {
-            entryStream = open_memstream(&buf, &bufSize);
-            if (!entryStream) {
-                 return CELIX_ENOMEM;
-            }
-        }
+    entryStream = open_memstream(&buf, &bufSize);
+    if (!entryStream) {
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i <= max && status == CELIX_SUCCESS; ++i) {
         if (val[i] == ESCAPE_CHAR) {
             // escape character, next char must be escapeChar or separatorChar
             if (i + 1 < max && (val[i + 1] == ESCAPE_CHAR || val[i + 1] == SEPARATOR_CHAR)) {
@@ -176,32 +174,32 @@ static celix_status_t celix_utils_convertStringToArrayList(const char* val,
                 i += 1;
                 int rc = fputc(val[i], entryStream);
                 if (rc == EOF) {
-                    return CELIX_ENOMEM;
+                    status = CELIX_ENOMEM;
                 }
-                continue;
             } else {
                 // invalid escape (ending with escapeChar or followed by an invalid char)
                 status = CELIX_ILLEGAL_ARGUMENT;
-                break;
             }
         } else if (val[i] == SEPARATOR_CHAR || val[i] == '\0') {
             //end of entry
-            fclose(celix_steal_ptr(entryStream));
-            entryStream = NULL;
-            status = addEntry(list, buf);
-            free(celix_steal_ptr(buf));
-            if (status == CELIX_ENOMEM) {
-                return status;
+            int rc = fputc('\0', entryStream);
+            if (rc == EOF) {
+                status = CELIX_ENOMEM;
+                continue;
             }
+            fflush(entryStream);
+            status = addEntry(list, buf);
+            rewind(entryStream);
         } else {
             //normal char
             int rc = fputc(val[i], entryStream);
             if (rc == EOF) {
-                return CELIX_ENOMEM;
+                status = CELIX_ENOMEM;
             }
         }
     }
-
+    fclose(entryStream);
+    free(buf);
 
     if (status == CELIX_SUCCESS) {
         *listOut = celix_steal_ptr(list);