PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398127100


##########
libs/utils/src/properties.c:
##########
@@ -103,12 +131,265 @@ static void updateBuffers(char **key, char ** value, 
char **output, int outputPo
     }
 }
 
-static void parseLine(const char* line, celix_properties_t *props) {
-    int linePos = 0;
+/**
+ * Create a new string from the provided str by either using strdup or storing 
the string the short properties
+ * optimization string buffer.
+ */
+char* celix_properties_createString(celix_properties_t* properties, const 
char* str) {
+    if (str == NULL) {
+        return (char*)CELIX_PROPERTIES_EMPTY_STRVAL;
+    }
+    size_t len = strnlen(str, CELIX_UTILS_MAX_STRLEN) + 1;
+    size_t left = CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE - 
properties->currentStringBufferIndex;
+    char* result;
+    if (len < left) {
+        
memcpy(&properties->stringBuffer[properties->currentStringBufferIndex], str, 
len);
+        result = 
&properties->stringBuffer[properties->currentStringBufferIndex];
+        properties->currentStringBufferIndex += (int)len;
+    } else {
+        result = celix_utils_strdup(str);
+    }
+    return result;
+}
+/**
+ * Free string, but first check if it a static const char* const string or 
part of the short properties
+ * optimization.
+ */
+static void celix_properties_freeString(celix_properties_t* properties, char* 
str) {
+    if (str == CELIX_PROPERTIES_BOOL_TRUE_STRVAL || str == 
CELIX_PROPERTIES_BOOL_FALSE_STRVAL ||
+        str == CELIX_PROPERTIES_EMPTY_STRVAL) {
+        // str is static const char* const -> nop
+    } else if (str >= properties->stringBuffer &&
+               str < (properties->stringBuffer + 
CELIX_PROPERTIES_OPTIMIZATION_STRING_BUFFER_SIZE)) {
+        // str is part of the properties string buffer -> nop
+    } else {
+        free(str);
+    }
+}
+
+/**
+ * Fill entry and optional use the short properties optimization string buffer.
+ */
+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));
+        if (written) {
+            entry->value = celix_properties_createString(properties, 
convertedValueBuffer);
+        } else {
+            entry->value = 
celix_version_toString(prototype->typed.versionValue);
+        }
+    } else if (prototype->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) {
+        int written = snprintf(convertedValueBuffer, 
sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue);
+        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) {
+        entry->value = entry->typed.boolValue ? 
CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL;
+    } else /*string value*/ {
+        assert(prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING);
+        entry->value = celix_properties_createString(properties, 
prototype->typed.strValue);
+        entry->typed.strValue = entry->value;
+    }
+
+    if (entry->value == NULL) {
+        return CELIX_ENOMEM;
+    }
+    return CELIX_SUCCESS;
+}
+
+/**
+ * Allocate entry and optionally use the short properties optimization entries 
buffer.
+ */
+celix_properties_entry_t* celix_properties_allocEntry(celix_properties_t* 
properties) {
+    celix_properties_entry_t* entry;
+    if (properties->currentEntriesBufferIndex < 
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE) {
+        entry = 
&properties->entriesBuffer[properties->currentEntriesBufferIndex++];
+    } else {
+        entry = malloc(sizeof(*entry));
+    }
+    return entry;
+}
+
+/**
+ * Create entry and optionally use the short properties optimization entries 
buffer and take ownership of the
+ * provided key and value strings.
+ */
+static celix_properties_entry_t* 
celix_properties_createEntryWithNoCopy(celix_properties_t* properties,
+                                                                        const 
char* strValue) {
+    celix_properties_entry_t* entry = celix_properties_allocEntry(properties);
+    if (entry == NULL) {
+        return NULL;
+    }
+    entry->value = strValue;
+    entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING;
+    entry->typed.strValue = strValue;
+    return entry;
+}
+
+/**
+ * Create entry and optionally use the short properties optimization buffers.
+ * Only 1 of the types values (strValue, LongValue, etc) should be provided.
+ */
+static celix_properties_entry_t* 
celix_properties_createEntry(celix_properties_t* properties,
+                                                              const 
celix_properties_entry_t* prototype) {
+    celix_properties_entry_t* entry = celix_properties_allocEntry(properties);
+    if (entry == NULL) {
+        if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+            
celix_version_destroy((celix_version_t*)prototype->typed.versionValue);
+        }
+        celix_err_pushf("Cannot allocate property entry");
+        return NULL;
+    }
+
+    celix_status_t status = celix_properties_fillEntry(properties, entry, 
prototype);
+    if (status != CELIX_SUCCESS) {
+        celix_err_pushf("Cannot fill property entry");
+        if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+            
celix_version_destroy((celix_version_t*)prototype->typed.versionValue);
+        }
+        if (entry >= properties->entriesBuffer &&
+            entry <= (properties->entriesBuffer + 
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE)) {
+            // entry is part of the properties entries buffer -> decrease the 
currentEntriesBufferIndex
+            properties->currentEntriesBufferIndex -= 1;
+        } else {
+            free(entry);
+        }
+        entry = NULL;
+    }
+    return entry;
+}
+
+static void celix_properties_destroyEntry(celix_properties_t* properties, 
celix_properties_entry_t* entry) {
+    celix_properties_freeString(properties, (char*)entry->value);
+    if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+        celix_version_destroy((celix_version_t*)entry->typed.versionValue);
+    }
+
+    if (entry >= properties->entriesBuffer &&
+        entry <= (properties->entriesBuffer + 
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE)) {
+        // entry is part of the properties entries buffer -> nop.
+    } else {
+        free(entry);
+    }
+}
+
+/**
+ * Create and add entry and optionally use the short properties optimization 
buffers.
+ * The prototype is used to determine the type of the value.
+ */
+static celix_status_t celix_properties_createAndSetEntry(celix_properties_t* 
properties,
+                                                         const char* key,
+                                                         const 
celix_properties_entry_t* prototype) {
+    if (!properties) {
+        if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+            
celix_version_destroy((celix_version_t*)prototype->typed.versionValue);
+        }
+        return CELIX_SUCCESS; // silently ignore
+    }
+
+    if (!key) {
+        if (prototype->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+            
celix_version_destroy((celix_version_t*)prototype->typed.versionValue);
+        }
+        celix_err_pushf("Cannot set property with NULL key");
+        return CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    celix_properties_entry_t* entry = celix_properties_createEntry(properties, 
prototype);
+    if (!entry) {
+        return CELIX_ENOMEM;
+    }
+
+    const char* mapKey = key;
+    if (!celix_stringHashMap_hasKey(properties->map, key)) {
+        // new entry, needs new allocated key;
+        mapKey = celix_properties_createString(properties, key);
+        if (!mapKey) {
+            celix_properties_destroyEntry(properties, entry);
+            return CELIX_ENOMEM;
+        }
+    }
+
+    celix_status_t status = celix_stringHashMap_put(properties->map, mapKey, 
entry);
+    if (status != CELIX_SUCCESS) {
+        celix_properties_destroyEntry(properties, entry);
+        if (mapKey != key) {
+            celix_properties_freeString(properties, (char*)mapKey);
+        }
+    }
+    return status;
+}
+
+static void celix_properties_removeKeyCallback(void* handle, char* key) {
+    celix_properties_t* properties = handle;
+    celix_properties_freeString(properties, key);
+}
+
+static void celix_properties_removeEntryCallback(void* handle,
+                                                 const char* key 
__attribute__((unused)),
+                                                 celix_hash_map_value_t val) {
+    celix_properties_t* properties = handle;
+    celix_properties_entry_t* entry = val.ptrValue;
+    celix_properties_destroyEntry(properties, entry);
+}
+
+celix_properties_t* celix_properties_create() {
+    celix_properties_t* props = malloc(sizeof(*props));
+    if (props != NULL) {
+        celix_string_hash_map_create_options_t opts = 
CELIX_EMPTY_STRING_HASH_MAP_CREATE_OPTIONS;
+        opts.storeKeysWeakly = true;
+        opts.initialCapacity = (unsigned 
int)ceil(CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE / 0.75);

Review Comment:
   ```suggestion
           opts.initialCapacity = 
CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE;
   ```
   A default load factor of 0.75 no longer applies.



-- 
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