diff --git a/libs/utils/gtest/src/HashMapTestSuite.cc b/libs/utils/gtest/src/HashMapTestSuite.cc index fed19b5e3..901495300 100644 --- a/libs/utils/gtest/src/HashMapTestSuite.cc +++ b/libs/utils/gtest/src/HashMapTestSuite.cc @@ -615,11 +615,15 @@ TEST_F(HashMapTestSuite, StoreKeysWeaklyTest) { free(key); }; auto* sMap = celix_stringHashMap_createWithOptions(&opts); - EXPECT_FALSE(celix_stringHashMap_putLong(sMap, celix_utils_strdup("key1"), 1)); // new key -> takes ownership - EXPECT_TRUE(celix_stringHashMap_putLong(sMap, "key1", 2)); // replace key -> takes no ownership - - EXPECT_FALSE(celix_stringHashMap_putLong(sMap, celix_utils_strdup("key2"), 3)); // new key -> takes ownership - EXPECT_TRUE(celix_stringHashMap_putLong(sMap, "key2", 4)); // replace key -> takes no ownership + EXPECT_FALSE(celix_stringHashMap_hasKey(sMap, "key1")); + EXPECT_EQ(CELIX_SUCCESS, celix_stringHashMap_putLong(sMap, celix_utils_strdup("key1"), 1)); // new key -> takes ownership + EXPECT_TRUE(celix_stringHashMap_hasKey(sMap, "key1")); + EXPECT_EQ(CELIX_SUCCESS, celix_stringHashMap_putLong(sMap, "key1", 2)); // replace key -> takes no ownership + + EXPECT_FALSE(celix_stringHashMap_hasKey(sMap, "key2")); + EXPECT_EQ(CELIX_SUCCESS, celix_stringHashMap_putLong(sMap, celix_utils_strdup("key2"), 3)); // new key -> takes ownership + EXPECT_TRUE(celix_stringHashMap_hasKey(sMap, "key2")); + EXPECT_EQ(CELIX_SUCCESS, celix_stringHashMap_putLong(sMap, "key2", 4)); // replace key -> takes no ownership celix_stringHashMap_remove(sMap, "key1"); celix_stringHashMap_destroy(sMap); @@ -639,6 +643,9 @@ TEST_F(HashMapTestSuite, StringHashMapEqualsTest) { celix_autoptr(celix_string_hash_map_t) map1 = celix_stringHashMap_create(); celix_autoptr(celix_string_hash_map_t) map2 = celix_stringHashMap_create(); EXPECT_TRUE(celix_stringHashMap_equals(map1, map2)); + EXPECT_FALSE(celix_stringHashMap_equals(map1, nullptr)); + EXPECT_FALSE(celix_stringHashMap_equals(nullptr, map2)); + EXPECT_TRUE(celix_stringHashMap_equals(nullptr, nullptr)); celix_stringHashMap_putLong(map1, "key1", 42); celix_stringHashMap_putBool(map1, "key2", true); @@ -657,6 +664,9 @@ TEST_F(HashMapTestSuite, LongHashMapEqualsTest) { celix_autoptr(celix_long_hash_map_t) map1 = celix_longHashMap_create(); celix_autoptr(celix_long_hash_map_t) map2 = celix_longHashMap_create(); EXPECT_TRUE(celix_longHashMap_equals(map1, map2)); + EXPECT_FALSE(celix_longHashMap_equals(map1, nullptr)); + EXPECT_FALSE(celix_longHashMap_equals(nullptr, map2)); + EXPECT_TRUE(celix_longHashMap_equals(nullptr, nullptr)); celix_longHashMap_putLong(map1, 1, 42); celix_longHashMap_putBool(map1, 2, true); diff --git a/libs/utils/include/celix_string_hash_map.h b/libs/utils/include/celix_string_hash_map.h index f3ce3e421..72fb869f7 100644 --- a/libs/utils/include/celix_string_hash_map.h +++ b/libs/utils/include/celix_string_hash_map.h @@ -319,6 +319,8 @@ CELIX_UTILS_EXPORT void celix_stringHashMap_clear(celix_string_hash_map_t* map); * Equals means that both maps have the same number of entries and that all entries in the first map * are also present in the second map and have the same value. * + * The value are compared using memcpy, so for pointer values the pointer value is compared and not the value itself. + * * @param[in] map1 The first map to compare. * @param[in] map2 The second map to compare. * @return true if the maps are equal, false otherwise. @@ -359,6 +361,8 @@ CELIX_UTILS_EXPORT void celix_stringHashMapIterator_next(celix_string_hash_map_i /** * @brief Compares two celix_string_hash_map_iterator_t objects for equality. * + * The value are compared using memcpy, so for pointer values the pointer value is compared and not the value itself. + * * @param[in] iterator The first iterator to compare. * @param[in] other The second iterator to compare. * @return true if the iterators point to the same entry in the same hash map, false otherwise. diff --git a/libs/utils/src/celix_hash_map.c b/libs/utils/src/celix_hash_map.c index 58ca3f4e8..7fe4f7124 100644 --- a/libs/utils/src/celix_hash_map.c +++ b/libs/utils/src/celix_hash_map.c @@ -592,18 +592,14 @@ void celix_longHashMap_clear(celix_long_hash_map_t* map) { } static bool celix_hashMap_equals(const celix_hash_map_t* map1, const celix_hash_map_t* map2) { - if (map1 == NULL && map2 == NULL) { - return true; - } - if (map1 == NULL || map2 == NULL) { - return false; - } if (map1->size != map2->size) { return false; } for (celix_hash_map_entry_t* entry = celix_hashMap_firstEntry(map1); entry != NULL; entry = celix_hashMap_nextEntry(map1, entry)) { - void* value = celix_hashMap_get(map2, entry->key.strKey, entry->key.longKey); - if (value == NULL || memcmp(&entry->value, value, sizeof(entry->value)) != 0) { + celix_hash_map_entry_t* entryMap2 = celix_hashMap_getEntry(map2, entry->key.strKey, entry->key.longKey); + + //note using memcmp, so for void* values the pointer value is compared, not the value itself. + if (entryMap2 == NULL || memcmp(&entryMap2->value, &entry->value, sizeof(entryMap2->value)) != 0) { return false; } } @@ -611,10 +607,25 @@ static bool celix_hashMap_equals(const celix_hash_map_t* map1, const celix_hash_ } bool celix_stringHashMap_equals(const celix_string_hash_map_t* map1, const celix_string_hash_map_t* map2) { + if (map1 == NULL && map2 == NULL) { + return true; + } + if (map1 == NULL || map2 == NULL) { + return false; + } return celix_hashMap_equals(&map1->genericMap, &map2->genericMap); } bool celix_longHashMap_equals(const celix_long_hash_map_t* map1, const celix_long_hash_map_t* map2) { + if (map1 == NULL && map2 == NULL) { + return true; + } + if (map1 == NULL || map2 == NULL) { + return false; + } + if (map1->genericMap.size != map2->genericMap.size) { + return false; + } return celix_hashMap_equals(&map1->genericMap, &map2->genericMap); }