You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2019/02/22 14:17:08 UTC
[trafficserver] branch master updated: IpMap: Add move constructor.
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 <am...@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);
+};