This is an automated email from the ASF dual-hosted git repository. bneradt pushed a commit to branch dev-1-0-13 in repository https://gitbox.apache.org/repos/asf/trafficserver-libswoc.git
commit 14a7a589729f3c7337d3b57efb3f0c916f469504 Author: Alan M. Carroll <[email protected]> AuthorDate: Tue Feb 25 15:47:35 2020 -0600 Fix bug in IPSpace::mark with hull computation. Fix bug in IP support for converting between different types. --- swoc++/include/swoc/DiscreteRange.h | 10 ++++++---- swoc++/include/swoc/swoc_ip.h | 38 +++++++++++++++++++++++++------------ swoc++/src/RBTree.cc | 15 ++++++--------- swoc++/src/swoc_ip.cc | 4 ++-- unit_tests/test_ip.cc | 35 ++++++++++++++-------------------- 5 files changed, 54 insertions(+), 48 deletions(-) diff --git a/swoc++/include/swoc/DiscreteRange.h b/swoc++/include/swoc/DiscreteRange.h index d74dd7e..f961449 100644 --- a/swoc++/include/swoc/DiscreteRange.h +++ b/swoc++/include/swoc/DiscreteRange.h @@ -832,12 +832,14 @@ template<typename METRIC, typename PAYLOAD> void DiscreteSpace<METRIC, PAYLOAD>::Node::structure_fixup() { if (_left) { if (_right) { - _hull = this->left()->_range.hull(this->right()->_range); + _hull = this->left()->_hull.hull(this->right()->_hull); } else { - _hull = this->left()->_range; + _hull = this->left()->_hull; } } else if (_right) { - _hull = this->right()->_range; + _hull = this->right()->_hull; + } else { + _hull = _range; // always contain at least self in the hull. } } @@ -1014,7 +1016,7 @@ DiscreteSpace<METRIC, PAYLOAD>::mark(DiscreteSpace::range_type const &range, PAY // y->min >= min (or y would have been selected). Therefore in this // case the request covers the next node therefore it can be reused. x = y; - x->assign(range).assign(payload); + x->assign(range).assign(payload).ripple_structure_fixup(); n = x; // this gets bumped again, which is correct. } } else { diff --git a/swoc++/include/swoc/swoc_ip.h b/swoc++/include/swoc/swoc_ip.h index b327c49..61165a9 100644 --- a/swoc++/include/swoc/swoc_ip.h +++ b/swoc++/include/swoc/swoc_ip.h @@ -956,11 +956,13 @@ public: value_type const* operator->() const; protected: - using super_type::super_type; + using super_type::super_type; /// Inherit supertype constructors. }; - const_iterator begin() const { return const_iterator(_ip4.begin(), _ip6.begin()); } - const_iterator end() const { return const_iterator(_ip4.end(), _ip6.end()); } + /// @return A constant iterator to the first element. + const_iterator begin() const; + /// @return A constent iterator past the last element. + const_iterator end() const; iterator begin() { return iterator{_ip4.begin(), _ip6.begin()}; } iterator end() { return iterator{_ip4.end(), _ip6.end()}; } @@ -1187,23 +1189,23 @@ IPAddr::assign(in6_addr const &addr) { inline IPAddr & IPAddr::assign(sockaddr_in const *addr) { - if (addr) - { + if (addr) { _family = AF_INET; - _addr._ip4 = addr->sin_addr.s_addr; - } else - { _family = AF_UNSPEC; } + _addr._ip4 = addr; + } else { + _family = AF_UNSPEC; + } return *this; } inline IPAddr & IPAddr::assign(sockaddr_in6 const *addr) { - if (addr) - { + if (addr) { _family = AF_INET6; _addr._ip6 = addr->sin6_addr; - } else - { _family = AF_UNSPEC; } + } else { + _family = AF_UNSPEC; + } return *this; } @@ -1677,5 +1679,17 @@ void IPSpace<PAYLOAD>::clear() { _ip6.clear(); } +template<typename PAYLOAD> +auto IPSpace<PAYLOAD>::begin() const -> const_iterator { + auto nc_this = const_cast<self_type*>(this); + return const_iterator(nc_this->_ip4.begin(), nc_this->_ip6.begin()); +} + +template<typename PAYLOAD> +auto IPSpace<PAYLOAD>::end() const -> const_iterator { + auto nc_this = const_cast<self_type*>(this); + return const_iterator(nc_this->_ip4.end(), nc_this->_ip6.end()); +} + } // namespace swoc diff --git a/swoc++/src/RBTree.cc b/swoc++/src/RBTree.cc index 180ca51..fab84c1 100644 --- a/swoc++/src/RBTree.cc +++ b/swoc++/src/RBTree.cc @@ -191,15 +191,12 @@ namespace detail } /* The node to be removed from the tree. - If @c this (the target node) has both children, we remove - its successor, which cannot have a left child and - put that node in place of the target node. Otherwise this - node has at most one child, so we can remove it. - Note that the successor of a node with a right child is always - a right descendant of the node. Therefore, remove_node - is an element of the tree rooted at this node. - Because of the initial special case checks, we know - that remove_node is @b not the root node. + If @c this (the target node) has both children, we remove its successor, which cannot have a + left child and put that node in place of the target node. Otherwise this node has at most + one child, so we can remove it. Note that the successor of a node with a right child is + always a right descendant of the node. Therefore, remove_node is an element of the tree + rooted at this node. Because of the initial special case checks, we know that remove_node is + @b not the root node. */ self_type *remove_node(_left && _right ? _right->left_most_descendant() : this); diff --git a/swoc++/src/swoc_ip.cc b/swoc++/src/swoc_ip.cc index 391988e..7fd4264 100644 --- a/swoc++/src/swoc_ip.cc +++ b/swoc++/src/swoc_ip.cc @@ -102,8 +102,8 @@ IPAddr& IPAddr::assign(sockaddr const *addr) { if (addr) { switch (addr->sa_family) { - case AF_INET:return this->assign(reinterpret_cast<sockaddr_in const *>(addr)->sin_addr.s_addr); - case AF_INET6:return this->assign(reinterpret_cast<sockaddr_in6 const *>(addr)->sin6_addr); + case AF_INET:return this->assign(reinterpret_cast<sockaddr_in const *>(addr)); + case AF_INET6:return this->assign(reinterpret_cast<sockaddr_in6 const *>(addr)); default:break; } } diff --git a/unit_tests/test_ip.cc b/unit_tests/test_ip.cc index 4cec9f1..9d97cc2 100644 --- a/unit_tests/test_ip.cc +++ b/unit_tests/test_ip.cc @@ -28,8 +28,9 @@ using swoc::IP6Range; using swoc::IPAddr; using swoc::IPRange; +using W = swoc::LocalBufferWriter<256>; -TEST_CASE("ink_inet", "[libswoc][ip]") { +TEST_CASE("Basic IP", "[libswoc][ip]") { // Use TextView because string_view(nullptr) fails. Gah. struct ip_parse_spec { TextView hostspec; @@ -62,6 +63,11 @@ TEST_CASE("ink_inet", "[libswoc][ip]") { REQUIRE(s.rest == rest); } + IP4Addr alpha { "172.96.12.134"}; + CHECK(alpha == IP4Addr{"172.96.12.134"}); + CHECK(alpha == IP4Addr{IPAddr{"172.96.12.134"}}); + CHECK(alpha == IPAddr{IPEndpoint{"172.96.12.134:80"}}); + // Do a bit of IPv6 testing. IP6Addr a6_null; IP6Addr a6_1{"fe80:88b5:4a:20c:29ff:feae:5587:1c33"}; @@ -303,6 +309,8 @@ TEST_CASE("IP Space Int", "[libswoc][ip][ipspace]") { REQUIRE(nullptr == space.find(r_2.min())); REQUIRE(nullptr != space.find(r_1.min())); REQUIRE(nullptr != space.find(r_1.max())); + REQUIRE(nullptr != space.find(IP4Addr{"1.1.1.7"})); + CHECK(0x1 == *space.find(IP4Addr{"1.1.1.7"})); space.blend(r_2, 0x2, BF); REQUIRE(space.count() == 2); @@ -346,33 +354,20 @@ TEST_CASE("IP Space Int", "[libswoc][ip][ipspace]") { space.clear(); for (auto &&[text, value] : ranges) { + IPRange range{text}; space.mark(IPRange{text}, value); } + CHECK(7 == space.count()); + CHECK(nullptr != space.find(IP4Addr{"100.0.4.16"})); + CHECK(nullptr != space.find(IPAddr{"100.0.4.16"})); + CHECK(nullptr != space.find(IPAddr{IPEndpoint{"100.0.4.16:80"}})); } TEST_CASE("IPSpace bitset", "[libswoc][ipspace][bitset]") { using PAYLOAD = std::bitset<32>; using Space = swoc::IPSpace<PAYLOAD>; - auto dump = [](Space&space) -> void { - swoc::LocalBufferWriter<1024> w; - std::cout << "Dumping " << space.count() << " ranges" << std::endl; - for (auto &&[r, payload] : space) { - w.clear().print("{}-{} :", r.min(), r.max()); - std::cout << w << payload << std::endl; - } - }; - auto reverse_dump = [](Space&space) -> void { - swoc::LocalBufferWriter<1024> w; - std::cout << "Dumping " << space.count() << " ranges" << std::endl; - for (auto spot = space.end(); spot != space.begin();) { - auto &&[r, payload]{*--spot}; - w.clear().print("{} :", r); - std::cout << w << payload << std::endl; - } - }; - std::array<std::tuple<TextView, std::initializer_list<unsigned>>, 6> ranges = { { {"172.28.56.12-172.28.56.99"_tv, {0, 2, 3}} @@ -393,8 +388,6 @@ TEST_CASE("IPSpace bitset", "[libswoc][ipspace][bitset]") { space.mark(IPRange{text}, bits); } REQUIRE(space.count() == ranges.size()); - dump(space); - reverse_dump(space); } TEST_CASE("IPSpace docJJ", "[libswoc][ipspace][docJJ]") {
