diff --git a/NEWS b/NEWS index 778918e60b..cc47e033e6 100644 --- a/NEWS +++ b/NEWS @@ -343,6 +343,11 @@ Deprecated Functionality *e* and *p*, were named in misleading way (*e* is the modulus) and now deprecated in favor of the new *modulus* and *exponent* parameters. +- The ``IterCookie`` version of iteration over ``Dictionary`` and ``PDict`` + objects wars marked as deprecated. It was replaced by standard-library + compatible iterators. This enables the use of standard constructs such + as ranged-for loops to iterate over those objects. + Zeek 3.2.0 ========== diff --git a/src/CompHash.cc b/src/CompHash.cc index c1b283ec46..5d94fba0cd 100644 --- a/src/CompHash.cc +++ b/src/CompHash.cc @@ -210,11 +210,10 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0, kp1 = reinterpret_cast(kp+1); auto tbl = tv->AsTable(); - auto it = tbl->InitForIteration(); auto lv = make_intrusive(TYPE_ANY); struct HashKeyComparer { - bool operator()(const HashKey* a, const HashKey* b) const + bool operator()(const std::unique_ptr& a, const std::unique_ptr& b) const { if ( a->Hash() != b->Hash() ) return a->Hash() < b->Hash(); @@ -226,19 +225,16 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0, } }; - std::map hashkeys; - HashKey* k; + std::map, int, HashKeyComparer> hashkeys; auto idx = 0; - while ( tbl->NextEntry(k, it) ) + for ( const auto& entry : *tbl ) { - hashkeys[k] = idx++; + auto k = entry.GetHashKey(); lv->Append(tv->RecreateIndex(*k)); + hashkeys[std::move(k)] = idx++; } - for ( auto& kv : hashkeys ) - delete kv.first; - for ( auto& kv : hashkeys ) { auto idx = kv.second; @@ -253,11 +249,10 @@ char* CompositeHash::SingleValHash(bool type_check, char* kp0, auto val = tv->FindOrDefault(key); if ( ! (kp1 = SingleValHash(type_check, kp1, val->GetType().get(), - val.get(), false)) ) + val.get(), false)) ) return nullptr; } } - } break; diff --git a/src/Dict.cc b/src/Dict.cc index 0ca02e2337..80402037f1 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -24,7 +24,7 @@ namespace zeek { -class IterCookie { +class [[deprecated("Remove in v5.1. Use the standard-library-compatible version of iteration.")]] IterCookie { public: IterCookie(Dictionary* d) : d(d) {} @@ -184,6 +184,8 @@ TEST_CASE("dict iteration") dict.Insert(key, &val); dict.Insert(key2, &val2); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" detail::HashKey* it_key; IterCookie* it = dict.InitForIteration(); CHECK(it != nullptr); @@ -191,32 +193,269 @@ TEST_CASE("dict iteration") while ( uint32_t* entry = dict.NextEntry(it_key, it) ) { - if ( count == 0 ) + switch ( count ) { - // The DictEntry constructor typecasts this down to a uint32_t, so - // we can't just check the value directly. - // Explanation: hash_t is 64bit, open-dict only uses 32bit hash to - // save space for each item (24 bytes aligned). OpenDict has table - // size of 2^N and only take the lower bits of the hash. (The - // original hash takes transformation in FibHash() to map into a - // smaller 2^N range). - CHECK(it_key->Hash() == (uint32_t)key2->Hash()); - CHECK(*entry == 10); - } - else - { - CHECK(it_key->Hash() == (uint32_t)key->Hash()); - CHECK(*entry == 15); + case 0: + // The DictEntry constructor typecasts this down to a uint32_t, so + // we can't just check the value directly. + // Explanation: hash_t is 64bit, open-dict only uses 32bit hash to + // save space for each item (24 bytes aligned). OpenDict has table + // size of 2^N and only take the lower bits of the hash. (The + // original hash takes transformation in FibHash() to map into a + // smaller 2^N range). + CHECK(it_key->Hash() == (uint32_t)key2->Hash()); + CHECK(*entry == 10); + break; + case 1: + CHECK(it_key->Hash() == (uint32_t)key->Hash()); + CHECK(*entry == 15); + break; + default: + break; } count++; delete it_key; } + CHECK(count == 2); + +#pragma GCC diagnostic pop + delete key; delete key2; } +TEST_CASE("dict new iteration") + { + PDict dict; + + uint32_t val = 15; + uint32_t key_val = 5; + detail::HashKey* key = new detail::HashKey(key_val); + + uint32_t val2 = 10; + uint32_t key_val2 = 25; + detail::HashKey* key2 = new detail::HashKey(key_val2); + + dict.Insert(key, &val); + dict.Insert(key2, &val2); + + int count = 0; + + for ( const auto& entry : dict ) + { + auto* v = static_cast(entry.value); + uint64_t k = *(uint32_t*) entry.GetKey(); + + switch ( count ) + { + case 0: + CHECK(k == key_val2); + CHECK(*v == val2); + break; + case 1: + CHECK(k == key_val); + CHECK(*v == val); + break; + default: + break; + } + + count++; + } + + CHECK(count == 2); + + delete key; + delete key2; + } + +TEST_CASE("dict robust iteration") + { + PDict dict; + + uint32_t val = 15; + uint32_t key_val = 5; + detail::HashKey* key = new detail::HashKey(key_val); + + uint32_t val2 = 10; + uint32_t key_val2 = 25; + detail::HashKey* key2 = new detail::HashKey(key_val2); + + uint32_t val3 = 20; + uint32_t key_val3 = 35; + detail::HashKey* key3 = new detail::HashKey(key_val3); + + dict.Insert(key, &val); + dict.Insert(key2, &val2); + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + detail::HashKey* it_key; + IterCookie* it = dict.InitForIteration(); + CHECK(it != nullptr); + dict.MakeRobustCookie(it); + + int count = 0; + + while ( uint32_t* entry = dict.NextEntry(it_key, it) ) + { + switch ( count ) + { + case 0: + CHECK(it_key->Hash() == (uint32_t)key2->Hash()); + CHECK(*entry == 10); + dict.Insert(key3, &val3); + break; + case 1: + CHECK(it_key->Hash() == (uint32_t)key->Hash()); + CHECK(*entry == 15); + break; + case 2: + CHECK(it_key->Hash() == (uint32_t)key3->Hash()); + CHECK(*entry == 20); + break; + default: + // We shouldn't get here. + CHECK(false); + break; + } + + count++; + delete it_key; + } + + CHECK(count == 3); + + IterCookie* it2 = dict.InitForIteration(); + CHECK(it2 != nullptr); + dict.MakeRobustCookie(it2); + + count = 0; + while ( uint32_t* entry = dict.NextEntry(it_key, it2) ) + { + switch ( count ) + { + case 0: + CHECK(it_key->Hash() == (uint32_t)key2->Hash()); + CHECK(*entry == 10); + dict.Remove(key3); + break; + case 1: + CHECK(it_key->Hash() == (uint32_t)key->Hash()); + CHECK(*entry == 15); + break; + default: + // We shouldn't get here. + CHECK(false); + break; + } + + count++; + delete it_key; + } + + CHECK(count == 2); + +#pragma GCC diagnostic pop + + delete key; + delete key2; + delete key3; + } + +TEST_CASE("dict new robust iteration") + { + PDict dict; + + uint32_t val = 15; + uint32_t key_val = 5; + detail::HashKey* key = new detail::HashKey(key_val); + + uint32_t val2 = 10; + uint32_t key_val2 = 25; + detail::HashKey* key2 = new detail::HashKey(key_val2); + + uint32_t val3 = 20; + uint32_t key_val3 = 35; + detail::HashKey* key3 = new detail::HashKey(key_val3); + + dict.Insert(key, &val); + dict.Insert(key2, &val2); + + { + int count = 0; + auto it = dict.begin_robust(); + + for ( ; it != dict.end_robust(); ++it ) + { + auto* v = it->GetValue(); + uint64_t k = *(uint32_t*) it->GetKey(); + + switch ( count ) + { + case 0: + CHECK(k == key_val2); + CHECK(*v == val2); + dict.Insert(key3, &val3); + break; + case 1: + CHECK(k == key_val); + CHECK(*v == val); + break; + case 2: + CHECK(k == key_val3); + CHECK(*v == val3); + break; + default: + // We shouldn't get here. + CHECK(false); + break; + } + count++; + } + + CHECK(count == 3); + } + + { + int count = 0; + auto it = dict.begin_robust(); + + for ( ; it != dict.end_robust(); ++it ) + { + auto* v = it->GetValue(); + uint64_t k = *(uint32_t*) it->GetKey(); + + switch ( count ) + { + case 0: + CHECK(k == key_val2); + CHECK(*v == val2); + dict.Insert(key3, &val3); + dict.Remove(key3); + break; + case 1: + CHECK(k == key_val); + CHECK(*v == val); + break; + default: + // We shouldn't get here. + CHECK(false); + break; + } + count++; + } + + CHECK(count == 2); + } + + delete key; + delete key2; + delete key3; + } + TEST_CASE("dict iterator invalidation") { PDict dict; @@ -238,6 +477,8 @@ TEST_CASE("dict iterator invalidation") detail::HashKey* it_key; bool iterators_invalidated = false; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" IterCookie* it = dict.InitForIteration(); CHECK(it != nullptr); @@ -277,6 +518,7 @@ TEST_CASE("dict iterator invalidation") dict.StopIteration(it); break; } +#pragma GCC diagnostic pop CHECK(dict.Length() == 2); CHECK(*static_cast(dict.Lookup(key)) == val2); @@ -415,7 +657,7 @@ int Dictionary::OffsetInClusterByPosition(int position) const return position - head; } -// Find the next valid entry after the position. Positiion can be -1, which means +// Find the next valid entry after the position. Position can be -1, which means // look for the next valid entry point altogether. int Dictionary::Next(int position) const { @@ -613,7 +855,7 @@ void Dictionary::Dump(int level) const } } -////////////////////////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////////////////////////// //Initialization. //////////////////////////////////////////////////////////////////////////////////////////////////// Dictionary::Dictionary(DictOrder ordering, int initial_size) @@ -661,6 +903,11 @@ void Dictionary::Clear() delete cookies; cookies = nullptr; } + if ( iterators ) + { + delete iterators; + iterators = nullptr; + } log2_buckets = 0; num_iterators = 0; remaps = 0; @@ -787,7 +1034,7 @@ int Dictionary::LookupIndex(const void* key, int key_size, detail::hash_t hash, /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Insert -////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// void* Dictionary::Insert(void* key, int key_size, detail::hash_t hash, void* val, bool copy_key, bool* iterators_invalidated) { @@ -827,6 +1074,15 @@ void* Dictionary::Insert(void* key, int key_size, detail::hash_t hash, void* val if ( it != c->inserted->end() ) it->value = val; } + + if ( iterators && ! iterators->empty() ) + //need to set new v for iterators too. + for ( auto c: *iterators ) + { + auto it = std::find(c->inserted->begin(), c->inserted->end(), table[position]); + if ( it != c->inserted->end() ) + it->value = val; + } } else { @@ -879,6 +1135,12 @@ void Dictionary::InsertRelocateAndAdjust(detail::DictEntry& entry, int insert_po if ( cookies && ! cookies->empty() ) for ( auto c: *cookies ) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + AdjustOnInsert(c, entry, insert_position, last_affected_position); +#pragma GCC diagnostic pop + if ( iterators && ! iterators->empty() ) + for ( auto c: *iterators ) AdjustOnInsert(c, entry, insert_position, last_affected_position); } @@ -916,6 +1178,9 @@ void Dictionary::InsertAndRelocate(detail::DictEntry& entry, int insert_position } } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + /// Adjust Cookies on Insert. void Dictionary::AdjustOnInsert(IterCookie* c, const detail::DictEntry& entry, int insert_position, int last_affected_position) { @@ -931,6 +1196,21 @@ void Dictionary::AdjustOnInsert(IterCookie* c, const detail::DictEntry& entry, i } } +#pragma GCC diagnostic pop + +void Dictionary::AdjustOnInsert(RobustDictIterator* c, const detail::DictEntry& entry, + int insert_position, int last_affected_position) + { + if ( insert_position < c->next ) + c->inserted->push_back(entry); + if ( insert_position < c->next && c->next <= last_affected_position ) + { + int k = TailOfClusterByPosition(c->next); + ASSERT(k >= 0 && k < Capacity()); + c->visited->push_back(table[k]); + } + } + void Dictionary::SizeUp() { int prev_capacity = Capacity(); @@ -953,7 +1233,7 @@ void Dictionary::SizeUp() /////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Remove -///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// void* Dictionary::Remove(const void* key, int key_size, detail::hash_t hash, bool dont_delete, bool* iterators_invalidated) {//cookie adjustment: maintain inserts here. maintain next in lower level version. @@ -999,6 +1279,13 @@ detail::DictEntry Dictionary::RemoveRelocateAndAdjust(int position) if ( cookies && ! cookies->empty() ) for ( auto c: *cookies ) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + AdjustOnRemove(c, entry, position, last_affected_position); +#pragma GCC diagnostic pop + + if ( iterators && ! iterators->empty() ) + for ( auto c: *iterators ) AdjustOnRemove(c, entry, position, last_affected_position); return entry; @@ -1029,6 +1316,9 @@ detail::DictEntry Dictionary::RemoveAndRelocate(int position, int* last_affected return entry; } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void Dictionary::AdjustOnRemove(IterCookie* c, const detail::DictEntry& entry, int position, int last_affected_position) { ASSERT_VALID(c); @@ -1046,6 +1336,25 @@ void Dictionary::AdjustOnRemove(IterCookie* c, const detail::DictEntry& entry, i c->next = Next(c->next); } +#pragma GCC diagnostic pop + +void Dictionary::AdjustOnRemove(RobustDictIterator* c, const detail::DictEntry& entry, + int position, int last_affected_position) + { + c->inserted->erase(std::remove(c->inserted->begin(), c->inserted->end(), entry), c->inserted->end()); + if ( position < c->next && c->next <= last_affected_position ) + { + int moved = HeadOfClusterByPosition(c->next-1); + if ( moved < position ) + moved = position; + c->inserted->push_back(table[moved]); + } + + //if not already the end of the dictionary, adjust next to a valid one. + if ( c->next < Capacity() && table[c->next].Empty() ) + c->next = Next(c->next); + } + /////////////////////////////////////////////////////////////////////////////////////////////////// //Remap /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -1057,7 +1366,7 @@ void Dictionary::Remap() ///remap from bottom up. ///remap creates two parts of the dict: [0,remap_end] (remap_end, ...]. the former is mixed with old/new entries; the latter contains all new entries. /// - if ( num_iterators ) + if ( num_iterators > 0 ) return; int left = detail::DICT_REMAP_ENTRIES; @@ -1076,7 +1385,7 @@ bool Dictionary::Remap(int position, int* new_position) { ASSERT_VALID(this); ///Remap changes item positions by remove() and insert(). to avoid excessive operation. avoid it when safe iteration is in progress. - ASSERT(! cookies || cookies->empty()); + ASSERT( ( ! cookies || cookies->empty() ) && ( ! iterators || iterators->empty() ) ); int current = BucketByPosition(position);//current bucket int expected = BucketByHash(table[position].hash, log2_buckets); //expected bucket in new table. //equal because 1: it's a new item, 2: it's an old item, but new bucket is the same as old. 50% of old items act this way due to fibhash. @@ -1097,10 +1406,6 @@ bool Dictionary::Remap(int position, int* new_position) return true; } -////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// Iteration -/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - void* Dictionary::NthEntry(int n, const void*& key, int& key_size) const { if ( ! order || n < 0 || n >= Length() ) @@ -1111,6 +1416,13 @@ void* Dictionary::NthEntry(int n, const void*& key, int& key_size) const return entry.value; } +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// Iteration +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void Dictionary::MakeRobustCookie(IterCookie* cookie) { //make sure c->next >= 0. if ( ! cookies ) @@ -1164,21 +1476,10 @@ void* Dictionary::NextEntryNonConst(detail::HashKey*& h, IterCookie*& c, bool re if ( c->next < 0 ) c->next = Next(-1); - // if resize happens during iteration. before sizeup, c->next points to Capacity(), - // but now Capacity() doubles up and c->next doesn't point to the end anymore. - // this is fine because c->next may be filled now. - // however, c->next can also be empty. - // before sizeup, we use c->next >= Capacity() to indicate the end of the iteration. - // now this guard is invalid, we may face c->next is valid but empty now.F - //fix it here. - int capacity = Capacity(); - if ( c->next < capacity && table[c->next].Empty() ) - { - ASSERT(false); //stop to check the condition here. why it's happening. - c->next = Next(c->next); - } + ASSERT(c->next >= Capacity() || ! table[c->next].Empty()); //filter out visited keys. + int capacity = Capacity(); if ( c->visited && ! c->visited->empty() ) //filter out visited entries. while ( c->next < capacity ) @@ -1229,4 +1530,193 @@ void Dictionary::StopIteration(IterCookie* cookie) const dp->StopIterationNonConst(cookie); } +#pragma GCC diagnostic pop + +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// New Iteration +/////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +DictIterator::DictIterator(const Dictionary* d, detail::DictEntry* begin, detail::DictEntry* end) + : curr(begin), end(end) + { + // Make sure that we're starting on a non-empty element. + while ( curr != end && curr->Empty() ) + ++curr; + + // Cast away the constness so that the number of iterators can be modified in the dictionary. This does + // violate the constness guarantees of const-begin()/end() and cbegin()/cend(), but we're not modifying the + // actual data in the collection, just a counter in the wrapper of the collection. + dict = const_cast(d); + dict->num_iterators++; + } + +DictIterator::~DictIterator() + { + assert(dict->num_iterators > 0); + dict->num_iterators--; + } + +DictIterator& DictIterator::operator++() + { + // The non-robust case is easy. Just advanced the current position forward until you find + // one isn't empty and isn't the end. + do { + ++curr; + } + while ( curr != end && curr->Empty() ); + + return *this; + } + + + + +RobustDictIterator Dictionary::MakeRobustIterator() + { + if ( ! iterators ) + iterators = new std::vector; + + return { this }; + } + +detail::DictEntry Dictionary::GetNextRobustIteration(RobustDictIterator* iter) + { + // If there are any inserted entries, return them first. + // That keeps the list small and helps avoiding searching + // a large list when deleting an entry. + if ( ! table ) + { + iter->Complete(); + return detail::DictEntry(nullptr); // end of iteration + } + + if ( iter->inserted && ! iter->inserted->empty() ) + { + // Return the last one. Order doesn't matter, + // and removing from the tail is cheaper. + detail::DictEntry e = iter->inserted->back(); + iter->inserted->pop_back(); + return e; + } + + if ( iter->next < 0 ) + iter->next = Next(-1); + + ASSERT(iter->next >= Capacity() || ! table[iter->next].Empty()); + + // Filter out visited keys. + int capacity = Capacity(); + if ( iter->visited && ! iter->visited->empty() ) + // Filter out visited entries. + while ( iter->next < capacity ) + { + ASSERT(! table[iter->next].Empty()); + auto it = std::find(iter->visited->begin(), iter->visited->end(), table[iter->next]); + if ( it == iter->visited->end() ) + break; + iter->visited->erase(it); + iter->next = Next(iter->next); + } + + if ( iter->next >= capacity ) + { + iter->Complete(); + return detail::DictEntry(nullptr); // end of iteration + } + + ASSERT(! table[iter->next].Empty()); + detail::DictEntry e = table[iter->next]; + + //prepare for next time. + iter->next = Next(iter->next); + return e; + } + +RobustDictIterator::RobustDictIterator(Dictionary* d) : curr(nullptr), dict(d) + { + next = -1; + inserted = new std::vector(); + visited = new std::vector(); + + dict->num_iterators++; + dict->iterators->push_back(this); + + // Advance the iterator one step so that we're at the first element. + curr = dict->GetNextRobustIteration(this); + } + +RobustDictIterator::RobustDictIterator(const RobustDictIterator& other) : curr(nullptr) + { + dict = nullptr; + + if ( other.dict ) + { + next = other.next; + inserted = new std::vector(); + visited = new std::vector(); + + if ( other.inserted ) + std::copy(other.inserted->begin(), other.inserted->end(), std::back_inserter(*inserted)); + + if ( other.visited) + std::copy(other.visited->begin(), other.visited->end(), std::back_inserter(*visited)); + + dict = other.dict; + dict->num_iterators++; + dict->iterators->push_back(this); + + curr = other.curr; + } + } + +RobustDictIterator::RobustDictIterator(RobustDictIterator&& other) : curr(nullptr) + { + dict = nullptr; + + if ( other.dict ) + { + next = other.next; + inserted = other.inserted; + visited = other.visited; + + dict = other.dict; + dict->iterators->push_back(this); + dict->iterators->erase(std::remove(dict->iterators->begin(), dict->iterators->end(), &other), + dict->iterators->end()); + other.dict = nullptr; + + curr = std::move(other.curr); + } + } + +RobustDictIterator::~RobustDictIterator() + { + Complete(); + } + +void RobustDictIterator::Complete() + { + if ( dict ) + { + assert(dict->num_iterators > 0); + dict->num_iterators--; + + dict->iterators->erase(std::remove(dict->iterators->begin(), dict->iterators->end(), this), + dict->iterators->end()); + + delete inserted; + delete visited; + + inserted = nullptr; + visited = nullptr; + dict = nullptr; + } + } + +RobustDictIterator& RobustDictIterator::operator++() + { + curr = dict->GetNextRobustIteration(this); + return *this; + } + } // namespace zeek diff --git a/src/Dict.h b/src/Dict.h index 77fc89d1c7..34bd7e0097 100644 --- a/src/Dict.h +++ b/src/Dict.h @@ -4,10 +4,12 @@ #include #include +#include #include "zeek/Hash.h" ZEEK_FORWARD_DECLARE_NAMESPACED(IterCookie, zeek); +ZEEK_FORWARD_DECLARE_NAMESPACED(Dictionary, zeek); ZEEK_FORWARD_DECLARE_NAMESPACED(DictEntry, zeek::detail); // Type for function to be called when deleting elements. @@ -69,7 +71,7 @@ public: uint32_t hash = 0; void* value = nullptr; - union{ + union { char key_here[8]; //hold key len<=8. when over 8, it's a pointer to real keys. char* key; }; @@ -78,6 +80,9 @@ public: int16_t d = TOO_FAR_TO_REACH, bool copy_key = false) : distance(d), key_size(key_size), hash((uint32_t)hash), value(value) { + if ( ! arg_key ) + return; + if ( key_size <= 8 ) { memcpy(key_here, arg_key, key_size); @@ -120,6 +125,13 @@ public: } const char* GetKey() const { return key_size <= 8 ? key_here : key; } + std::unique_ptr GetHashKey() const + { + return std::make_unique(GetKey(), key_size, hash); + } + + template + T GetValue() const { return static_cast(value); } bool Equal(const char* arg_key, int arg_key_size, hash_t arg_hash) const {//only 40-bit hash comparison. @@ -138,6 +150,76 @@ public: } // namespace detail +class DictIterator { +public: + using value_type = detail::DictEntry; + using reference = detail::DictEntry&; + using pointer = detail::DictEntry*; + using difference_type = std::ptrdiff_t; + using iterator_category = std::forward_iterator_tag; + + ~DictIterator(); + + reference operator*() { return *curr; } + pointer operator->() { return curr; } + + DictIterator& operator++(); + DictIterator operator++(int) { auto temp(*this); ++*this; return temp; } + + bool operator==( const DictIterator& that ) const { return curr == that.curr; } + bool operator!=( const DictIterator& that ) const { return !(*this == that); } + +private: + friend class Dictionary; + + DictIterator() = default; + DictIterator(const Dictionary* d, detail::DictEntry* begin, detail::DictEntry* end); + + Dictionary* dict = nullptr; + detail::DictEntry* curr = nullptr; + detail::DictEntry* end = nullptr; +}; + +class RobustDictIterator { +public: + using value_type = detail::DictEntry; + using reference = detail::DictEntry&; + using pointer = detail::DictEntry*; + using difference_type = std::ptrdiff_t; + using iterator_category = std::forward_iterator_tag; + + RobustDictIterator() : curr(nullptr) {} + RobustDictIterator(Dictionary* d); + RobustDictIterator(const RobustDictIterator& other); + RobustDictIterator(RobustDictIterator&& other); + ~RobustDictIterator(); + + reference operator*() { return curr; } + pointer operator->() { return &curr; } + + RobustDictIterator& operator++(); +// RobustDictIterator operator++(int) { auto temp(*this); ++*this; return temp; } + + bool operator==( const RobustDictIterator& that ) const { return curr == that.curr; } + bool operator!=( const RobustDictIterator& that ) const { return !(*this == that); } + +private: + friend class Dictionary; + + void Complete(); + + // Tracks the new entries inserted while iterating. + std::vector* inserted = nullptr; + + // Tracks the entries already visited but were moved across the next iteration + // point due to an insertion. + std::vector* visited = nullptr; + + detail::DictEntry curr; + Dictionary* dict = nullptr; + int next = -1; +}; + /** * A dictionary type that uses clustered hashing, a variation of Robinhood/Open Addressing * hashing. The following posts help to understand the implementation: @@ -227,8 +309,11 @@ public: // // If return_hash is true, a HashKey for the entry is returned in h, // which should be delete'd when no longer needed. + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] IterCookie* InitForIteration() const; + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] void* NextEntry(detail::HashKey*& h, IterCookie*& cookie, bool return_hash) const; + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] void StopIteration(IterCookie* cookie) const; void SetDeleteFunc(dict_delete_func f) { delete_func = f; } @@ -239,6 +324,7 @@ public: // and (ii) we won't visit any still-unseen entries which are getting // removed. (We don't get this for free, so only use it if // necessary.) + [[deprecated("Remove in v5.1. Use begin_robust() and the standard-library-compatible version of iteration.")]] void MakeRobustCookie(IterCookie* cookie); // Remove all entries. @@ -257,8 +343,35 @@ public: void DistanceStats(int& max_distance, int* distances = 0, int num_distances = 0) const; void DumpKeys() const; + // Type traits needed for some of the std algorithms to work + using value_type = detail::DictEntry; + using pointer = detail::DictEntry*; + using const_pointer = const detail::DictEntry*; + + // Iterator support + using iterator = DictIterator; + using const_iterator = const iterator; + using reverse_iterator = std::reverse_iterator; + using const_reverse_iterator = std::reverse_iterator; + + iterator begin() { return { this, table, table + Capacity() }; } + iterator end() { return { this, table + Capacity(), table + Capacity() }; } + const_iterator begin() const { return { this, table, table + Capacity() }; } + const_iterator end() const { return { this, table + Capacity(), table + Capacity() }; } + const_iterator cbegin() { return { this, table, table + Capacity() }; } + const_iterator cend() { return { this, table + Capacity(), table + Capacity() }; } + + // begin_robust returns a shared pointer here because we tend to store them for long + // periods of time (see TableVal::expire_iterator). We don't want to keep the whole + // object in a TableVal if it's never used. end_robust() does not return a pointer + // since it's only used in the loop. + RobustDictIterator begin_robust() { return MakeRobustIterator(); } + RobustDictIterator end_robust() { return RobustDictIterator(); } + private: friend zeek::IterCookie; + friend zeek::DictIterator; + friend zeek::RobustDictIterator; /// Buckets of the table, not including overflow size. int Buckets(bool expected = false) const; @@ -301,9 +414,12 @@ private: void Init(); - //Iteration + // Iteration + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] IterCookie* InitForIterationNonConst(); + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] void* NextEntryNonConst(detail::HashKey*& h, IterCookie*& cookie, bool return_hash); + [[deprecated("Remove in v5.1. Use begin() and the standard-library-compatible version of iteration.")]] void StopIterationNonConst(IterCookie* cookie); //Lookup @@ -320,7 +436,10 @@ private: void InsertAndRelocate(detail::DictEntry& entry, int insert_position, int* last_affected_position = nullptr); /// Adjust Cookies on Insert. + [[deprecated("Remove in v5.1. Use the standard-library-compatible version of iteration and the version that takes a RobustDictIterator.")]] void AdjustOnInsert(IterCookie* c, const detail::DictEntry& entry, int insert_position, int last_affected_position); + void AdjustOnInsert(RobustDictIterator* c, const detail::DictEntry& entry, + int insert_position, int last_affected_position); ///Remove, Relocate & Adjust cookies. detail::DictEntry RemoveRelocateAndAdjust(int position); @@ -329,7 +448,10 @@ private: detail::DictEntry RemoveAndRelocate(int position, int* last_affected_position = nullptr); ///Adjust safe cookies after Removal of entry at position. + [[deprecated("Remove in v5.1. Use the standard-library-compatible version of iteration and the version that takes a RobustDictIterator.")]] void AdjustOnRemove(IterCookie* c, const detail::DictEntry& entry, int position, int last_affected_position); + void AdjustOnRemove(RobustDictIterator* c, const detail::DictEntry& entry, + int position, int last_affected_position); bool Remapping() const { return remap_end >= 0;} //remap in reverse order. @@ -344,7 +466,12 @@ private: void SizeUp(); bool HaveOnlyRobustIterators() const - { return num_iterators == 0 || (cookies && cookies->size() == num_iterators); } + { + return (num_iterators == 0) || ((cookies ? cookies->size() : 0) + (iterators ? iterators->size() : 0) == num_iterators); + } + + RobustDictIterator MakeRobustIterator(); + detail::DictEntry GetNextRobustIteration(RobustDictIterator* iter); //alligned on 8-bytes with 4-leading bytes. 7*8=56 bytes a dictionary. @@ -364,11 +491,12 @@ private: int num_entries = 0; int max_entries = 0; - uint64_t cum_entries = 0; + dict_delete_func delete_func = nullptr; detail::DictEntry* table = nullptr; std::vector* cookies = nullptr; + std::vector* iterators = nullptr; // Order means the order of insertion. means no deletion until exit. will be inefficient. std::vector* order = nullptr; @@ -403,13 +531,23 @@ public: int key_len; return (T*) Dictionary::NthEntry(n, (const void*&) key, key_len); } + [[deprecated("Remove in v5.1. Use the standard-library-compatible version of iteration.")]] T* NextEntry(IterCookie*& cookie) const { detail::HashKey* h; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" return (T*) Dictionary::NextEntry(h, cookie, false); +#pragma GCC diagnostic pop } + [[deprecated("Remove in v5.1. Use the standard-library-compatible version of iteration.")]] T* NextEntry(detail::HashKey*& h, IterCookie*& cookie) const - { return (T*) Dictionary::NextEntry(h, cookie, true); } + { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + return (T*) Dictionary::NextEntry(h, cookie, true); +#pragma GCC diagnostic pop + } T* RemoveEntry(const detail::HashKey* key, bool* iterators_invalidated = nullptr) { return (T*) Remove(key->Key(), key->Size(), key->Hash(), false, iterators_invalidated); } T* RemoveEntry(const detail::HashKey& key, bool* iterators_invalidated = nullptr) diff --git a/src/Reporter.cc b/src/Reporter.cc index 5b0245baf3..947568e99f 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -77,16 +77,12 @@ void Reporter::InitOptions() auto wl_val = id::find_val(name)->AsTableVal(); auto wl_table = wl_val->AsTable(); - detail::HashKey* k; - IterCookie* c = wl_table->InitForIteration(); - TableEntryVal* v; - - while ( (v = wl_table->NextEntry(k, c)) ) + for ( const auto& wle : *wl_table ) { + auto k = wle.GetHashKey(); auto index = wl_val->RecreateIndex(*k); std::string key = index->Idx(0)->AsString()->CheckString(); set->emplace(move(key)); - delete k; } }; diff --git a/src/Stmt.cc b/src/Stmt.cc index eaa65bd4da..16ea4acada 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1266,13 +1266,11 @@ ValPtr ForStmt::DoExec(Frame* f, Val* v, StmtFlowType& flow) const if ( ! loop_vals->Length() ) return nullptr; - HashKey* k; - TableEntryVal* current_tev; - IterCookie* c = loop_vals->InitForIteration(); - while ( (current_tev = loop_vals->NextEntry(k, c)) ) + for ( const auto& lve : *loop_vals ) { + auto k = lve.GetHashKey(); + auto* current_tev = lve.GetValue(); auto ind_lv = tv->RecreateIndex(*k); - delete k; if ( value_var ) f->SetElement(value_var, current_tev->GetVal()); @@ -1281,24 +1279,10 @@ ValPtr ForStmt::DoExec(Frame* f, Val* v, StmtFlowType& flow) const f->SetElement((*loop_vars)[i], ind_lv->Idx(i)); flow = FLOW_NEXT; - - try - { - ret = body->Exec(f, flow); - } - catch ( InterpreterException& ) - { - loop_vals->StopIteration(c); - throw; - } + ret = body->Exec(f, flow); if ( flow == FLOW_BREAK || flow == FLOW_RETURN ) - { - // If we broke or returned from inside a for loop, - // the cookie may still exist. - loop_vals->StopIteration(c); break; - } } } diff --git a/src/Val.cc b/src/Val.cc index 1c01058a8e..2f5b8e3f94 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -495,13 +495,15 @@ static void BuildJSON(threading::formatter::JSON::NullDoubleWriter& writer, Val* else writer.StartObject(); - detail::HashKey* k; + std::unique_ptr k; TableEntryVal* entry; - auto c = table->InitForIteration(); - while ( (entry = table->NextEntry(k, c)) ) + + for ( const auto& te : *table ) { + entry = te.GetValue(); + k = te.GetHashKey(); + auto lv = tval->RecreateIndex(*k); - delete k; Val* entry_key = lv->Length() == 1 ? lv->Idx(0).get() : lv.get(); if ( tval->GetType()->IsSet() ) @@ -1438,7 +1440,7 @@ void TableVal::Init(TableTypePtr t) table_type = std::move(t); expire_func = nullptr; expire_time = nullptr; - expire_cookie = nullptr; + expire_iterator = nullptr; timer = nullptr; def_val = nullptr; @@ -1460,6 +1462,7 @@ TableVal::~TableVal() delete table_hash; delete table_val; delete subnets; + delete expire_iterator; } void TableVal::RemoveAll() @@ -1483,11 +1486,9 @@ int TableVal::RecursiveSize() const GetType()->AsTableType()->Yield()->Tag() != TYPE_TABLE ) return n; - IterCookie* c = table_val->InitForIteration(); - - TableEntryVal* tv; - while ( (tv = table_val->NextEntry(c)) ) + for ( const auto& ve : *table_val ) { + auto* tv = ve.GetValue(); if ( tv->GetVal() ) n += tv->GetVal()->AsTableVal()->RecursiveSize(); } @@ -1657,15 +1658,12 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const return false; } - IterCookie* c = table_val->InitForIteration(); - - detail::HashKey* k; - TableEntryVal* v; - while ( (v = table_val->NextEntry(k, c)) ) + for ( const auto& tble : *table_val ) { - std::unique_ptr hk{k}; + auto k = tble.GetHashKey(); + auto* v = tble.GetValue(); - if ( is_first_init && t->AsTable()->Lookup(k) ) + if ( is_first_init && t->AsTable()->Lookup(k.get()) ) { auto key = table_hash->RecoverVals(*k); // ### Shouldn't complain if their values are equal. @@ -1675,12 +1673,12 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const if ( type->IsSet() ) { - if ( ! t->Assign(v->GetVal(), std::move(hk), nullptr) ) + if ( ! t->Assign(v->GetVal(), std::move(k), nullptr) ) return false; } else { - if ( ! t->Assign(nullptr, std::move(hk), v->GetVal()) ) + if ( ! t->Assign(nullptr, std::move(k), v->GetVal()) ) return false; } } @@ -1704,18 +1702,15 @@ bool TableVal::RemoveFrom(Val* val) const return false; } - IterCookie* c = table_val->InitForIteration(); - - detail::HashKey* k; - while ( table_val->NextEntry(k, c) ) + for ( const auto& tble : *table_val ) { // Not sure that this is 100% sound, since the HashKey // comes from one table but is being used in another. // OTOH, they are both the same type, so as long as // we don't have hash keys that are keyed per dictionary, // it should work ... + auto k = tble.GetHashKey(); t->Remove(*k); - delete k; } return true; @@ -1736,16 +1731,15 @@ TableValPtr TableVal::Intersection(const TableVal& tv) const t0 = tmp; } - IterCookie* c = t1->InitForIteration(); - detail::HashKey* k; - while ( t1->NextEntry(k, c) ) + const PDict* tbl = AsTable(); + for ( const auto& tble : *tbl ) { + auto k = tble.GetHashKey(); + // Here we leverage the same assumption about consistent // hashes as in TableVal::RemoveFrom above. - if ( t0->Lookup(k) ) - result->table_val->Insert(k, new TableEntryVal(nullptr)); - - delete k; + if ( t0->Lookup(k.get()) ) + result->table_val->Insert(k.get(), new TableEntryVal(nullptr)); } return result; @@ -1759,20 +1753,14 @@ bool TableVal::EqualTo(const TableVal& tv) const if ( t0->Length() != t1->Length() ) return false; - IterCookie* c = t0->InitForIteration(); - detail::HashKey* k; - while ( t0->NextEntry(k, c) ) + for ( const auto& tble : *t0 ) { + auto k = tble.GetHashKey(); + // Here we leverage the same assumption about consistent // hashes as in TableVal::RemoveFrom above. - if ( ! t1->Lookup(k) ) - { - delete k; - t0->StopIteration(c); + if ( ! t1->Lookup(k.get()) ) return false; - } - - delete k; } return true; @@ -1786,20 +1774,14 @@ bool TableVal::IsSubsetOf(const TableVal& tv) const if ( t0->Length() > t1->Length() ) return false; - IterCookie* c = t0->InitForIteration(); - detail::HashKey* k; - while ( t0->NextEntry(k, c) ) + for ( const auto& tble : *t0 ) { + auto k = tble.GetHashKey(); + // Here we leverage the same assumption about consistent // hashes as in TableVal::RemoveFrom above. - if ( ! t1->Lookup(k) ) - { - delete k; - t0->StopIteration(c); + if ( ! t1->Lookup(k.get()) ) return false; - } - - delete k; } return true; @@ -2331,11 +2313,9 @@ ListValPtr TableVal::ToListVal(TypeTag t) const { auto l = make_intrusive(t); - IterCookie* c = table_val->InitForIteration(); - - detail::HashKey* k; - while ( table_val->NextEntry(k, c) ) + for ( const auto& tble : *table_val ) { + auto k = tble.GetHashKey(); auto index = table_hash->RecoverVals(*k); if ( t == TYPE_ANY ) @@ -2348,8 +2328,6 @@ ListValPtr TableVal::ToListVal(TypeTag t) const l->Append(index->Idx(0)); } - - delete k; } return l; @@ -2400,16 +2378,16 @@ void TableVal::Describe(ODesc* d) const d->PushIndent(); } - IterCookie* c = table_val->InitForIteration(); + auto iter = table_val->begin(); for ( int i = 0; i < n; ++i ) { - detail::HashKey* k; - TableEntryVal* v = table_val->NextEntry(k, c); - - if ( ! v ) + if ( iter == table_val->end() ) reporter->InternalError("hash table underflow in TableVal::Describe"); + auto k = iter->GetHashKey(); + auto* v = iter->GetValue(); + auto vl = table_hash->RecoverVals(*k); int dim = vl->Length(); @@ -2434,8 +2412,6 @@ void TableVal::Describe(ODesc* d) const vl->Describe(d); - delete k; - if ( table_type->IsSet() ) { // We're a set, not a table. if ( d->IsReadable() ) @@ -2455,9 +2431,11 @@ void TableVal::Describe(ODesc* d) const d->Add(" @"); d->Add(util::detail::fmt_access_time(v->ExpireAccessTime())); } + + ++iter; } - if ( table_val->NextEntry(c) ) + if ( iter != table_val->end() ) reporter->InternalError("hash table overflow in TableVal::Describe"); if ( d->IsPortable() || d->IsReadable() ) @@ -2552,20 +2530,23 @@ void TableVal::DoExpire(double t) // error, it has been reported already. return; - if ( ! expire_cookie ) + if ( ! expire_iterator ) { - expire_cookie = table_val->InitForIteration(); - table_val->MakeRobustCookie(expire_cookie); + auto it = table_val->begin_robust(); + expire_iterator = new RobustDictIterator(std::move(it)); } - detail::HashKey* k = nullptr; + std::unique_ptr k = nullptr; TableEntryVal* v = nullptr; TableEntryVal* v_saved = nullptr; bool modified = false; for ( int i = 0; i < zeek::detail::table_incremental_step && - (v = table_val->NextEntry(k, expire_cookie)); ++i ) + *expire_iterator != table_val->end_robust(); ++i, ++(*expire_iterator) ) { + k = (*expire_iterator)->GetHashKey(); + v = (*expire_iterator)->GetValue(); + if ( v->ExpireAccessTime() == 0 ) { // This happens when we insert val while network_time @@ -2588,12 +2569,11 @@ void TableVal::DoExpire(double t) // function modified or deleted the table // value, so look it up again. v_saved = v; - v = table_val->Lookup(k); + v = table_val->Lookup(k.get()); if ( ! v ) { // user-provided function deleted it v = v_saved; - delete k; continue; } @@ -2602,7 +2582,6 @@ void TableVal::DoExpire(double t) // User doesn't want us to expire // this now. v->SetExpireAccess(run_state::network_time - timeout + secs); - delete k; continue; } @@ -2616,7 +2595,7 @@ void TableVal::DoExpire(double t) reporter->InternalWarning("index not in prefix table"); } - table_val->RemoveEntry(k); + table_val->RemoveEntry(k.get()); if ( change_func ) { if ( ! idx ) @@ -2628,16 +2607,15 @@ void TableVal::DoExpire(double t) delete v; modified = true; } - - delete k; } if ( modified ) Modified(); - if ( ! v ) + if ( (*expire_iterator) == table_val->end_robust() ) { - expire_cookie = nullptr; + delete expire_iterator; + expire_iterator = nullptr; InitTimer(zeek::detail::table_expire_interval); } else @@ -2740,22 +2718,19 @@ ValPtr TableVal::DoClone(CloneState* state) auto tv = make_intrusive(table_type); state->NewClone(this, tv); - IterCookie* cookie = table_val->InitForIteration(); - - detail::HashKey* key; - TableEntryVal* val; - while ( (val = table_val->NextEntry(key, cookie)) ) + const PDict* tbl = AsTable(); + for ( const auto& tble : *table_val ) { + auto key = tble.GetHashKey(); + auto* val = tble.GetValue(); TableEntryVal* nval = val->Clone(state); - tv->table_val->Insert(key, nval); + tv->table_val->Insert(key.get(), nval); if ( subnets ) { auto idx = RecreateIndex(*key); tv->subnets->Insert(idx.get(), nval); } - - delete key; } tv->attrs = attrs; @@ -2783,11 +2758,9 @@ unsigned int TableVal::MemoryAllocation() const { unsigned int size = 0; - IterCookie* c = table_val->InitForIteration(); - - TableEntryVal* tv; - while ( (tv = table_val->NextEntry(c)) ) + for ( const auto& ve : *table_val ) { + auto* tv = ve.GetValue(); if ( tv->GetVal() ) size += tv->GetVal()->MemoryAllocation(); size += padded_sizeof(TableEntryVal); @@ -2833,17 +2806,13 @@ void TableVal::DoneParsing() TableVal::ParseTimeTableState TableVal::DumpTableState() { - IterCookie* cookie = table_val->InitForIteration(); - - detail::HashKey* key; - TableEntryVal* val; - ParseTimeTableState rval; - - while ( (val = table_val->NextEntry(key, cookie)) ) + for ( const auto& tble : *table_val ) { + auto key = tble.GetHashKey(); + auto* val = tble.GetValue(); + rval.emplace_back(RecreateIndex(*key), val->GetVal()); - delete key; } RemoveAll(); diff --git a/src/Val.h b/src/Val.h index ecdb5bb8ee..d72550b3ff 100644 --- a/src/Val.h +++ b/src/Val.h @@ -14,6 +14,7 @@ #include "zeek/Notifier.h" #include "zeek/Reporter.h" #include "zeek/net_util.h" +#include "zeek/Dict.h" // We have four different port name spaces: TCP, UDP, ICMP, and UNKNOWN. // We distinguish between them based on the bits specified in the *_PORT_MASK @@ -26,13 +27,10 @@ #define ICMP_PORT_MASK 0x30000 namespace zeek { -template class PDict; class String; } -template using PDict [[deprecated("Remove in v4.1. Use zeek::PDict instead.")]] = zeek::PDict; using BroString [[deprecated("Remove in v4.1. Use zeek::String instead.")]] = zeek::String; -ZEEK_FORWARD_DECLARE_NAMESPACED(IterCookie, zeek); ZEEK_FORWARD_DECLARE_NAMESPACED(Frame, zeek::detail); ZEEK_FORWARD_DECLARE_NAMESPACED(Func, zeek); ZEEK_FORWARD_DECLARE_NAMESPACED(IPAddr, zeek); @@ -1107,7 +1105,7 @@ protected: detail::ExprPtr expire_time; detail::ExprPtr expire_func; TableValTimer* timer; - IterCookie* expire_cookie; + RobustDictIterator* expire_iterator; detail::PrefixTable* subnets; ValPtr def_val; detail::ExprPtr change_func; diff --git a/src/broker/Data.cc b/src/broker/Data.cc index ddb643766f..4d8287f41f 100644 --- a/src/broker/Data.cc +++ b/src/broker/Data.cc @@ -927,14 +927,12 @@ broker::expected val_to_data(const Val* v) else rval = broker::table(); - zeek::detail::HashKey* hk; - TableEntryVal* entry; - auto c = table->InitForIteration(); - - while ( (entry = table->NextEntry(hk, c)) ) + for ( const auto& te : *table ) { + auto hk = te.GetHashKey(); + auto* entry = te.GetValue(); + auto vl = table_val->RecreateIndex(*hk); - delete hk; broker::vector composite_key; composite_key.reserve(vl->Length()); diff --git a/src/broker/messaging.bif b/src/broker/messaging.bif index 6c9e27920b..53f3d6fbaf 100644 --- a/src/broker/messaging.bif +++ b/src/broker/messaging.bif @@ -34,14 +34,13 @@ std::set val_to_topic_set(zeek::Val* val) if ( tbl->Length() == 0 ) return rval; - zeek::IterCookie* c = tbl->InitForIteration(); - zeek::detail::HashKey* k; - - while ( tbl->NextEntry(k, c) ) + for ( const auto& te : *tbl ) { + auto k = te.GetHashKey(); + auto* v = te.GetValue(); + auto index = val->AsTableVal()->RecreateIndex(*k); rval.emplace(index->Idx(0)->AsString()->CheckString()); - delete k; } } diff --git a/src/file_analysis/AnalyzerSet.h b/src/file_analysis/AnalyzerSet.h index a72757a1a2..c9990037b6 100644 --- a/src/file_analysis/AnalyzerSet.h +++ b/src/file_analysis/AnalyzerSet.h @@ -92,8 +92,14 @@ public: * @see Dictionary#InitForIteration * @return an iterator that may be used to loop over analyzers in the set. */ + [[deprecated("Remove in v5.1. Use standard-library compatible iteration.")]] IterCookie* InitForIteration() const - { return analyzer_map.InitForIteration(); } + { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + return analyzer_map.InitForIteration(); +#pragma GCC diagnostic pop + } /** * Get next entry in the analyzer set. @@ -102,8 +108,27 @@ public: * @return the next analyzer in the set or a null pointer if there is no * more left (in that case the cookie is also deleted). */ + [[deprecated("Remove in v5.1. Use standard-library compatible iteration.")]] file_analysis::Analyzer* NextEntry(IterCookie* c) - { return analyzer_map.NextEntry(c); } + { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + return analyzer_map.NextEntry(c); +#pragma GCC diagnostic pop + } + + // Iterator support + using iterator = zeek::DictIterator; + using const_iterator = const iterator; + using reverse_iterator = std::reverse_iterator; + using const_reverse_iterator = std::reverse_iterator; + + iterator begin() { return analyzer_map.begin(); } + iterator end() { return analyzer_map.end(); } + const_iterator begin() const { return analyzer_map.begin(); } + const_iterator end() const { return analyzer_map.end(); } + const_iterator cbegin() { return analyzer_map.cbegin(); } + const_iterator cend() { return analyzer_map.cend(); } protected: diff --git a/src/file_analysis/File.cc b/src/file_analysis/File.cc index 55265fa96e..1c7511ebbc 100644 --- a/src/file_analysis/File.cc +++ b/src/file_analysis/File.cc @@ -393,11 +393,10 @@ void File::DeliverStream(const u_char* data, uint64_t len) util::fmt_bytes((const char*) data, std::min((uint64_t)40, len)), len > 40 ? "..." : ""); - file_analysis::Analyzer* a = nullptr; - IterCookie* c = analyzers.InitForIteration(); - - while ( (a = analyzers.NextEntry(c)) ) + for ( const auto& entry : analyzers ) { + auto* a = entry.GetValue(); + DBG_LOG(DBG_FILE_ANALYSIS, "stream delivery to analyzer %s", file_mgr->GetComponentName(a->Tag()).c_str()); if ( ! a->GotStreamDelivery() ) { @@ -497,11 +496,10 @@ void File::DeliverChunk(const u_char* data, uint64_t len, uint64_t offset) util::fmt_bytes((const char*) data, std::min((uint64_t)40, len)), len > 40 ? "..." : ""); - file_analysis::Analyzer* a = nullptr; - IterCookie* c = analyzers.InitForIteration(); - - while ( (a = analyzers.NextEntry(c)) ) + for ( const auto& entry : analyzers ) { + auto* a = entry.GetValue(); + DBG_LOG(DBG_FILE_ANALYSIS, "chunk delivery to analyzer %s", file_mgr->GetComponentName(a->Tag()).c_str()); if ( ! a->Skipping() ) { @@ -561,11 +559,10 @@ void File::EndOfFile() done = true; - file_analysis::Analyzer* a = nullptr; - IterCookie* c = analyzers.InitForIteration(); - - while ( (a = analyzers.NextEntry(c)) ) + for ( const auto& entry : analyzers ) { + auto* a = entry.GetValue(); + if ( ! a->EndOfFile() ) analyzers.QueueRemove(a->Tag(), a->GetArgs()); } @@ -594,11 +591,10 @@ void File::Gap(uint64_t offset, uint64_t len) DeliverStream((const u_char*) "", 0); } - file_analysis::Analyzer* a = nullptr; - IterCookie* c = analyzers.InitForIteration(); - - while ( (a = analyzers.NextEntry(c)) ) + for ( const auto& entry : analyzers ) { + auto* a = entry.GetValue(); + if ( ! a->Undelivered(offset, len) ) analyzers.QueueRemove(a->Tag(), a->GetArgs()); } diff --git a/src/input/Manager.cc b/src/input/Manager.cc index 27b47491eb..359b3ef222 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -274,17 +274,16 @@ bool Manager::CreateStream(Stream* info, RecordVal* description) { // create config mapping in ReaderInfo. Has to be done before the construction of reader_obj. - zeek::detail::HashKey* k; - IterCookie* c = info->config->AsTable()->InitForIteration(); - - TableEntryVal* v; - while ( (v = info->config->AsTable()->NextEntry(k, c)) ) + auto* info_config_table = info->config->AsTable(); + for ( const auto& icte : *info_config_table ) { + auto k = icte.GetHashKey(); + auto* v = icte.GetValue(); + auto index = info->config->RecreateIndex(*k); string key = index->Idx(0)->AsString()->CheckString(); string value = v->GetVal()->AsString()->CheckString(); rinfo.config.insert(std::make_pair(util::copy_string(key.c_str()), util::copy_string(value.c_str()))); - delete k; } } @@ -1340,16 +1339,14 @@ void Manager::EndCurrentSend(ReaderFrontend* reader) } assert(i->stream_type == TABLE_STREAM); - TableStream* stream = (TableStream*) i; + auto* stream = static_cast(i); // lastdict contains all deleted entries and should be empty apart from that - IterCookie *c = stream->lastDict->InitForIteration(); - stream->lastDict->MakeRobustCookie(c); - InputHash* ih; - zeek::detail::HashKey *lastDictIdxKey; - - while ( ( ih = stream->lastDict->NextEntry(lastDictIdxKey, c) ) ) + for ( auto it = stream->lastDict->begin_robust(); it != stream->lastDict->end_robust(); ++it ) { + auto lastDictIdxKey = it->GetHashKey(); + InputHash* ih = it->GetValue(); + ValPtr val; ValPtr predidx; EnumValPtr ev; @@ -1376,8 +1373,7 @@ void Manager::EndCurrentSend(ReaderFrontend* reader) { // Keep it. Hence - we quit and simply go to the next entry of lastDict // ah well - and we have to add the entry to currDict... - stream->currDict->Insert(lastDictIdxKey, stream->lastDict->RemoveEntry(lastDictIdxKey)); - delete lastDictIdxKey; + stream->currDict->Insert(lastDictIdxKey.get(), stream->lastDict->RemoveEntry(lastDictIdxKey.get())); continue; } } @@ -1393,13 +1389,12 @@ void Manager::EndCurrentSend(ReaderFrontend* reader) } stream->tab->Remove(*ih->idxkey); - stream->lastDict->Remove(lastDictIdxKey); // delete in next line - delete lastDictIdxKey; - delete(ih); + stream->lastDict->Remove(lastDictIdxKey.get()); // delete in next line + delete ih; } - stream->lastDict->Clear(); // should be empt. buti- well... who knows... - delete(stream->lastDict); + stream->lastDict->Clear(); // should be empty. but well... who knows... + delete stream->lastDict; stream->lastDict = stream->currDict; stream->currDict = new PDict; diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 58045a1e4d..a0594e8265 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -889,17 +889,16 @@ bool Manager::Write(EnumVal* id, RecordVal* columns_arg) info->path = util::copy_string(path.c_str()); info->network_time = run_state::network_time; - zeek::detail::HashKey* k; - IterCookie* c = filter->config->AsTable()->InitForIteration(); - - TableEntryVal* v; - while ( (v = filter->config->AsTable()->NextEntry(k, c)) ) + auto* filter_config_table = filter->config->AsTable(); + for ( const auto& fcte : *filter_config_table ) { + auto k = fcte.GetHashKey(); + auto* v = fcte.GetValue(); + auto index = filter->config->RecreateIndex(*k); string key = index->Idx(0)->AsString()->CheckString(); string value = v->GetVal()->AsString()->CheckString(); info->config.insert(std::make_pair(util::copy_string(key.c_str()), util::copy_string(value.c_str()))); - delete k; } // CreateWriter() will set the other fields in info. diff --git a/src/reporter.bif b/src/reporter.bif index cda29f7255..950b58f846 100644 --- a/src/reporter.bif +++ b/src/reporter.bif @@ -183,16 +183,14 @@ function Reporter::set_weird_sampling_whitelist%(weird_sampling_whitelist: strin auto wl_table = wl_val->AsTable(); std::unordered_set whitelist_set; - zeek::detail::HashKey* k; - IterCookie* c = wl_table->InitForIteration(); - TableEntryVal* v; - - while ( (v = wl_table->NextEntry(k, c)) ) + for ( const auto& tble : *wl_table ) { + auto k = tble.GetHashKey(); + auto* v = tble.GetValue(); + auto index = wl_val->RecreateIndex(*k); string key = index->Idx(0)->AsString()->CheckString(); whitelist_set.emplace(move(key)); - delete k; } reporter->SetWeirdSamplingWhitelist(std::move(whitelist_set)); return zeek::val_mgr->True(); @@ -223,16 +221,12 @@ function Reporter::set_weird_sampling_global_list%(weird_sampling_global_list: s auto wl_table = wl_val->AsTable(); std::unordered_set global_list_set; - zeek::detail::HashKey* k; - IterCookie* c = wl_table->InitForIteration(); - TableEntryVal* v; - - while ( (v = wl_table->NextEntry(k, c)) ) + for ( const auto& tble : *wl_table ) { + auto k = tble.GetHashKey(); auto index = wl_val->RecreateIndex(*k); string key = index->Idx(0)->AsString()->CheckString(); global_list_set.emplace(move(key)); - delete k; } reporter->SetWeirdSamplingGlobalList(std::move(global_list_set)); return zeek::val_mgr->True(); diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index 2fd08a5061..315be81e80 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -1261,14 +1261,13 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node) auto cluster_table_val = node->GetField("cluster")->AsTableVal(); auto cluster_table = cluster_table_val->AsTable(); - auto c = cluster_table->InitForIteration(); - detail::HashKey* k; - TableEntryVal* v; - while ( (v = cluster_table->NextEntry(k, c)) ) + for ( const auto& cte : *cluster_table ) { + auto k = cte.GetHashKey(); + auto* v = cte.GetValue(); + auto key = cluster_table_val->RecreateIndex(*k); - delete k; auto name = key->Idx(0)->AsStringVal()->ToStdString(); auto rv = v->GetVal()->AsRecordVal();