From d96a84105058e98d393a4554624e4d700cad5d81 Mon Sep 17 00:00:00 2001
From: Daniel Agar <daniel@agar.ca>
Date: Thu, 19 Mar 2020 12:38:50 -0400
Subject: [PATCH] List: handle re-inserting

 - clear sibling on removal
 - add reinsertion test case to test_list
---
 src/include/containers/List.hpp    |  4 ++
 src/systemcmds/tests/test_List.cpp | 76 ++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/src/include/containers/List.hpp b/src/include/containers/List.hpp
index 69cb747e0e..df9f584294 100644
--- a/src/include/containers/List.hpp
+++ b/src/include/containers/List.hpp
@@ -95,6 +95,8 @@ public:
 				_head = _head->getSibling();
 			}
 
+			removeNode->setSibling(nullptr);
+
 			return true;
 		}
 
@@ -109,6 +111,8 @@ public:
 					node->setSibling(nullptr);
 				}
 
+				removeNode->setSibling(nullptr);
+
 				return true;
 			}
 		}
diff --git a/src/systemcmds/tests/test_List.cpp b/src/systemcmds/tests/test_List.cpp
index a06c5887e2..2eeabfbf2c 100644
--- a/src/systemcmds/tests/test_List.cpp
+++ b/src/systemcmds/tests/test_List.cpp
@@ -55,6 +55,7 @@ public:
 	bool test_add();
 	bool test_remove();
 	bool test_range_based_for();
+	bool test_reinsert();
 
 };
 
@@ -63,6 +64,7 @@ bool ListTest::run_tests()
 	ut_run_test(test_add);
 	ut_run_test(test_remove);
 	ut_run_test(test_range_based_for);
+	ut_run_test(test_reinsert);
 
 	return (_tests_failed == 0);
 }
@@ -218,4 +220,78 @@ bool ListTest::test_range_based_for()
 	return true;
 }
 
+bool ListTest::test_reinsert()
+{
+	List<testContainer *> l1;
+
+	// size should be 0 initially
+	ut_compare("size initially 0", l1.size(), 0);
+	ut_assert_true(l1.empty());
+
+	// insert 100
+	for (int i = 0; i < 100; i++) {
+		testContainer *t = new testContainer();
+		t->i = i;
+		l1.add(t);
+
+		ut_compare("size increasing with i", l1.size(), i + 1);
+		ut_assert_true(!l1.empty());
+	}
+
+	// verify full size (100)
+	ut_assert_true(l1.size() == 100);
+	ut_assert_false(l1.empty());
+
+	// test removing elements
+	for (int remove_i = 0; remove_i < 100; remove_i++) {
+
+		ut_assert_false(l1.empty());
+
+		// find node with i == remove_i
+		testContainer *removed = nullptr;
+
+		for (auto t : l1) {
+			if (t->i == remove_i) {
+				ut_assert_true(l1.remove(t));
+				removed = t;
+			}
+		}
+
+		// l1 shouldn't be empty until the very last iteration
+		ut_assert_false(l1.empty());
+
+		// iterate list again to verify removal
+		for (auto t : l1) {
+			ut_assert_true(t->i != remove_i);
+		}
+
+		// size temporarily reduced by 1
+		ut_assert_true(l1.size() == 100 - 1);
+
+		// now re-insert the removed node
+		const size_t sz1 = l1.size();
+		l1.add(removed);
+		const size_t sz2 = l1.size();
+
+		// verify the size increase
+		ut_assert_true(sz2 > sz1);
+
+		// size restored to 100
+		ut_assert_true(l1.size() == 100);
+	}
+
+	// queue shouldn't be empty
+	ut_assert_false(l1.empty());
+	ut_compare("size still 100", l1.size(), 100);
+
+	// delete all elements
+	l1.clear();
+
+	// verify list has been cleared
+	ut_assert_true(l1.empty());
+	ut_compare("size 0", l1.size(), 0);
+
+	return true;
+}
+
 ut_declare_test_c(test_List, ListTest)