Skip to content

Commit f982cde

Browse files
authored
Fix a bug in hash_map_get_next_key_and_data. (ros2#375)
What was happening was that we were incorrectly using the previous_key for finding our last "spot" in the hash_map for iteration. That's because of a silly typo. This wasn't picked up in the tests because the tests also had a bug; they were reusing the same key pointer for the previous and the current key, and because of that they didn't pick up on the bug. Fix both of these issues, which fixes iteration over the hash map. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
1 parent 53f952f commit f982cde

2 files changed

Lines changed: 4 additions & 2 deletions

File tree

src/hash_map.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,8 @@ rcutils_hash_map_get_next_key_and_data(
539539
}
540540

541541
if (NULL != previous_key) {
542-
already_exists = hash_map_find(hash_map, key, &key_hash, &map_index, &bucket_index, &entry);
542+
already_exists = hash_map_find(
543+
hash_map, previous_key, &key_hash, &map_index, &bucket_index, &entry);
543544
if (!already_exists) {
544545
return RCUTILS_RET_NOT_FOUND;
545546
}

test/test_hash_map.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,8 @@ TEST_F(HashMapPreInitTest, get_next_key_and_data_working) {
431431
last_key = ret_key;
432432

433433
// Get the next entry
434-
ret = rcutils_hash_map_get_next_key_and_data(&map, &ret_key, &ret_key, &ret_data);
434+
ret_key = 0;
435+
ret = rcutils_hash_map_get_next_key_and_data(&map, &last_key, &ret_key, &ret_data);
435436
EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;
436437
EXPECT_TRUE(1 == ret_key || 2 == ret_key); // we only put these two keys in the map
437438
EXPECT_NE(last_key, ret_key);

0 commit comments

Comments
 (0)