PengZheng commented on code in PR #721:
URL: https://github.com/apache/celix/pull/721#discussion_r1537299608


##########
libs/utils/src/celix_convert_utils.c:
##########
@@ -107,25 +112,281 @@ long celix_utils_convertStringToLong(const char* val, 
long defaultValue, bool* c
     return result;
 }
 
-celix_version_t* celix_utils_convertStringToVersion(const char* val, const 
celix_version_t* defaultValue, bool* converted) {
-    celix_version_t* result = NULL;
-    if (val != NULL) {
-        //check if string has two dots ('.'), and only try to create string if 
it has two dots
-        char* firstDot = strchr(val, '.');
-        char* lastDot = strrchr(val, '.');
-        if (firstDot != NULL && lastDot != NULL && firstDot != lastDot) {
-            char buf[64];
-            char* valCopy = celix_utils_writeOrCreateString(buf, sizeof(buf), 
"%s", val);
-            char *trimmed = celix_utils_trimInPlace(valCopy);
-            result = celix_version_createVersionFromString(trimmed);
-            celix_utils_freeStringIfNotEqual(buf, valCopy);
+celix_status_t
+celix_utils_convertStringToVersion(const char* val, const celix_version_t* 
defaultValue, celix_version_t** version) {
+    assert(version != NULL);
+    if (!val && defaultValue) {
+        *version = celix_version_copy(defaultValue);
+        return *version ? CELIX_ILLEGAL_ARGUMENT : CELIX_ENOMEM;
+    }
+
+    celix_status_t status = celix_version_parse(val, version);
+    if (status == CELIX_ILLEGAL_ARGUMENT) {
+        if (defaultValue) {
+            *version = celix_version_copy(defaultValue);
+            return *version ? status : CELIX_ENOMEM;
+        }
+        return status;
+    }
+    return status;
+}
+
+/**
+ * @brief Convert the provided string to an array list using the provided 
addEntry callback to convert the string
+ * to a specific type and add it to the list.
+ */
+static celix_status_t celix_utils_convertStringToArrayList(const char* val,
+                                                           const 
celix_array_list_t* defaultValue,
+                                                           
celix_array_list_t** list,
+                                                           void 
(*freeCb)(void*),
+                                                           celix_status_t 
(*addEntry)(celix_array_list_t*,
+                                                                               
       const char*)) {
+    assert(list != NULL);
+    *list = NULL;
+
+    if (!val && defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_ILLEGAL_ARGUMENT : CELIX_ENOMEM;
+    } else if (!val) {
+        return CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    celix_array_list_create_options_t opts = 
CELIX_EMPTY_ARRAY_LIST_CREATE_OPTIONS;
+    opts.simpleRemovedCallback = freeCb;
+    celix_autoptr(celix_array_list_t) result = 
celix_arrayList_createWithOptions(&opts);
+    if (!result) {
+        return CELIX_ENOMEM;
+    }
+
+    char* buf = NULL;
+    size_t bufSize = 0;
+    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;
+            }
+        }
+        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;
+                }
+                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(entryStream);
+            entryStream = NULL;
+            status = addEntry(result, buf);
+            if (status == CELIX_ENOMEM) {
+                return status;
+            }
+        } else {
+            //normal char
+            int rc = fputc(val[i], entryStream);
+            if (rc == EOF) {
+                return CELIX_ENOMEM;
+            }
+        }
+    }
+
+
+    if (status == CELIX_SUCCESS) {
+        *list = celix_steal_ptr(result);
+    } else if (status == CELIX_ILLEGAL_ARGUMENT) {
+        if (defaultValue) {
+            *list = celix_arrayList_copy(defaultValue);
+            return *list ? status : CELIX_ENOMEM;
         }
+        return status;
+    }
+    return status;
+}
+
+celix_status_t celix_utils_addLongEntry(celix_array_list_t* list, const char* 
entry) {
+    bool converted;
+    long l = celix_utils_convertStringToLong(entry, 0L, &converted);
+    if (!converted) {
+        return CELIX_ILLEGAL_ARGUMENT;
+    }
+    return celix_arrayList_addLong(list, l);
+}
+
+celix_status_t celix_utils_convertStringToLongArrayList(const char* val,

Review Comment:
   I think the main difficulty of replacing 
`celix_utils_convertStringToXXXArrayList` by 
`celix_utils_convertStringToArrayList` is that the string contains no typing 
information, unlike JSON.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to