pnoltes commented on code in PR #470: URL: https://github.com/apache/celix/pull/470#discussion_r1388473469
########## libs/utils/src/properties.c: ########## @@ -103,12 +132,275 @@ 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_SHORT_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_SHORT_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 char** strValue, + const long* longValue, + const double* doubleValue, + const bool* boolValue, + celix_version_t* versionValue) { + char convertedValueBuffer[32]; + if (strValue != NULL) { + entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING; + entry->value = celix_properties_createString(properties, *strValue); + entry->typed.strValue = entry->value; + } else if (longValue != NULL) { + entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_LONG; + entry->typed.longValue = *longValue; + int written = snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%li", entry->typed.longValue); + if (written < 0 || written >= sizeof(convertedValueBuffer)) { + entry->value = celix_properties_createString(properties, convertedValueBuffer); + } else { + char* val = NULL; + asprintf(&val, "%li", entry->typed.longValue); + entry->value = val; + } + } else if (doubleValue != NULL) { + entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_DOUBLE; + entry->typed.doubleValue = *doubleValue; + 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 (boolValue != NULL) { + entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_BOOL; + entry->typed.boolValue = *boolValue; + entry->value = entry->typed.boolValue ? CELIX_PROPERTIES_BOOL_TRUE_STRVAL : CELIX_PROPERTIES_BOOL_FALSE_STRVAL; + } else /*versionValue*/ { + assert(versionValue != NULL); + entry->valueType = CELIX_PROPERTIES_VALUE_TYPE_VERSION; + entry->typed.versionValue = versionValue; + bool written = celix_version_fillString(versionValue, convertedValueBuffer, sizeof(convertedValueBuffer)); + if (written) { + entry->value = celix_properties_createString(properties, convertedValueBuffer); + } else { + entry->value = celix_version_toString(versionValue); + } + } + 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_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_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 char** strValue, + const long* longValue, + const double* doubleValue, + const bool* boolValue, + celix_version_t* versionValue) { + celix_properties_entry_t* entry = celix_properties_allocEntry(properties); + if (entry == NULL) { + return NULL; + } + + celix_status_t status = + celix_properties_fillEntry(properties, entry, strValue, longValue, doubleValue, boolValue, versionValue); + if (status != CELIX_SUCCESS) { + if (entry >= properties->entriesBuffer && + entry <= (properties->entriesBuffer + CELIX_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_SIZE)) { + // entry is part of the properties entries buffer -> nop. + } 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_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_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. + * Only 1 of the types values (strValue, LongValue, etc) should be provided. + */ +static celix_status_t celix_properties_createAndSetEntry(celix_properties_t* properties, + const char* key, + const char** strValue, + const long* longValue, + const double* doubleValue, + const bool* boolValue, + celix_version_t* versionValue) { + if (!properties) { + return CELIX_SUCCESS; // silently ignore + } + if (!key) { + celix_err_push("Cannot set property with NULL key"); + return CELIX_SUCCESS; // print error and ignore + } + + celix_properties_entry_t* entry = + celix_properties_createEntry(properties, strValue, longValue, doubleValue, boolValue, versionValue); + 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_SHORT_PROPERTIES_OPTIMIZATION_ENTRIES_SIZE / 0.75); + opts.removedCallbackData = props; + opts.removedCallback = celix_properties_removeEntryCallback; + opts.removedKeyCallback = celix_properties_removeKeyCallback; + props->map = celix_stringHashMap_createWithOptions(&opts); + props->currentStringBufferIndex = 0; + props->currentEntriesBufferIndex = 0; + if (props->map == NULL) { + free(props); + props = NULL; + } + } + return props; +} + +void celix_properties_destroy(celix_properties_t* props) { + if (props != NULL) { + celix_stringHashMap_destroy(props->map); + free(props); + } +} + +celix_properties_t* celix_properties_load(const char* filename) { + FILE* file = fopen(filename, "r"); + if (file == NULL) { + celix_err_pushf("Cannot open file '%s'", filename); + return NULL; + } + celix_properties_t* props = celix_properties_loadWithStream(file); + fclose(file); + return props; +} + +static void celix_properties_parseLine(const char* line, celix_properties_t* props) { Review Comment: I agree that this can and should be improved, but I propose to do this in a separate PR: #685. Again to prevent that the scope of this PR becomes much larger. -- 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