trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject [trafficserver] branch master updated: IpMap: Add move constructor.
Date Fri, 22 Feb 2019 14:17:08 GMT
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 <amc@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);
+};


Mime
View raw message