gh-674: Optimize memstream usage to reduce dynamic allocation and han…

…dle CELIX_ILLEGAL_ARGUMENT of addEntry callback.
apache · Mar 29, 2024 · be178de · be178de
1 parent fc556fa
commit be178de
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 22 deletions.
diff --git 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
@@ -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
@@ -157,51 +157,49 @@ 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)) {
                 // write escaped char
                 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);