PengZheng commented on code in PR #721:
URL: https://github.com/apache/celix/pull/721#discussion_r1469304431
##########
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);
Review Comment:
The original implementation, i.e. one possible large allocation, plus
in-place escaping handling and tokenization is better.
~~Here I see memory leak also happens.~~
--
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
