Erase elements in LinkedList::remove_if
Change-Id: I5119a78c73ffe780a81c53ab5ff0266d5c82d319
diff --git a/linker/linked_list.h b/linker/linked_list.h
index e51eb9c..14fe1e5 100644
--- a/linker/linked_list.h
+++ b/linker/linked_list.h
@@ -88,24 +88,50 @@
template<typename F>
void for_each(F&& action) {
for (LinkedListEntry<T>* e = head_; e != nullptr; e = e->next) {
- if (e->element != nullptr) {
- action(e->element);
- }
+ action(e->element);
}
}
template<typename F>
- void remove_if(F&& predicate) {
- for (LinkedListEntry<T>* e = head_; e != nullptr; e = e->next) {
- if (e->element != nullptr && predicate(e->element)) {
- e->element = nullptr;
+ void remove_if(F predicate) {
+ for (LinkedListEntry<T>* e = head_, *p = nullptr; e != nullptr;) {
+ if (predicate(e->element)) {
+ LinkedListEntry<T>* next = e->next;
+ if (p == nullptr) {
+ head_ = next;
+ } else {
+ p->next = next;
+ }
+ Allocator::free(e);
+ e = next;
+ } else {
+ p = e;
+ e = e->next;
}
}
}
- bool contains(const T* el) {
+ size_t size() const {
+ size_t sz = 0;
for (LinkedListEntry<T>* e = head_; e != nullptr; e = e->next) {
- if (e->element != nullptr && e->element == el) {
+ ++sz;
+ }
+
+ return sz;
+ }
+
+ size_t copy_to_array(T* array[], size_t array_length) const {
+ size_t sz = 0;
+ for (LinkedListEntry<T>* e = head_; sz < array_length && e != nullptr; e = e->next) {
+ array[sz++] = e->element;
+ }
+
+ return sz;
+ }
+
+ bool contains(const T* el) const {
+ for (LinkedListEntry<T>* e = head_; e != nullptr; e = e->next) {
+ if (e->element == el) {
return true;
}
}
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 91ec49f..3be2ec6 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -609,6 +609,8 @@
return NULL;
}
+
+
// Another soinfo list allocator to use in dlsym. We don't reuse
// SoinfoListAllocator because it is write-protected most of the time.
static LinkerAllocator<LinkedListEntry<soinfo>> g_soinfo_list_allocator_rw;
@@ -861,10 +863,17 @@
si->CallDestructors();
if (si->has_min_version(0)) {
- si->get_children().for_each([&] (soinfo* child) {
- TRACE("%s needs to unload %s", si->name, child->name);
- soinfo_unload(child);
- });
+ // It is not safe to do si->get_children().for_each, because
+ // during soinfo_free the child will concurrently modify the si->children
+ // list, therefore we create a copy and use it to unload children.
+ size_t children_count = si->get_children().size();
+ soinfo* children[children_count];
+ si->get_children().copy_to_array(children, children_count);
+
+ for (size_t i = 0; i < children_count; ++i) {
+ TRACE("%s needs to unload %s", si->name, children[i]->name);
+ soinfo_unload(children[i]);
+ }
} else {
for (ElfW(Dyn)* d = si->dynamic; d->d_tag != DT_NULL; ++d) {
if (d->d_tag == DT_NEEDED) {
@@ -1618,7 +1627,7 @@
});
parents.for_each([&] (soinfo* parent) {
- parent->children.for_each([&] (const soinfo* child) {
+ parent->children.remove_if([&] (const soinfo* child) {
return child == this;
});
});
diff --git a/linker/tests/linked_list_test.cpp b/linker/tests/linked_list_test.cpp
index b9816fa..0483b84 100644
--- a/linker/tests/linked_list_test.cpp
+++ b/linker/tests/linked_list_test.cpp
@@ -80,7 +80,7 @@
});
ASSERT_TRUE(!alloc_called);
- ASSERT_TRUE(!free_called);
+ ASSERT_TRUE(free_called);
ASSERT_EQ("dba", test_list_to_string(list));
alloc_called = free_called = false;
@@ -103,15 +103,82 @@
ASSERT_EQ("ab", test_list_to_string(list));
list.push_back("c");
ASSERT_EQ("abc", test_list_to_string(list));
- ASSERT_EQ("a", list.pop_front());
+ ASSERT_STREQ("a", list.pop_front());
ASSERT_EQ("bc", test_list_to_string(list));
- ASSERT_EQ("b", list.pop_front());
+ ASSERT_STREQ("b", list.pop_front());
ASSERT_EQ("c", test_list_to_string(list));
- ASSERT_EQ("c", list.pop_front());
+ ASSERT_STREQ("c", list.pop_front());
ASSERT_EQ("", test_list_to_string(list));
ASSERT_TRUE(list.pop_front() == nullptr);
list.push_back("r");
ASSERT_EQ("r", test_list_to_string(list));
- ASSERT_EQ("r", list.pop_front());
+ ASSERT_STREQ("r", list.pop_front());
ASSERT_TRUE(list.pop_front() == nullptr);
}
+
+TEST(linked_list, remove_if_then_pop) {
+ test_list_t list;
+ list.push_back("a");
+ list.push_back("b");
+ list.push_back("c");
+ list.push_back("d");
+ list.remove_if([](const char* c) {
+ return *c == 'b' || *c == 'c';
+ });
+
+ ASSERT_EQ("ad", test_list_to_string(list));
+ ASSERT_STREQ("a", list.pop_front());
+ ASSERT_EQ("d", test_list_to_string(list));
+ ASSERT_STREQ("d", list.pop_front());
+ ASSERT_TRUE(list.pop_front() == nullptr);
+}
+
+TEST(linked_list, copy_to_array) {
+ test_list_t list;
+ const size_t max_size = 128;
+ const char* buf[max_size];
+ memset(buf, 0, sizeof(buf));
+
+ ASSERT_EQ(0U, list.size());
+ ASSERT_EQ(0U, list.copy_to_array(buf, max_size));
+ ASSERT_EQ(nullptr, buf[0]);
+
+ list.push_back("a");
+ list.push_back("b");
+ list.push_back("c");
+ list.push_back("d");
+
+ memset(buf, 0, sizeof(buf));
+ ASSERT_EQ(4U, list.size());
+ ASSERT_EQ(2U, list.copy_to_array(buf, 2));
+ ASSERT_EQ('a', *buf[0]);
+ ASSERT_EQ('b', *buf[1]);
+ ASSERT_EQ(nullptr, buf[2]);
+
+ ASSERT_EQ(4U, list.copy_to_array(buf, max_size));
+ ASSERT_EQ('a', *buf[0]);
+ ASSERT_EQ('b', *buf[1]);
+ ASSERT_EQ('c', *buf[2]);
+ ASSERT_EQ('d', *buf[3]);
+ ASSERT_EQ(nullptr, buf[4]);
+
+ memset(buf, 0, sizeof(buf));
+ list.remove_if([](const char* c) {
+ return *c != 'c';
+ });
+ ASSERT_EQ(1U, list.size());
+ ASSERT_EQ(1U, list.copy_to_array(buf, max_size));
+ ASSERT_EQ('c', *buf[0]);
+ ASSERT_EQ(nullptr, buf[1]);
+
+ memset(buf, 0, sizeof(buf));
+
+ list.remove_if([](const char* c) {
+ return *c == 'c';
+ });
+
+ ASSERT_EQ(0U, list.size());
+ ASSERT_EQ(0U, list.copy_to_array(buf, max_size));
+ ASSERT_EQ(nullptr, buf[0]);
+}
+