pnoltes commented on code in PR #721: URL: https://github.com/apache/celix/pull/721#discussion_r1538104266
########## 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: A single `StringToArrayList` implies that we should detect the types in the string. This is currently not implemented. I most worry about versions (1.2.3 is a valid version, but also 1.2). This also triggered me. If we want a properties to (de)serializer to JSON. Do we also want to support / JSON mixed arrays. So something like: `[true, 1, 2.3, "string", "1.2.3.qualified"]` ? -- 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