diff --git a/CHANGES b/CHANGES index 69b4b26f96..d22a818b2f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,22 @@ +3.2.0-dev.484 | 2020-05-08 11:50:54 -0700 + + * Change timer_list in BroList to be an unordered list. (Tim Wojtulewicz, Corelight) + + This type is used by Conn and Analyzer to hold onto timers being added and + removed. We don't expect the elements in those lists to maintain an order + as the list is being modified. + + * Add ability for List to be ordered/unordered (Tim Wojtulewicz, Corelight) + + This fixes a "bug" with List where remove_nth() can be an O(n) operation + when it doesn't need to be. remove_nth for lists that don't necessarily + need to keep an order can be an O(1) operation instead. + + * Add unit testing for List (Tim Wojtulewicz, Corelight) + + * Fix bug with List where replace() doesn't work with non-pointer types (Tim Wojtulewicz, Corelight) + 3.2.0-dev.478 | 2020-05-08 11:47:38 -0700 * Added examples to set_to_regex comments (James Lagermann, Corelight) diff --git a/VERSION b/VERSION index 37c79c0fa6..d0c029b134 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.478 +3.2.0-dev.484 diff --git a/src/BroList.h b/src/BroList.h index 48e9ebee04..a37f40d84a 100644 --- a/src/BroList.h +++ b/src/BroList.h @@ -23,4 +23,4 @@ class Attr; typedef PList attr_list; class Timer; -typedef PList timer_list; +typedef PList timer_list; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8e533f5733..facc913a59 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -246,6 +246,7 @@ set(MAIN_SRCS IntSet.cc IP.cc IPAddr.cc + List.cc Reporter.cc NFA.cc Net.cc diff --git a/src/List.cc b/src/List.cc new file mode 100644 index 0000000000..c19353b341 --- /dev/null +++ b/src/List.cc @@ -0,0 +1,130 @@ +#include +#include <3rdparty/doctest.h> + +TEST_CASE("list construction") + { + List list; + CHECK(list.empty()); + + List list2(10); + CHECK(list2.empty()); + CHECK(list2.max() == 10); + } + +TEST_CASE("list operation") + { + List list({ 1, 2, 3 }); + CHECK(list.size() == 3); + CHECK(list.max() == 3); + CHECK(list[0] == 1); + CHECK(list[1] == 2); + CHECK(list[2] == 3); + + // push_back forces a resize of the list here, which grows the list + // by a growth factor. That makes the max elements equal to 6. + list.push_back(4); + CHECK(list.size() == 4); + CHECK(list.max() == 6); + CHECK(list[3] == 4); + + CHECK(list.front() == 1); + CHECK(list.back() == 4); + + list.pop_front(); + CHECK(list.size() == 3); + CHECK(list.front() == 2); + + list.pop_back(); + CHECK(list.size() == 2); + CHECK(list.back() == 3); + + list.push_back(4); + CHECK(list.is_member(2)); + CHECK(list.member_pos(2) == 0); + + list.remove(2); + CHECK(list.size() == 2); + CHECK(list[0] == 3); + CHECK(list[1] == 4); + + // Squash the list down to the existing elements. + list.resize(); + CHECK(list.size() == 2); + CHECK(list.max() == 2); + + // Attempt replacing a known position. + int old = list.replace(0, 10); + CHECK(list.size() == 2); + CHECK(list.max() == 2); + CHECK(old == 3); + CHECK(list[0] == 10); + CHECK(list[1] == 4); + + // Attempt replacing an element off the end of the list, which + // causes a resize. + old = list.replace(3, 5); + CHECK(list.size() == 4); + CHECK(list.max() == 4); + CHECK(old == 0); + CHECK(list[0] == 10); + CHECK(list[1] == 4); + CHECK(list[2] == 0); + CHECK(list[3] == 5); + + // Attempt replacing an element with a negative index, which returns the + // default value for the list type. + old = list.replace(-1, 50); + CHECK(list.size() == 4); + CHECK(list.max() == 4); + CHECK(old == 0); + + list.clear(); + CHECK(list.size() == 0); + CHECK(list.max() == 0); + } + +TEST_CASE("list iteration") + { + List list({ 1, 2, 3, 4}); + + int index = 1; + for ( int v : list ) + CHECK(v == index++); + + index = 1; + for ( auto it = list.begin(); it != list.end(); index++, ++it) + CHECK(*it == index); + } + +TEST_CASE("plists") + { + PList list; + list.push_back(new int(1)); + list.push_back(new int(2)); + list.push_back(new int(3)); + + CHECK(*list[0] == 1); + + int* new_val = new int(5); + auto old = list.replace(-1, new_val); + delete new_val; + CHECK(old == nullptr); + + for ( auto v : list ) + delete v; + list.clear(); + } + +TEST_CASE("unordered list operation") + { + List list({1, 2, 3, 4}); + CHECK(list.size() == 4); + + // An unordered list doesn't maintain the ordering of the elements when + // one is removed. It just swaps the last element into the hole. + list.remove(2); + CHECK(list.size() == 3); + CHECK(list[0] == 1); + CHECK(list[1] == 4); + CHECK(list[2] == 3); + } diff --git a/src/List.h b/src/List.h index d4b9798e4d..c6d0902013 100644 --- a/src/List.h +++ b/src/List.h @@ -29,7 +29,9 @@ // TODO: this can be removed in v3.1 when List::sort() is removed typedef int (*list_cmp_func)(const void* v1, const void* v2); -template +enum class ListOrder : int { ORDERED, UNORDERED }; + +template class List { public: @@ -195,13 +197,11 @@ public: bool remove(const T& a) // delete entry from list { - for ( int i = 0; i < num_entries; ++i ) + int pos = member_pos(a); + if ( pos != -1 ) { - if ( a == entries[i] ) - { - remove_nth(i); - return true; - } + remove_nth(pos); + return true; } return false; @@ -212,10 +212,22 @@ public: assert(n >=0 && n < num_entries); T old_ent = entries[n]; - --num_entries; - for ( ; n < num_entries; ++n ) - entries[n] = entries[n+1]; + // For data where we don't care about ordering, we don't care about keeping + // the list in the same order when removing an element. Just swap the last + // element with the element being removed. + if constexpr ( Order == ListOrder::ORDERED ) + { + --num_entries; + + for ( ; n < num_entries; ++n ) + entries[n] = entries[n+1]; + } + else + { + entries[n] = entries[num_entries - 1]; + --num_entries; + } return old_ent; } @@ -240,16 +252,16 @@ public: T replace(int ent_index, const T& new_ent) // replace entry #i with a new value { if ( ent_index < 0 ) - return 0; + return T{}; - T old_ent = nullptr; + T old_ent{}; if ( ent_index > num_entries - 1 ) { // replacement beyond the end of the list resize(ent_index + 1); for ( int i = num_entries; i < max_entries; ++i ) - entries[i] = nullptr; + entries[i] = T{}; num_entries = max_entries; } else @@ -318,8 +330,8 @@ protected: // Specialization of the List class to store pointers of a type. -template -using PList = List; +template +using PList = List; // Popular type of list: list of strings. typedef PList name_list;