PengZheng commented on code in PR #721: URL: https://github.com/apache/celix/pull/721#discussion_r1546338774
########## libs/utils/src/properties.c: ########## @@ -138,33 +138,43 @@ static celix_status_t celix_properties_fillEntry(celix_properties_t* properties, celix_properties_entry_t* entry, const celix_properties_entry_t* prototype) { char convertedValueBuffer[21] = {0}; - *entry = *prototype; - if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { - bool written = celix_version_fillString(prototype->typed.versionValue, convertedValueBuffer, sizeof(convertedValueBuffer)); + memcpy(entry, prototype, sizeof(*entry)); + entry->value = NULL; + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { + bool written = celix_version_fillString(entry->typed.versionValue, convertedValueBuffer, sizeof(convertedValueBuffer)); if (written) { entry->value = celix_properties_createString(properties, convertedValueBuffer); } else { - entry->value = celix_version_toString(prototype->typed.versionValue); + entry->value = celix_version_toString(entry->typed.versionValue); } - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { // LONG_MAX str is 19 chars, LONG_MIN str is 20 chars (void)snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%li", entry->typed.longValue); entry->value = celix_properties_createString(properties, convertedValueBuffer); - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { int written = snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue); - if (written >= 0 || written < sizeof(convertedValueBuffer)) { + if (written >= 0 && written < sizeof(convertedValueBuffer)) { entry->value = celix_properties_createString(properties, convertedValueBuffer); } else { char* val = NULL; asprintf(&val, "%f", entry->typed.doubleValue); entry->value = val; } - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { entry->value = entry->typed.boolValue ? CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL; + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) { + entry->value = celix_utils_longArrayListToString(entry->typed.arrayValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) { + entry->value = celix_utils_doubleArrayListToString(entry->typed.arrayValue); Review Comment: > Do we also want to support / JSON mixed arrays. So something like: [true, 1, 2.3, "string", "1.2.3.qualified"] ? As a dynamically typed language, JS can support heterogeneous arrays. As a statically typed language, Java supports heterogeneous arrays via "the mother of all" Object. Here we may need a generic "entry". ########## libs/utils/src/array_list.c: ########## @@ -28,6 +28,14 @@ #include "celix_utils.h" #include "celix_version.h" +#define STRING_VALUE_UNDEFINED_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED" Review Comment: I think "Undefined"/"undefined" will have better readability than "CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED" as console output. ########## libs/utils/src/properties.c: ########## @@ -138,33 +138,43 @@ static celix_status_t celix_properties_fillEntry(celix_properties_t* properties, celix_properties_entry_t* entry, const celix_properties_entry_t* prototype) { char convertedValueBuffer[21] = {0}; - *entry = *prototype; - if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { - bool written = celix_version_fillString(prototype->typed.versionValue, convertedValueBuffer, sizeof(convertedValueBuffer)); + memcpy(entry, prototype, sizeof(*entry)); + entry->value = NULL; + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { + bool written = celix_version_fillString(entry->typed.versionValue, convertedValueBuffer, sizeof(convertedValueBuffer)); if (written) { entry->value = celix_properties_createString(properties, convertedValueBuffer); } else { - entry->value = celix_version_toString(prototype->typed.versionValue); + entry->value = celix_version_toString(entry->typed.versionValue); } - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { // LONG_MAX str is 19 chars, LONG_MIN str is 20 chars (void)snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%li", entry->typed.longValue); entry->value = celix_properties_createString(properties, convertedValueBuffer); - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { int written = snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue); - if (written >= 0 || written < sizeof(convertedValueBuffer)) { + if (written >= 0 && written < sizeof(convertedValueBuffer)) { entry->value = celix_properties_createString(properties, convertedValueBuffer); } else { char* val = NULL; asprintf(&val, "%f", entry->typed.doubleValue); entry->value = val; } - } else if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL) { entry->value = entry->typed.boolValue ? CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL; + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) { + entry->value = celix_utils_longArrayListToString(entry->typed.arrayValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) { + entry->value = celix_utils_doubleArrayListToString(entry->typed.arrayValue); Review Comment: > I am still a a bit worried about JSON serialization and especially empty ([]) arrays. Yeah, this is an annoying case. -- 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