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


##########
libs/utils/src/properties.c:
##########
@@ -809,11 +910,518 @@ celix_properties_setVersion(celix_properties_t* props, 
const char* key, const ce
     return celix_properties_createAndSetEntry(props, key, &prototype);
 }
 
-celix_status_t celix_properties_setVersionWithoutCopy(celix_properties_t* 
props, const char* key, celix_version_t* version) {
+celix_status_t
+celix_properties_assignVersion(celix_properties_t* properties, const char* 
key, celix_version_t* version) {
+    assert(version != NULL);
     celix_properties_entry_t prototype = {0};
     prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
     prototype.typed.versionValue = version;
-    return celix_properties_createAndSetEntry(props, key, &prototype);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsLongArrayList(const celix_properties_t* 
properties,
+                                                   const char* key,
+                                                   const celix_array_list_t* 
defaultValue,
+                                                   celix_array_list_t** list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToLongArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+celix_status_t
+celix_properties_setLongArrayList(celix_properties_t* properties, const char* 
key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignLongArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setLongs(celix_properties_t* properties, const char* key, 
const long* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a long array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addLong(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add long to array list");
+            return status;
+        }
+    }
+
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+const celix_array_list_t* celix_properties_getLongArrayList(const 
celix_properties_t* properties,
+                                                            const char* key,
+                                                            const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t
+celix_properties_setDoubleArrayList(celix_properties_t* properties, const 
char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignDoubleArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setDoubles(celix_properties_t* properties, const char* key, 
const double* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a double array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addDouble(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add double to array list");
+            return status;
+        }
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsDoubleArrayList(const celix_properties_t* 
properties,
+                                                     const char* key,
+                                                     const celix_array_list_t* 
defaultValue,
+                                                     celix_array_list_t** 
list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToDoubleArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+const celix_array_list_t* celix_properties_getDoubleArrayList(const 
celix_properties_t* properties,
+                                                              const char* key,
+                                                              const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t celix_properties_setBoolArrayList(celix_properties_t* 
properties, const char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);

Review Comment:
   Shallow copy seems OK here, only if `values` were indeed BoolArrayList. 
Otherwise, our class typing invariants will be messed up. We need to check for 
type mismatch.
   
   The above remark also applies to `celix_properties_setDoubleArrayList` and 
`celix_properties_setLongArrayList`.



##########
libs/utils/src/properties.c:
##########
@@ -809,11 +910,518 @@ celix_properties_setVersion(celix_properties_t* props, 
const char* key, const ce
     return celix_properties_createAndSetEntry(props, key, &prototype);
 }
 
-celix_status_t celix_properties_setVersionWithoutCopy(celix_properties_t* 
props, const char* key, celix_version_t* version) {
+celix_status_t
+celix_properties_assignVersion(celix_properties_t* properties, const char* 
key, celix_version_t* version) {
+    assert(version != NULL);
     celix_properties_entry_t prototype = {0};
     prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
     prototype.typed.versionValue = version;
-    return celix_properties_createAndSetEntry(props, key, &prototype);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsLongArrayList(const celix_properties_t* 
properties,
+                                                   const char* key,
+                                                   const celix_array_list_t* 
defaultValue,
+                                                   celix_array_list_t** list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToLongArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+celix_status_t
+celix_properties_setLongArrayList(celix_properties_t* properties, const char* 
key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignLongArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setLongs(celix_properties_t* properties, const char* key, 
const long* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a long array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addLong(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add long to array list");
+            return status;
+        }
+    }
+
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+const celix_array_list_t* celix_properties_getLongArrayList(const 
celix_properties_t* properties,
+                                                            const char* key,
+                                                            const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t
+celix_properties_setDoubleArrayList(celix_properties_t* properties, const 
char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignDoubleArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setDoubles(celix_properties_t* properties, const char* key, 
const double* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a double array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addDouble(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add double to array list");
+            return status;
+        }
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsDoubleArrayList(const celix_properties_t* 
properties,
+                                                     const char* key,
+                                                     const celix_array_list_t* 
defaultValue,
+                                                     celix_array_list_t** 
list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToDoubleArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+const celix_array_list_t* celix_properties_getDoubleArrayList(const 
celix_properties_t* properties,
+                                                              const char* key,
+                                                              const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t celix_properties_setBoolArrayList(celix_properties_t* 
properties, const char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignBoolArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setBooleans(celix_properties_t* properties, const char* key, 
const bool* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a bool array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addBool(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add bool to array list.");
+            return status;
+        }
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsBoolArrayList(const celix_properties_t* 
properties,
+                                                     const char* key,
+                                                     const celix_array_list_t* 
defaultValue,

Review Comment:
   There is no type checking for `defaultValue`, i.e. to tell whether it is a 
BoolArrayList.



##########
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:
   As external API, `celix_utils_convertStringToXXXArrayList` all suffer from 
lack of type checking for `defaultValue`.



##########
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,
+                                                        const 
celix_array_list_t* defaultValue,
+                                                        celix_array_list_t** 
list) {
+    return celix_utils_convertStringToArrayList(val, defaultValue, list, NULL, 
celix_utils_addLongEntry);
+}
+
+celix_status_t celix_utils_addDoubleEntry(celix_array_list_t* list, const 
char* entry) {
+    bool converted;
+    double d = celix_utils_convertStringToDouble(entry, 0.0, &converted);
+    if (!converted) {
+        return CELIX_ILLEGAL_ARGUMENT;
     }
-    if (converted) {
-        *converted = result != NULL;
+    return celix_arrayList_addDouble(list, d);
+}
+
+celix_status_t celix_utils_convertStringToDoubleArrayList(const char* val,
+                                                          const 
celix_array_list_t* defaultValue,
+                                                          celix_array_list_t** 
list) {
+    return celix_utils_convertStringToArrayList(val, defaultValue, list, NULL, 
celix_utils_addDoubleEntry);
+}
+
+celix_status_t celix_utils_addBoolEntry(celix_array_list_t* list, const char* 
entry) {
+    bool converted;
+    bool b = celix_utils_convertStringToBool(entry, true, &converted);
+    if (!converted) {
+        return CELIX_ILLEGAL_ARGUMENT;
     }
-    if (result == NULL && defaultValue != NULL) {
-        result = celix_version_copy(defaultValue);
+    return celix_arrayList_addBool(list, b);
+}
+
+celix_status_t celix_utils_convertStringToBoolArrayList(const char* val,
+                                                          const 
celix_array_list_t* defaultValue,
+                                                          celix_array_list_t** 
list) {
+    return celix_utils_convertStringToArrayList(val, defaultValue, list, NULL, 
celix_utils_addBoolEntry);
+}
+
+celix_status_t celix_utils_addStringEntry(celix_array_list_t* list, const 
char* entry) {
+    char* copy = celix_utils_strdup(entry);
+    if (!copy) {
+            return CELIX_ENOMEM;
     }
+    return celix_arrayList_add(list, copy);
+}
+
+celix_status_t celix_utils_convertStringToStringArrayList(const char* val,
+                                                          const 
celix_array_list_t* defaultValue,
+                                                          celix_array_list_t** 
list) {
+    return celix_utils_convertStringToArrayList(val, defaultValue, list, free, 
celix_utils_addStringEntry);

Review Comment:
   If `defaultValue` is shallow copied into `list`, then `list` depends on 
`defaultValue`, i.e. `defaultValue` cannot be destroyed before `list`, which 
will be totally unexpected by our users.
   
   The same remark applies to `celix_utils_convertStringToVersionArrayList`.



##########
documents/services.md:
##########
@@ -58,7 +58,7 @@ The following C functions can be used to create and 
manipulate properties:
 - `celix_properties_getAsLong` - Get a long property value or string value 
parsed as long.
 - `celix_properties_getAsDouble` - Get a double property value or string value 
parsed as double.
 - `celix_properties_getAsBool` - Get a bool property value or string value 
parsed as bool.
-- `celix_properties_getVersion` - Get a pointer to the version property if the 
property value is a version or was
+- `celix_properties_getAsVersion` - Get a pointer to the version property if 
the property value is a version or was

Review Comment:
   This should be `celix_properties_getVersion`, since `getAsVersion` is 
mentioned afterwards.



##########
libs/utils/src/properties.c:
##########
@@ -809,11 +910,518 @@ celix_properties_setVersion(celix_properties_t* props, 
const char* key, const ce
     return celix_properties_createAndSetEntry(props, key, &prototype);
 }
 
-celix_status_t celix_properties_setVersionWithoutCopy(celix_properties_t* 
props, const char* key, celix_version_t* version) {
+celix_status_t
+celix_properties_assignVersion(celix_properties_t* properties, const char* 
key, celix_version_t* version) {
+    assert(version != NULL);
     celix_properties_entry_t prototype = {0};
     prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION;
     prototype.typed.versionValue = version;
-    return celix_properties_createAndSetEntry(props, key, &prototype);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsLongArrayList(const celix_properties_t* 
properties,
+                                                   const char* key,
+                                                   const celix_array_list_t* 
defaultValue,
+                                                   celix_array_list_t** list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToLongArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+celix_status_t
+celix_properties_setLongArrayList(celix_properties_t* properties, const char* 
key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignLongArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setLongs(celix_properties_t* properties, const char* key, 
const long* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a long array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addLong(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add long to array list");
+            return status;
+        }
+    }
+
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+const celix_array_list_t* celix_properties_getLongArrayList(const 
celix_properties_t* properties,
+                                                            const char* key,
+                                                            const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t
+celix_properties_setDoubleArrayList(celix_properties_t* properties, const 
char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignDoubleArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setDoubles(celix_properties_t* properties, const char* key, 
const double* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a double array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addDouble(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add double to array list");
+            return status;
+        }
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsDoubleArrayList(const celix_properties_t* 
properties,
+                                                     const char* key,
+                                                     const celix_array_list_t* 
defaultValue,
+                                                     celix_array_list_t** 
list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);
+        if (!copy) {
+            return CELIX_ENOMEM;
+        }
+        *list = copy;
+        return CELIX_SUCCESS;
+    }
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_STRING) {
+        celix_status_t convertStatus = 
celix_utils_convertStringToDoubleArrayList(entry->value, defaultValue, list);
+        if (convertStatus == CELIX_ILLEGAL_ARGUMENT) {
+            // conversion failed, but no memory error so defaultValue is 
copied and set
+            return CELIX_SUCCESS;
+        }
+        return convertStatus;
+    }
+    if (defaultValue) {
+        *list = celix_arrayList_copy(defaultValue);
+        return *list ? CELIX_SUCCESS : CELIX_ENOMEM;
+    }
+    *list = NULL;
+    return CELIX_SUCCESS;
+}
+
+const celix_array_list_t* celix_properties_getDoubleArrayList(const 
celix_properties_t* properties,
+                                                              const char* key,
+                                                              const 
celix_array_list_t* defaultValue) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        return entry->typed.arrayValue;
+    }
+    return defaultValue;
+}
+
+celix_status_t celix_properties_setBoolArrayList(celix_properties_t* 
properties, const char* key, const celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_array_list_t* copy = celix_arrayList_copy(values);
+    if (!copy) {
+        return CELIX_ENOMEM;
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = copy;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_assignBoolArrayList(celix_properties_t* properties, const 
char* key, celix_array_list_t* values) {
+    assert(values != NULL);
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = values;
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t
+celix_properties_setBooleans(celix_properties_t* properties, const char* key, 
const bool* values, size_t nrOfValues) {
+    assert(values != NULL);
+    celix_autoptr(celix_array_list_t) copy = celix_arrayList_create();
+    if (!copy) {
+        celix_err_push("Failed to create a bool array");
+        return CELIX_ENOMEM;
+    }
+    for (size_t i = 0; i < nrOfValues; ++i) {
+        celix_status_t status = celix_arrayList_addBool(copy, values[i]);
+        if (status != CELIX_SUCCESS) {
+            celix_err_push("Failed to add bool to array list.");
+            return status;
+        }
+    }
+    celix_properties_entry_t prototype = {0};
+    prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY;
+    prototype.typed.arrayValue = celix_steal_ptr(copy);
+    return celix_properties_createAndSetEntry(properties, key, &prototype);
+}
+
+celix_status_t celix_properties_getAsBoolArrayList(const celix_properties_t* 
properties,
+                                                     const char* key,
+                                                     const celix_array_list_t* 
defaultValue,
+                                                     celix_array_list_t** 
list) {
+    const celix_properties_entry_t* entry = 
celix_properties_getEntry(properties, key);
+    if (entry != NULL && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY) {
+        celix_array_list_t* copy = 
celix_arrayList_copy(entry->typed.arrayValue);

Review Comment:
   It should be OK to shallow copy embedded ArrayList of trivial types (like 
Bool/Double/Long), provided `celix_properties_setXXXArrayList` does not mess up 
typing invariants.



-- 
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