This is an automated email from the ASF dual-hosted git repository. wwbmmm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push: new f97cbe5c Fix FlatMap assign operator bug (#2622) f97cbe5c is described below commit f97cbe5c65de314dd4b7db18b0900c68cf499f8a Author: Bright Chen <chenguangmin...@foxmail.com> AuthorDate: Mon Apr 29 10:17:44 2024 +0800 Fix FlatMap assign operator bug (#2622) --- src/butil/containers/flat_map.h | 16 ++++++++-------- src/butil/containers/flat_map_inl.h | 28 +++++++++++++++++++--------- test/flat_map_unittest.cpp | 15 +++++++++++++++ 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/butil/containers/flat_map.h b/src/butil/containers/flat_map.h index 9bfd8ec1..50f44540 100644 --- a/src/butil/containers/flat_map.h +++ b/src/butil/containers/flat_map.h @@ -151,12 +151,12 @@ public: key_type key; }; - FlatMap(const hasher& hashfn = hasher(), - const key_equal& eql = key_equal(), - const allocator_type& alloc = allocator_type()); + explicit FlatMap(const hasher& hashfn = hasher(), + const key_equal& eql = key_equal(), + const allocator_type& alloc = allocator_type()); ~FlatMap(); - FlatMap(const FlatMap& rhs); - void operator=(const FlatMap& rhs); + FlatMap(const FlatMap& rhs); + FlatMap& operator=(const FlatMap& rhs); void swap(FlatMap & rhs); // Must be called to initialize this map, otherwise insert/operator[] @@ -307,9 +307,9 @@ public: typedef typename Map::key_equal key_equal; typedef typename Map::allocator_type allocator_type; - FlatSet(const hasher& hashfn = hasher(), - const key_equal& eql = key_equal(), - const allocator_type& alloc = allocator_type()) + explicit FlatSet(const hasher& hashfn = hasher(), + const key_equal& eql = key_equal(), + const allocator_type& alloc = allocator_type()) : _map(hashfn, eql, alloc) {} void swap(FlatSet & rhs) { _map.swap(rhs._map); } diff --git a/src/butil/containers/flat_map_inl.h b/src/butil/containers/flat_map_inl.h index 264d28c7..65830368 100644 --- a/src/butil/containers/flat_map_inl.h +++ b/src/butil/containers/flat_map_inl.h @@ -261,20 +261,19 @@ FlatMap<_K, _T, _H, _E, _S, _A>::FlatMap(const FlatMap& rhs) } template <typename _K, typename _T, typename _H, typename _E, bool _S, typename _A> -void +FlatMap<_K, _T, _H, _E, _S, _A>& FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const FlatMap<_K, _T, _H, _E, _S, _A>& rhs) { if (this == &rhs) { - return; + return *this; } // NOTE: assignment does not change _load_factor/_hashfn/_eql if |this| is // initialized clear(); - if (rhs.empty()) { - return; - } - if (!initialized()) { - _load_factor = rhs._load_factor; + if (!rhs.initialized()) { + return *this; } + bool need_copy = !rhs.empty(); + _load_factor = rhs._load_factor; if (_buckets == NULL || is_too_crowded(rhs._size)) { get_allocator().Free(_buckets); _nbucket = rhs._nbucket; @@ -282,18 +281,28 @@ FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const FlatMap<_K, _T, _H, _E, _S, _A> _buckets = (Bucket*)get_allocator().Alloc(sizeof(Bucket) * (_nbucket + 1/*note*/)); if (NULL == _buckets) { LOG(ERROR) << "Fail to new _buckets"; - return; + return *this; + } + // If no need to copy, set buckets invalid. + if (!need_copy) { + for (size_t i = 0; i < _nbucket; ++i) { + _buckets[i].set_invalid(); + } + _buckets[_nbucket].next = NULL; } if (_S) { free(_thumbnail); _thumbnail = bit_array_malloc(_nbucket); if (NULL == _thumbnail) { LOG(ERROR) << "Fail to new _thumbnail"; - return; + return *this; } bit_array_clear(_thumbnail, _nbucket); } } + if (!need_copy) { + return *this; + } if (_nbucket == rhs._nbucket) { // For equivalent _nbucket, walking through _buckets instead of using // iterators is more efficient. @@ -322,6 +331,7 @@ FlatMap<_K, _T, _H, _E, _S, _A>::operator=(const FlatMap<_K, _T, _H, _E, _S, _A> Element::second_ref_from_value(*it); } } + return *this; } template <typename _K, typename _T, typename _H, typename _E, bool _S, typename _A> diff --git a/test/flat_map_unittest.cpp b/test/flat_map_unittest.cpp index 6ba7b3ec..d689bdf1 100644 --- a/test/flat_map_unittest.cpp +++ b/test/flat_map_unittest.cpp @@ -1332,6 +1332,21 @@ TEST_F(FlatMapTest, copy) { m2 = m1; ASSERT_FALSE(m1.is_too_crowded(m1.size())); ASSERT_FALSE(m2.is_too_crowded(m1.size())); + + butil::FlatMap<int, int> m3; + ASSERT_FALSE(m3.initialized()); + m1 = m3; + ASSERT_TRUE(m1.empty()); + ASSERT_TRUE(m1.initialized()); + + m3 = m2; + ASSERT_TRUE(m3.initialized()); + m3.clear(); + ASSERT_TRUE(m3.initialized()); + ASSERT_TRUE(m3.empty()); + butil::FlatMap<int, int> m4 = m3; + ASSERT_TRUE(m4.initialized()); + ASSERT_TRUE(m4.empty()); } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org