This is an automated email from the ASF dual-hosted git repository. amc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new b52c293 IpMap: Add move constructor. b52c293 is described below commit b52c293ef8f2acb9471dbebd46fce7e439695df3 Author: Alan M. Carroll <a...@apache.org> AuthorDate: Mon Feb 18 23:23:56 2019 -0600 IpMap: Add move constructor. --- include/tscore/IpMap.h | 108 +++++++++++++++++++----------------- src/tscore/IpMap.cc | 90 +++++++++++++++++++----------- src/tscore/unit_tests/test_IpMap.cc | 26 +++++++-- 3 files changed, 139 insertions(+), 85 deletions(-) diff --git a/include/tscore/IpMap.h b/include/tscore/IpMap.h index 34ec5dc..17ecc95 100644 --- a/include/tscore/IpMap.h +++ b/include/tscore/IpMap.h @@ -23,6 +23,9 @@ #pragma once +#include <algorithm> +#include <utility> + #include "tscore/ink_platform.h" #include "tscore/ink_defs.h" #include "tscore/RbTree.h" @@ -42,8 +45,8 @@ namespace detail typename A = T const & ///< Argument type. > struct Interval { - typedef T Metric; ///< Metric (storage) type. - typedef A ArgType; ///< Type used to pass instances of @c Metric. + using Metric = T; ///< Metric (storage) type. + using ArgType = A; ///< Type used to pass instances of @c Metric. Interval() {} ///< Default constructor. /// Construct with values. @@ -95,7 +98,7 @@ namespace detail class IpMap { public: - typedef IpMap self; ///< Self reference type. + using self_type = IpMap; ///< Self reference type. class iterator; // forward declare. @@ -107,7 +110,7 @@ public: friend class IpMap; public: - typedef Node self; ///< Self reference type. + using self_type = Node; ///< Self reference type. /// Default constructor. Node() : _data(nullptr) {} /// Construct with @a data. @@ -119,7 +122,7 @@ public: return _data; } /// Set client data. - virtual self & + virtual self_type & setData(void *data ///< Client data pointer to store. ) { @@ -146,30 +149,30 @@ public: friend class IpMap; public: - typedef iterator self; ///< Self reference type. - typedef Node value_type; ///< Referenced type for iterator. - typedef int difference_type; ///< Distance type. - typedef Node *pointer; ///< Pointer to referent. - typedef Node &reference; ///< Reference to referent. - typedef std::bidirectional_iterator_tag iterator_category; + using self_type = iterator; ///< Self reference type. + using value_type = Node; ///< Referenced type for iterator. + using difference_type = int; ///< Distance type. + using pointer = Node *; ///< Pointer to referent. + using reference = Node &; ///< Reference to referent. + using iterator_category = std::bidirectional_iterator_tag; /// Default constructor. - iterator() : _tree(nullptr), _node(nullptr) {} + iterator() = default; reference operator*() const; //!< value operator pointer operator->() const; //!< dereference operator - self &operator++(); //!< next node (prefix) - self operator++(int); //!< next node (postfix) - self &operator--(); ///< previous node (prefix) - self operator--(int); ///< next node (postfix) + self_type &operator++(); //!< next node (prefix) + self_type operator++(int); //!< next node (postfix) + self_type &operator--(); ///< previous node (prefix) + self_type operator--(int); ///< next node (postfix) /** Equality. @return @c true if the iterators refer to the same node. */ - bool operator==(self const &that) const; + bool operator==(self_type const &that) const; /** Inequality. @return @c true if the iterators refer to different nodes. */ bool - operator!=(self const &that) const + operator!=(self_type const &that) const { return !(*this == that); } @@ -177,20 +180,25 @@ public: private: /// Construct a valid iterator. iterator(IpMap const *tree, Node *node) : _tree(tree), _node(node) {} - IpMap const *_tree; ///< Container. - Node *_node; //!< Current node. + IpMap const *_tree = nullptr; ///< Container. + Node *_node = nullptr; //!< Current node. }; - IpMap(); ///< Default constructor. + IpMap(); ///< Default constructor. + IpMap(self_type const &that) = delete; + IpMap(self_type &&that) noexcept; ~IpMap(); ///< Destructor. + self_type &operator=(self_type const &that) = delete; + self_type &operator =(self_type &&that); + /** Mark a range. All addresses in the range [ @a min , @a max ] are marked with @a data. @return This object. */ - self &mark(sockaddr const *min, ///< Minimum value in range. - sockaddr const *max, ///< Maximum value in range. - void *data = nullptr ///< Client data payload. + self_type &mark(sockaddr const *min, ///< Minimum value in range. + sockaddr const *max, ///< Maximum value in range. + void *data = nullptr ///< Client data payload. ); /** Mark a range. @@ -198,9 +206,9 @@ public: @note Convenience overload for IPv4 addresses. @return This object. */ - self &mark(in_addr_t min, ///< Minimum address (network order). - in_addr_t max, ///< Maximum address (network order). - void *data = nullptr ///< Client data. + self_type &mark(in_addr_t min, ///< Minimum address (network order). + in_addr_t max, ///< Maximum address (network order). + void *data = nullptr ///< Client data. ); /** Mark a range. @@ -208,9 +216,9 @@ public: @note Convenience overload for IPv4 addresses. @return This object. */ - self &mark(IpAddr const &min, ///< Minimum address (network order). - IpAddr const &max, ///< Maximum address (network order). - void *data = nullptr ///< Client data. + self_type &mark(IpAddr const &min, ///< Minimum address (network order). + IpAddr const &max, ///< Maximum address (network order). + void *data = nullptr ///< Client data. ); /** Mark an IPv4 address @a addr with @a data. @@ -218,8 +226,8 @@ public: @note Convenience overload for IPv4 addresses. @return This object. */ - self &mark(in_addr_t addr, ///< Address (network order). - void *data = nullptr ///< Client data. + self_type &mark(in_addr_t addr, ///< Address (network order). + void *data = nullptr ///< Client data. ); /** Mark a range. @@ -227,9 +235,9 @@ public: @note Convenience overload. @return This object. */ - self &mark(IpEndpoint const *min, ///< Minimum address (network order). - IpEndpoint const *max, ///< Maximum address (network order). - void *data = nullptr ///< Client data. + self_type &mark(IpEndpoint const *min, ///< Minimum address (network order). + IpEndpoint const *max, ///< Maximum address (network order). + void *data = nullptr ///< Client data. ); /** Mark an address @a addr with @a data. @@ -237,8 +245,8 @@ public: @note Convenience overload. @return This object. */ - self &mark(IpEndpoint const *addr, ///< Address (network order). - void *data = nullptr ///< Client data. + self_type &mark(IpEndpoint const *addr, ///< Address (network order). + void *data = nullptr ///< Client data. ); /** Unmark addresses. @@ -248,14 +256,14 @@ public: @return This object. */ - self &unmark(sockaddr const *min, ///< Minimum value. - sockaddr const *max ///< Maximum value. + self_type &unmark(sockaddr const *min, ///< Minimum value. + sockaddr const *max ///< Maximum value. ); /// Unmark addresses (overload). - self &unmark(IpEndpoint const *min, IpEndpoint const *max); + self_type &unmark(IpEndpoint const *min, IpEndpoint const *max); /// Unmark overload. - self &unmark(in_addr_t min, ///< Minimum of range to unmark. - in_addr_t max ///< Maximum of range to unmark. + self_type &unmark(in_addr_t min, ///< Minimum of range to unmark. + in_addr_t max ///< Maximum of range to unmark. ); /** Fill addresses. @@ -269,13 +277,13 @@ public: @return This object. */ - self &fill(sockaddr const *min, sockaddr const *max, void *data = nullptr); + self_type &fill(sockaddr const *min, sockaddr const *max, void *data = nullptr); /// Fill addresses (overload). - self &fill(IpEndpoint const *min, IpEndpoint const *max, void *data = nullptr); + self_type &fill(IpEndpoint const *min, IpEndpoint const *max, void *data = nullptr); /// Fill addresses (overload). - self &fill(IpAddr const &min, IpAddr const &max, void *data = nullptr); + self_type &fill(IpAddr const &min, IpAddr const &max, void *data = nullptr); /// Fill addresses (overload). - self &fill(in_addr_t min, in_addr_t max, void *data = nullptr); + self_type &fill(in_addr_t min, in_addr_t max, void *data = nullptr); /** Test for membership. @@ -328,7 +336,7 @@ public: @note This is much faster than @c unmark. @return This object. */ - self &clear(); + self_type &clear(); /// Iterator for first element. iterator begin() const; @@ -354,8 +362,8 @@ protected: /// @return The IPv6 map. ts::detail::Ip6Map *force6(); - ts::detail::Ip4Map *_m4; ///< Map of IPv4 addresses. - ts::detail::Ip6Map *_m6; ///< Map of IPv6 addresses. + ts::detail::Ip4Map *_m4 = nullptr; ///< Map of IPv4 addresses. + ts::detail::Ip6Map *_m6 = nullptr; ///< Map of IPv6 addresses. }; inline IpMap & @@ -437,7 +445,7 @@ IpMap::iterator::operator++(int) inline IpMap::iterator IpMap::iterator::operator--(int) { - self tmp(*this); + self_type tmp(*this); --*this; return tmp; } diff --git a/src/tscore/IpMap.cc b/src/tscore/IpMap.cc index ed20657..d2583f2 100644 --- a/src/tscore/IpMap.cc +++ b/src/tscore/IpMap.cc @@ -1,6 +1,3 @@ -#include "tscore/IpMap.h" -#include "tscore/ink_inet.h" - /** @file IP address map support. @@ -46,6 +43,9 @@ before we had IpAddr as a type. */ +#include "tscore/IpMap.h" +#include "tscore/ink_inet.h" + namespace ts { namespace detail @@ -139,8 +139,9 @@ namespace detail using ArgType = typename N::ArgType; ///< Import type. using Metric = typename N::Metric; ///< Import type.g482 - IpMapBase() : _root(nullptr) {} - ~IpMapBase() { this->clear(); } + IpMapBase() = default; + IpMapBase(self_type &&that) : _root(that._root), _list(std::move(that._list)) { that._root = nullptr; } + ~IpMapBase(); /** Mark a range. All addresses in the range [ @a min , @a max ] are marked with @a data. @return This object. @@ -266,7 +267,7 @@ namespace detail return static_cast<N *>(_list.tail()); } - N *_root; ///< Root node. + N *_root = nullptr; ///< Root node. /// In order list of nodes. /// For ugly compiler reasons, this is a list of base class pointers /// even though we really store @a N instances on it. @@ -283,7 +284,6 @@ namespace detail } }; using NodeList = ts::IntrusiveDList<NodeLinkage>; - // typedef ts::IntrusiveDList<RBNode, &RBNode::_next, &RBNode::_prev> NodeList; /// This keeps track of all allocated nodes in order. /// Iteration depends on this list being maintained. NodeList _list; @@ -460,17 +460,14 @@ namespace detail Metric max_plus = N::deref(max); N::inc(max_plus); - /* Some subtlety - for IPv6 we overload the compare operators to do - the right thing, but we can't overload pointer - comparisons. Therefore we carefully never compare pointers in - this logic. Only @a min and @a max can be pointers, everything - else is an instance or a reference. Since there's no good reason - to compare @a min and @a max this isn't particularly tricky, but - it's good to keep in mind. If we were somewhat more clever, we - would provide static less than and equal operators in the - template class @a N and convert all the comparisons to use only - those two via static function call. - */ + /* Some subtlety - for IPv6 we overload the compare operators to do the right thing, but we + * can't overload pointer comparisons. Therefore we carefully never compare pointers in this + * logic. Only @a min and @a max can be pointers, everything else is an instance or a reference. + * Since there's no good reason to compare @a min and @a max this isn't particularly tricky, but + * it's good to keep in mind. If we were somewhat more clever, we would provide static less than + * and equal operators in the template class @a N and convert all the comparisons to use only + * those two via static function call. + */ /* We have lots of special cases here primarily to minimize memory allocation by re-using an existing node as often as possible. @@ -748,8 +745,10 @@ namespace detail return *this; } + template <typename N> IpMapBase<N>::~IpMapBase() { this->clear(); } + //---------------------------------------------------------------------------- - typedef Interval<in_addr_t, in_addr_t> Ip4Span; + using Ip4Span = Interval<in_addr_t, in_addr_t>; /** Node for IPv4 map. We store the address in host order in the @a _min and @a _max @@ -911,7 +910,13 @@ namespace detail /// is to use a pointer, not a reference. using ArgType = const ts::detail::Interval<sockaddr_in6, const sockaddr_in6 &>::Metric *; - /// Construct from pointers. + /** Construct from the argument type. + * + * @param min Minimum value in the range. + * @param max Maximum value in the range (inclusvie). + * @param data Data to attach to the range. + */ + Ip6Node(ArgType min, ///< Minimum address (network order). ArgType max, ///< Maximum address (network order). void *data ///< Client data. @@ -919,30 +924,36 @@ namespace detail : Node(data), Ip6Span(*min, *max) { } - /// Construct with values. - Ip6Node(Metric const &min, ///< Minimum address (network order). - Metric const &max, ///< Maximum address (network order). - void *data ///< Client data. - ) - : Node(data), Ip6Span(min, max) - { - } + + /** Construct from the underlying @c Metric type @a min to @a max + * + * @param min Minimum value in the range. + * @param max Maximum value in the range (inclusvie). + * @param data Data to attach to the range. + */ + Ip6Node(Metric const &min, Metric const &max, void *data) : Node(data), Ip6Span(min, max) {} + /// @return The minimum value of the interval. sockaddr const * min() const override { return ats_ip_sa_cast(&_min); } + /// @return The maximum value of the interval. sockaddr const * max() const override { return ats_ip_sa_cast(&_max); } - /// Set the client data. + + /** Set the client @a data. + * + * @param data Client data. + * @return @a this + */ self_type & - setData(void *data ///< Client data. - ) override + setData(void *data) override { _data = data; return *this; @@ -1081,6 +1092,23 @@ namespace detail } // namespace detail } // namespace ts //---------------------------------------------------------------------------- +IpMap::IpMap(IpMap::self_type &&that) noexcept : _m4(that._m4), _m6(that._m6) +{ + that._m4 = nullptr; + that._m6 = nullptr; +} + +IpMap::self_type & +IpMap::operator=(IpMap::self_type &&that) +{ + if (&that != this) { + this->clear(); + std::swap(_m4, that._m4); + std::swap(_m6, that._m6); + } + return *this; +} + IpMap::~IpMap() { delete _m4; diff --git a/src/tscore/unit_tests/test_IpMap.cc b/src/tscore/unit_tests/test_IpMap.cc index f8a2886..20a8231 100644 --- a/src/tscore/unit_tests/test_IpMap.cc +++ b/src/tscore/unit_tests/test_IpMap.cc @@ -540,11 +540,11 @@ TEST_CASE("IpMap CloseIntersection", "[libts][ipmap]") void *const markB = reinterpret_cast<void *>(2); void *const markC = reinterpret_cast<void *>(3); void *const markD = reinterpret_cast<void *>(4); - // void *mark; // for retrieval IpEndpoint a_1_l, a_1_u, a_2_l, a_2_u, a_3_l, a_3_u, a_4_l, a_4_u, a_5_l, a_5_u, a_6_l, a_6_u, a_7_l, a_7_u; IpEndpoint b_1_l, b_1_u; IpEndpoint c_1_l, c_1_u, c_2_l, c_2_u, c_3_l, c_3_u; + IpEndpoint c_3_m; IpEndpoint d_1_l, d_1_u, d_2_l, d_2_u; IpEndpoint a_1_m; @@ -573,6 +573,7 @@ TEST_CASE("IpMap CloseIntersection", "[libts][ipmap]") ats_ip_pton("123.90.112.0", &c_2_l); ats_ip_pton("123.90.119.255", &c_2_u); ats_ip_pton("123.90.132.0", &c_3_l); + ats_ip_pton("123.90.134.157", &c_3_m); ats_ip_pton("123.90.135.255", &c_3_u); ats_ip_pton("123.82.196.0", &d_1_l); @@ -590,16 +591,33 @@ TEST_CASE("IpMap CloseIntersection", "[libts][ipmap]") CHECK_THAT(map, IsMarkedAt(a_1_m)); map.mark(b_1_l, b_1_u, markB); - CHECK_THAT(map, IsMarkedAt(a_1_m)); + CHECK_THAT(map, IsMarkedWith(a_1_m, markA)); map.mark(c_1_l, c_1_u, markC); map.mark(c_2_l, c_2_u, markC); map.mark(c_3_l, c_3_u, markC); - CHECK_THAT(map, IsMarkedAt(a_1_m)); + CHECK_THAT(map, IsMarkedWith(a_1_m, markA)); map.mark(d_1_l, d_1_u, markD); map.mark(d_2_l, d_2_u, markD); CHECK_THAT(map, IsMarkedAt(a_1_m)); + CHECK_THAT(map, IsMarkedWith(b_1_u, markB)); + CHECK_THAT(map, IsMarkedWith(c_3_m, markC)); + CHECK_THAT(map, IsMarkedWith(d_2_l, markD)); CHECK(map.count() == 13); -} + + // Check move constructor. + IpMap m2{std::move(map)}; + // Original map should be empty. + REQUIRE(map.count() == 0); + // Do all these again on the destination map. + CHECK_THAT(m2, IsMarkedWith(a_1_m, markA)); + CHECK_THAT(m2, IsMarkedWith(a_1_m, markA)); + CHECK_THAT(m2, IsMarkedWith(a_1_m, markA)); + CHECK_THAT(m2, IsMarkedWith(a_1_m, markA)); + CHECK_THAT(m2, IsMarkedWith(b_1_u, markB)); + CHECK_THAT(m2, IsMarkedWith(c_3_m, markC)); + CHECK_THAT(m2, IsMarkedWith(d_2_l, markD)); + CHECK(m2.count() == 13); +};