PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1398122940
##########
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;
Review Comment:
Yes, this fixed the issue.
I just noticed we have duplicated code of entry deallocation here and in
`celix_properties_destroyEntry`.
To destroy an entry, we must deinitialise then deallocate it.
In the `celix_properties_deallocEntry`, we can check if the entry is the
last one.
If it is indeed the last one in pool, then we can perform
`currentEntriesBufferIndex -= 1`.
This consideration may/may not also apply to
https://github.com/apache/celix/pull/470#discussion_r1397781840
So that we don't introduce much complexity.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]