This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/674-improve-properties in repository https://gitbox.apache.org/repos/asf/celix.git
commit d55c411e440bea62235db7b747ecc65c0e8a6ce0 Author: Pepijn Noltes <[email protected]> AuthorDate: Mon Jan 22 19:27:28 2024 +0100 Fix memleak in properties --- .../gtest/src/PropertiesErrorInjectionTestSuite.cc | 2 +- libs/utils/src/properties.c | 48 +++++++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc index 16778ec3..c1fa7d8b 100644 --- a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc @@ -129,7 +129,7 @@ TEST_F(PropertiesErrorInjectionTestSuite, SetFailureTest) { ASSERT_EQ(celix_properties_set(props, "key", "value"), CELIX_ENOMEM); // When a celix_stringHashMap_put error injection is set for celix_properties_set with level 1 (during put) - celix_ei_expect_celix_stringHashMap_put((void*)celix_properties_set, 1, CELIX_ENOMEM); + celix_ei_expect_celix_stringHashMap_put((void*)celix_properties_set, 2, CELIX_ENOMEM); // Then the celix_properties_set call fails ASSERT_EQ(celix_properties_set(props, "key", "value"), CELIX_ENOMEM); diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c index 38055b2c..1fd9753e 100644 --- a/libs/utils/src/properties.c +++ b/libs/utils/src/properties.c @@ -215,26 +215,28 @@ static celix_properties_entry_t* celix_properties_createEntryWithNoCopy(celix_pr return entry; } -static void celix_properties_freeTypedEntry(const celix_properties_entry_t* entry) { - if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { - celix_version_destroy((celix_version_t*)entry->typed.versionValue); - } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) { - celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); - } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) { - celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); - } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY) { - celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); - } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING_ARRAY) { - celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); - } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION_ARRAY) { - celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); +static void celix_properties_freeTypedEntry(celix_properties_t* properties, celix_properties_entry_t* entry) { + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) { + celix_properties_freeString(properties, (char*)entry->typed.strValue); + entry->typed.strValue = NULL; + entry->value = NULL; + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION) { + celix_version_destroy((celix_version_t*)entry->typed.versionValue); + entry->typed.versionValue = NULL; + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY || + entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY || + entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY || + entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING_ARRAY || + entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION_ARRAY) { + celix_arrayList_destroy((celix_array_list_t*)entry->typed.arrayValue); + entry->typed.arrayValue = NULL; } else { // nop } } static void celix_properties_destroyEntry(celix_properties_t* properties, celix_properties_entry_t* entry) { - celix_properties_freeTypedEntry(entry); + celix_properties_freeTypedEntry(properties, entry); celix_properties_freeString(properties, (char*)entry->value); if (entry >= properties->entriesBuffer && entry <= (properties->entriesBuffer + CELIX_PROPERTIES_OPTIMIZATION_ENTRIES_BUFFER_SIZE)) { @@ -256,10 +258,10 @@ static void celix_properties_destroyEntry(celix_properties_t* properties, celix_ * 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* prototype) { celix_properties_entry_t* entry = celix_properties_allocEntry(properties); if (entry == NULL) { - celix_properties_freeTypedEntry(prototype); + celix_properties_freeTypedEntry(properties, prototype); celix_err_pushf("Cannot allocate property entry"); return NULL; } @@ -279,9 +281,9 @@ static celix_properties_entry_t* celix_properties_createEntry(celix_properties_t */ static celix_status_t celix_properties_createAndSetEntry(celix_properties_t* properties, const char* key, - const celix_properties_entry_t* prototype) { + celix_properties_entry_t* prototype) { if (!properties || !key) { - celix_properties_freeTypedEntry(prototype); + celix_properties_freeTypedEntry(properties, prototype); if (!properties) { return CELIX_SUCCESS; // silently ignore } @@ -613,7 +615,7 @@ celix_properties_t* celix_properties_copy(const celix_properties_t* properties) CELIX_PROPERTIES_ITERATE(properties, iter) { celix_status_t status; if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) { - status = celix_properties_set(copy, iter.key, iter.entry.value); + status = celix_properties_setString(copy, iter.key, iter.entry.value); } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG) { status = celix_properties_setLong(copy, iter.key, iter.entry.typed.longValue); } else if (iter.entry.valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { @@ -756,7 +758,13 @@ const char* celix_properties_getAsString(const celix_properties_t* properties, celix_status_t celix_properties_setString(celix_properties_t* properties, const char* key, const char* value) { - char* copy = properties ? celix_properties_createString(properties, value) : NULL; + if (!properties) { + return CELIX_SUCCESS; // silently ignore NULL properties + } + char* copy = celix_properties_createString(properties, value); + if (!copy) { + return CELIX_ENOMEM; + } celix_properties_entry_t prototype = {0}; prototype.valueType = CELIX_PROPERTIES_VALUE_TYPE_STRING; prototype.typed.strValue = copy;
