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

Reply via email to