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

Reply via email to