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.
##########
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);
+}
+
+static celix_status_t celix_utils_addVersionEntry(celix_array_list_t* list,
const char* entry) {
+ celix_version_t* version;
+ celix_status_t convertStatus = celix_utils_convertStringToVersion(entry,
NULL, &version);
+ if (convertStatus == CELIX_SUCCESS) {
+ return celix_arrayList_add(list, version);
+ }
+ return convertStatus;
+}
+
+static void celix_utils_destroyVersionEntry(void* entry) {
celix_version_destroy(entry); }
+
+celix_status_t celix_utils_convertStringToVersionArrayList(const char* val,
+ const
celix_array_list_t* defaultValue,
+
celix_array_list_t** list) {
+ return celix_utils_convertStringToArrayList(
+ val, defaultValue, list, celix_utils_destroyVersionEntry,
celix_utils_addVersionEntry);
+}
+
+/**
+ * @brief Convert the provided array list to a string using the provided print
callback.
+ *
+ * Will log an error message to celix_err if an error occurred.
+ *
+ * @param list The list to convert.
+ * @param printCb The callback to use for printing the list entries.
+ * @return The string representation of the list or NULL if an error occurred.
+ */
+static char* celix_utils_arrayListToString(const celix_array_list_t* list,
+ int (*printCb)(FILE* stream, const
celix_array_list_entry_t* entry)) {
+ char* result = NULL;
+ size_t len;
+ FILE* stream = open_memstream(&result, &len);
+ if (!stream) {
+ celix_err_push("Cannot open memstream");
+ return NULL;
+ }
+
+ int size = list ? celix_arrayList_size(list) : 0;
+ for (int i = 0; i < size; ++i) {
+ celix_array_list_entry_t entry = celix_arrayList_getEntry(list, i);
+ int rc = printCb(stream, &entry);
+ if (rc >= 0 && i < size - 1) {
+ rc = fputs(",", stream);
+ }
+ if (rc < 0) {
+ celix_err_push("Cannot print to stream");
+ fclose(stream);
+ free(result);
+ return NULL;
+ }
+ }
+ fclose(stream);
return result;
}
+
+static int celix_utils_printLongEntry(FILE* stream, const
celix_array_list_entry_t* entry) {
+ return fprintf(stream, "%li", entry->longVal);
+}
+
+char* celix_utils_longArrayListToString(const celix_array_list_t* list) {
+ return celix_utils_arrayListToString(list, celix_utils_printLongEntry);
+}
+
+static int celix_utils_printDoubleEntry(FILE* stream, const
celix_array_list_entry_t* entry) {
+ return fprintf(stream, "%lf", entry->doubleVal);
Review Comment:
What does `%lf` mean? I just checked `man 3 printf` and only found `%Lf` for
long double.
##########
libs/utils/include/celix_convert_utils.h:
##########
@@ -71,13 +77,153 @@ CELIX_UTILS_EXPORT long
celix_utils_convertStringToLong(const char* val, long de
*
* @param[in] val The string to convert.
* @param[in] defaultValue The default value if the string is not a valid
celix_version_t.
- * @param[out] converted If not NULL, will be set to true if the string is a
valid celix_version_t, otherwise false.
- * @return A new celix_version_t* if the string is a valid version, otherwise
NULL.
+ * @param[out] version The converted version. If the string is not a valid
version, the version will be set to a copy of
+ * the defaultValue.
+ * @return CELIX_SUCCESS if the string is a valid version,
CELIX_ILLEGAL_ARGUMENT if the string is not a valid version
+ * and CELIX_ENOMEM if memory could not be allocated. Note that on a
CELIX_ILLEGAL_ARGUMENT the version will be set to a
+ * copy of the defaultValue.
+ */
+CELIX_UTILS_EXPORT celix_status_t celix_utils_convertStringToVersion(const
char* val,
+ const
celix_version_t* defaultValue,
+
celix_version_t** version);
+
+/**
+ * @brief Convert a string to a celix_array_list_t* with long entries.
+ *
+ * The expected format of the string is a "," separated list of longs.
Whitespace is ignored.
+ * Long entries are created using celix_utils_convertStringToLong.
+ *
+ * @param[in] val The string to convert.
+ * @param[in] defaultValue The default value if the string is not a valid ","
separated list of longs.
+ * @param[out] list The converted list. If the string is not a valid list, the
list will be set to a copy of the
+ * defaultValue.
+ */
+CELIX_UTILS_EXPORT
+celix_status_t celix_utils_convertStringToLongArrayList(const char* val,
+ const
celix_array_list_t* defaultValue,
+ celix_array_list_t**
list);
+
+/**
+ * @brief Convert a celix_array_list_t* with long entries to a string.
+ *
+ * @param[in] list The list to convert.
+ * @return The string representation of the list. The returned string is
allocated and should be freed.
+ */
+CELIX_UTILS_EXPORT
+char* celix_utils_longArrayListToString(const celix_array_list_t* list);
+
+/**
+ * @brief Convert a string to a celix_array_list_t* with double entries.
+ *
+ * The expected format of the string is a "," separated list of doubles.
Whitespace is ignored.
+ * Double entries are created using celix_utils_convertStringToDouble.
+ *
+ * @param[in] val The string to convert.
+ * @param[in] defaultValue The default value if the string is not a valid ","
separated list of doubles.
+ * @param[out] list The converted list. If the string is not a valid list, the
list will be set to a copy of the
+ * defaultValue.
+ */
+CELIX_UTILS_EXPORT
+celix_status_t celix_utils_convertStringToDoubleArrayList(const char* val,
+ const
celix_array_list_t* defaultValue,
+ celix_array_list_t**
list);
+
+/**
+ * @brief Convert a celix_array_list_t* with double entries to a string.
+ *
+ * @param[in] list The list to convert.
+ * @return The string representation of the list. The returned string is
allocated and should be freed.
+ */
+CELIX_UTILS_EXPORT
+char* celix_utils_doubleArrayListToString(const celix_array_list_t* list);
+
+/**
+ * @brief Convert a string to a celix_array_list_t* with boolean entries.
+ *
+ * The expected format of the string is a "," separated list of booleans.
Whitespace is ignored.
+ * Boolean entries are converted using celix_utils_convertStringToBool.
+ *
+ * @param[in] val The string to convert.
+ * @param[in] defaultValue The default value if the string is not a valid ","
separated list of booleans.
+ * @param[out] list The converted list. If the string is not a valid list, the
list will be set to a copy of the
+ * defaultValue.
+ */
+CELIX_UTILS_EXPORT
+celix_status_t celix_utils_convertStringToBoolArrayList(const char* val,
+ const
celix_array_list_t* defaultValue,
+ celix_array_list_t**
list);
+
+/**
+ * @brief Convert a celix_array_list_t* with boolean entries to a string.
+ *
+ * @param[in] list The list to convert.
+ * @return The string representation of the list. The returned string is
allocated and should be freed.
+ */
+CELIX_UTILS_EXPORT
+char* celix_utils_boolArrayListToString(const celix_array_list_t* list);
+
+/**
+ * @brief Convert a string to a celix_array_list_t* with string entries.
+ *
+ * The expected format of the string is a "," separated list of strings.
Whitespace is preserved.
+ * String entries are copied and the returned list will be configured to call
free when entries are removed.
+ *
+ * The escaped character is "\" and can be used to escape "," and "\"
characters.
+ * E.g. "a,b\,\\,c" will be converted to "a", "b,\" and "c".
+ *
+ * @param[in] val The string to convert.
+ * @param[in] defaultValue The default value if the string is not a valid ","
separated list of strings.
+ * Note that the defaultValue is copied if the string
is not a valid list of string entries
+ * and the defaultValue is expected to be configured
with a removed entry callback so the
Review Comment:
An object is responsible for releasing all its resources, which is taken by
granted by our users. What is missing here is that if the provided string is
invalid, then the resulting `list` will depend on `defaultValue`, which means
accessing `list` after `defaultValue` destructed will be use-after-free.
##########
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) {
Review Comment:
Other return values, especially `CELIX_ILLEGAL_ARGUMENT` are ignored.
##########
libs/utils/src/version.c:
##########
@@ -159,92 +160,68 @@ void celix_version_destroy(celix_version_t* version) {
celix_version_t* celix_version_copy(const celix_version_t* version) {
- if (version == NULL) {
- return celix_version_createEmptyVersion();
+ if (!version) {
+ return NULL;
}
return celix_version_create(version->major, version->minor,
version->micro, version->qualifier);
}
celix_version_t* celix_version_createVersionFromString(const char *versionStr)
{
- if (versionStr == NULL) {
- return NULL;
- }
-
- int major = 0;
- int minor = 0;
- int micro = 0;
- char * qualifier = NULL;
-
- char delims[] = ".";
- char *token = NULL;
- char *last = NULL;
-
- int i = 0;
+ celix_version_t* version;
+ celix_status_t status = celix_version_parse(versionStr, &version);
+ (void)status; //silently ignore status
+ return version;
+}
- char* versionWrkStr = strdup(versionStr);
+celix_status_t celix_version_parse(const char *versionStr, celix_version_t**
version) {
+ *version = NULL;
- celix_status_t status = CELIX_SUCCESS;
- token = strtok_r(versionWrkStr, delims, &last);
- if (token != NULL) {
- for (i = 0; i < strlen(token); i++) {
- char ch = token[i];
- if (('0' <= ch) && (ch <= '9')) {
- continue;
- }
- status = CELIX_ILLEGAL_ARGUMENT;
- break;
- }
- major = atoi(token);
- token = strtok_r(NULL, delims, &last);
- if (token != NULL) {
- for (i = 0; i < strlen(token); i++) {
- char ch = token[i];
- if (('0' <= ch) && (ch <= '9')) {
- continue;
- }
- status = CELIX_ILLEGAL_ARGUMENT;
- break;
- }
- minor = atoi(token);
- token = strtok_r(NULL, delims, &last);
- if (token != NULL) {
- for (i = 0; i < strlen(token); i++) {
- char ch = token[i];
- if (('0' <= ch) && (ch <= '9')) {
- continue;
- }
- status = CELIX_ILLEGAL_ARGUMENT;
- break;
- }
- micro = atoi(token);
- token = strtok_r(NULL, delims, &last);
- if (token != NULL) {
- qualifier = strdup(token);
- token = strtok_r(NULL, delims, &last);
- if (token != NULL) {
- status = CELIX_ILLEGAL_ARGUMENT;
- }
- }
- }
- }
+ if (celix_utils_isStringNullOrEmpty(versionStr)) {
+ return CELIX_ILLEGAL_ARGUMENT;
}
- free(versionWrkStr);
-
- celix_version_t* version = NULL;
- if (status == CELIX_SUCCESS) {
- version = celix_version_create(major, minor, micro, qualifier);
+ char buffer[64];
+ char* versionWrkStr = celix_utils_writeOrCreateString(buffer,
sizeof(buffer), "%s", versionStr);
Review Comment:
This line may lead to unexpect inefficiency. Consider that `celix_filter`
may try to convert arbitrarily long string into version. We'd better avoid any
unnecessary memory allocations.
##########
libs/utils/src/celix_convert_utils.c:
##########
@@ -19,13 +19,18 @@
#include "celix_convert_utils.h"
-#include <stdlib.h>
+#include <assert.h>
+#include <ctype.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
-#include <ctype.h>
+#include "celix_array_list.h"
+#include "celix_err.h"
Review Comment:
The usage of err should be mentioned in API documentation.
--
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
