wrap destination string in i2p_peer in string_ptr handler (#2268)

bindings/python/src/alert.cpp

@@ -11,6 +11,8 @@
 #include <memory>
 #include "bytes.hpp"
 
+#include <boost/type_traits/is_polymorphic.hpp>
+
 using namespace boost::python;
 using namespace lt;
 

include/libtorrent/Makefile.am

@@ -187,6 +187,7 @@ nobase_include_HEADERS = \
   aux_/session_interface.hpp        \
   aux_/suggest_piece.hpp            \
   aux_/storage_piece_set.hpp        \
+  aux_/string_ptr.hpp               \
   aux_/time.hpp                     \
   aux_/file_progress.hpp            \
   aux_/openssl.hpp                  \

include/libtorrent/aux_/string_ptr.hpp

@@ -0,0 +1,76 @@
+/*
+
+Copyright (c) 2017, Arvid Norberg
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#ifndef TORRENT_STRING_PTR_HPP_INCLUDED
+#define TORRENT_STRING_PTR_HPP_INCLUDED
+
+#include "libtorrent/string_view.hpp"
+
+namespace libtorrent {
+namespace aux {
+
+	struct string_ptr
+	{
+		explicit string_ptr(string_view str) : m_ptr(new char[str.size() + 1])
+		{
+			std::copy(str.begin(), str.end(), m_ptr);
+			m_ptr[str.size()] = '\0';
+		}
+		~string_ptr()
+		{
+			delete[] m_ptr;
+		}
+		string_ptr(string_ptr&& rhs)
+			: m_ptr(rhs.m_ptr)
+		{
+			rhs.m_ptr = nullptr;
+		}
+		string_ptr& operator=(string_ptr&& rhs)
+		{
+			if (&rhs == this) return *this;
+			delete[] m_ptr;
+			m_ptr = rhs.m_ptr;
+			rhs.m_ptr = nullptr;
+			return *this;
+		}
+		string_ptr(string_ptr const& rhs) = delete;
+		string_ptr& operator=(string_ptr const& rhs) = delete;
+		char const* operator*() const { return m_ptr; }
+	private:
+		char* m_ptr;
+	};
+
+}
+}
+
+#endif
+

include/libtorrent/i2p_stream.hpp

@@ -100,7 +100,7 @@ public:
 
 	void set_session_id(char const* id) { m_id = id; }
 
-	void set_destination(std::string const& d) { m_dest = d; }
+	void set_destination(string_view d) { m_dest = d.to_string(); }
 	std::string const& destination() { return m_dest; }
 
 	template <class Handler>

include/libtorrent/peer_list.hpp

@@ -48,6 +48,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/peer_connection_interface.hpp"
 #include "libtorrent/aux_/deque.hpp"
 #include "libtorrent/peer_info.hpp" // for peer_source_flags_t
+#include "libtorrent/string_view.hpp"
 
 namespace libtorrent {
 
@@ -113,7 +114,7 @@ namespace libtorrent {
 		peer_list();
 
 #if TORRENT_USE_I2P
-		torrent_peer* add_i2p_peer(char const* destination
+		torrent_peer* add_i2p_peer(string_view destination
 			, peer_source_flags_t src, char flags
 			, torrent_state* state);
 #endif
@@ -213,7 +214,7 @@ namespace libtorrent {
 		void update_connect_candidates(int delta);
 
 		void update_peer(torrent_peer* p, peer_source_flags_t src, int flags
-		, tcp::endpoint const& remote, char const* destination);
+		, tcp::endpoint const& remote);
 		bool insert_peer(torrent_peer* p, iterator iter, int flags, torrent_state* state);
 
 		bool compare_peer_erase(torrent_peer const& lhs, torrent_peer const& rhs) const;

include/libtorrent/torrent_peer.hpp

@@ -37,6 +37,8 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/address.hpp"
 #include "libtorrent/socket.hpp"
 #include "libtorrent/peer_info.hpp" // for peer_source_flags_t
+#include "libtorrent/aux_/string_ptr.hpp"
+#include "libtorrent/string_view.hpp"
 
 namespace libtorrent {
 
@@ -59,7 +61,7 @@ namespace libtorrent {
 		std::uint32_t rank(external_ip const& external, int external_port) const;
 
 		libtorrent::address address() const;
-		char const* dest() const;
+		string_view dest() const;
 
 		tcp::endpoint ip() const { return tcp::endpoint(address(), port); }
 
@@ -213,12 +215,13 @@ namespace libtorrent {
 #if TORRENT_USE_I2P
 	struct TORRENT_EXTRA_EXPORT i2p_peer : torrent_peer
 	{
-		i2p_peer(char const* destination, bool connectable, peer_source_flags_t src);
-		i2p_peer(i2p_peer const&);
-		~i2p_peer();
-		i2p_peer& operator=(i2p_peer const&);
+		i2p_peer(string_view dst, bool connectable, peer_source_flags_t src);
+		i2p_peer(i2p_peer const&) = delete;
+		i2p_peer& operator=(i2p_peer const&) = delete;
+		i2p_peer(i2p_peer&&) = default;
+		i2p_peer& operator=(i2p_peer&&) = default;
 
-		char* destination;
+		aux::string_ptr destination;
 	};
 #endif
 
@@ -234,38 +237,33 @@ namespace libtorrent {
 
 	struct peer_address_compare
 	{
-		bool operator()(
-			torrent_peer const* lhs, libtorrent::address const& rhs) const
+		bool operator()(torrent_peer const* lhs, address const& rhs) const
 		{
 			return lhs->address() < rhs;
 		}
 
-		bool operator()(
-				libtorrent::address const& lhs, torrent_peer const* rhs) const
+		bool operator()(address const& lhs, torrent_peer const* rhs) const
 		{
 			return lhs < rhs->address();
 		}
 
 #if TORRENT_USE_I2P
-		bool operator()(
-				torrent_peer const* lhs, char const* rhs) const
+		bool operator()(torrent_peer const* lhs, string_view rhs) const
 		{
-			return strcmp(lhs->dest(), rhs) < 0;
+			return lhs->dest().compare(rhs) < 0;
 		}
 
-		bool operator()(
-				char const* lhs, torrent_peer const* rhs) const
+		bool operator()(string_view lhs, torrent_peer const* rhs) const
 		{
-			return strcmp(lhs, rhs->dest()) < 0;
+			return lhs.compare(rhs->dest()) < 0;
 		}
 #endif
 
-		bool operator()(
-				torrent_peer const* lhs, torrent_peer const* rhs) const
+		bool operator()(torrent_peer const* lhs, torrent_peer const* rhs) const
 		{
 #if TORRENT_USE_I2P
 			if (rhs->is_i2p_addr == lhs->is_i2p_addr)
-				return strcmp(lhs->dest(), rhs->dest()) < 0;
+				return lhs->dest().compare(rhs->dest()) < 0;
 #endif
 			return lhs->address() < rhs->address();
 		}

src/peer_list.cpp

@@ -952,8 +952,7 @@ namespace libtorrent {
 	}
 
 	void peer_list::update_peer(torrent_peer* p, peer_source_flags_t const src
-		, int flags
-		, tcp::endpoint const& remote, char const* /* destination*/)
+		, int flags, tcp::endpoint const& remote)
 	{
 		TORRENT_ASSERT(is_single_thread());
 		bool const was_conn_cand = is_connect_candidate(*p);
@@ -1008,29 +1007,25 @@ namespace libtorrent {
 	}
 
 #if TORRENT_USE_I2P
-	// TODO: 3 use string_view for destination
-	torrent_peer* peer_list::add_i2p_peer(char const* destination
+	torrent_peer* peer_list::add_i2p_peer(string_view const destination
 		, peer_source_flags_t const src, char flags, torrent_state* state)
 	{
 		TORRENT_ASSERT(is_single_thread());
 		INVARIANT_CHECK;
 
-		bool found = false;
-		iterator iter = std::lower_bound(
-			m_peers.begin(), m_peers.end()
-			, destination, peer_address_compare()
-		);
+		iterator iter = std::lower_bound(m_peers.begin(), m_peers.end()
+			, destination, peer_address_compare());
 
-		if (iter != m_peers.end() && strcmp((*iter)->dest(), destination) == 0)
-			found = true;
-
-		torrent_peer* p = nullptr;
-
-		if (!found)
+		if (iter != m_peers.end() && (*iter)->dest() == destination)
 		{
+			update_peer(*iter, src, flags, tcp::endpoint());
+			return *iter;
+		}
+
 		// we don't have any info about this peer.
 		// add a new entry
-			p = state->peer_allocator->allocate_peer_entry(torrent_peer_allocator_interface::i2p_peer_type);
+		torrent_peer* p = state->peer_allocator->allocate_peer_entry(
+			torrent_peer_allocator_interface::i2p_peer_type);
 		if (p == nullptr) return nullptr;
 		new (p) i2p_peer(destination, true, src);
 
@@ -1047,12 +1042,6 @@ namespace libtorrent {
 			state->peer_allocator->free_peer_entry(p);
 			return nullptr;
 		}
-		}
-		else
-		{
-			p = *iter;
-			update_peer(p, src, flags, tcp::endpoint(), destination);
-		}
 		return p;
 	}
 #endif // TORRENT_USE_I2P
@@ -1089,10 +1078,8 @@ namespace libtorrent {
 		}
 		else
 		{
-			iter = std::lower_bound(
-				m_peers.begin(), m_peers.end()
-				, remote.address(), peer_address_compare()
-			);
+			iter = std::lower_bound(m_peers.begin(), m_peers.end()
+				, remote.address(), peer_address_compare());
 
 			if (iter != m_peers.end() && (*iter)->address() == remote.address()) found = true;
 		}
@@ -1137,7 +1124,7 @@ namespace libtorrent {
 		{
 			p = *iter;
 			TORRENT_ASSERT(p->in_use);
-			update_peer(p, src, flags, remote, nullptr);
+			update_peer(p, src, flags, remote);
 			state->first_time_seen = false;
 		}
 

src/torrent.cpp

@@ -6509,7 +6509,7 @@ namespace libtorrent {
 				, m_ses.i2p_proxy(), *s, nullptr, nullptr, false, false);
 			(void)ret;
 			TORRENT_ASSERT(ret);
-			s->get<i2p_stream>()->set_destination(static_cast<i2p_peer*>(peerinfo)->destination);
+			s->get<i2p_stream>()->set_destination(static_cast<i2p_peer*>(peerinfo)->dest());
 			s->get<i2p_stream>()->set_command(i2p_stream::cmd_connect);
 			s->get<i2p_stream>()->set_session_id(m_ses.i2p_session());
 		}

src/torrent_peer.cpp

@@ -196,7 +196,7 @@ namespace libtorrent {
 	std::string torrent_peer::to_string() const
 	{
 #if TORRENT_USE_I2P
-		if (is_i2p_addr) return dest();
+		if (is_i2p_addr) return dest().to_string();
 #endif // TORRENT_USE_I2P
 		error_code ec;
 		return address().to_string(ec);
@@ -246,31 +246,16 @@ namespace libtorrent {
 	ipv4_peer& ipv4_peer::operator=(ipv4_peer const& p) = default;
 
 #if TORRENT_USE_I2P
-	i2p_peer::i2p_peer(char const* dest, bool connectable
+	i2p_peer::i2p_peer(string_view dest, bool connectable
 		, peer_source_flags_t const src)
-		: torrent_peer(0, connectable, src), destination(allocate_string_copy(dest))
+		: torrent_peer(0, connectable, src)
+		, destination(dest)
 	{
 #if TORRENT_USE_IPV6
 		is_v6_addr = false;
 #endif
 		is_i2p_addr = true;
 	}
-
-	i2p_peer::~i2p_peer()
-	{ free(destination); }
-
-	i2p_peer::i2p_peer(const i2p_peer& rhs)
-		: torrent_peer(rhs.port, rhs.connectable, rhs.peer_source())
-		, destination(allocate_string_copy(rhs.destination))
-	{}
-
-	i2p_peer& i2p_peer::operator=(i2p_peer const& rhs)
-	{
-		char* tmp = allocate_string_copy(rhs.destination);
-		free(destination);
-		destination = tmp;
-		return *this;
-	}
 #endif // TORRENT_USE_I2P
 
 #if TORRENT_USE_IPV6
@@ -290,10 +275,10 @@ namespace libtorrent {
 #endif // TORRENT_USE_IPV6
 
 #if TORRENT_USE_I2P
-	char const* torrent_peer::dest() const
+	string_view torrent_peer::dest() const
 	{
 		if (is_i2p_addr)
-			return static_cast<i2p_peer const*>(this)->destination;
+			return *static_cast<i2p_peer const*>(this)->destination;
 		return "";
 	}
 #endif

test/test_string.cpp

@@ -34,6 +34,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/aux_/escape_string.hpp"
 #include "libtorrent/hex.hpp"
 #include "libtorrent/string_util.hpp"
+#include "libtorrent/aux_/string_ptr.hpp"
 #include <iostream>
 #include <cstring> // for strcmp
 #include "libtorrent/aux_/escape_string.hpp" // for trim
@@ -448,3 +449,42 @@ TORRENT_TEST(string_eq_no_case)
 	TEST_CHECK(cmp(std::string("\0a", 2), std::string("\0a", 2)));
 }
 
+TORRENT_TEST(string_ptr_zero_termination)
+{
+	char str[] = {'f', 'o', 'o', 'b', 'a', 'r'};
+	aux::string_ptr p(string_view(str, sizeof(str)));
+
+	// make sure it's zero-terminated now
+	TEST_CHECK(strlen(*p) == 6);
+	TEST_CHECK((*p)[6] == '\0');
+	TEST_CHECK(*p == string_view("foobar"));
+}
+
+TORRENT_TEST(string_ptr_move_construct)
+{
+	aux::string_ptr p1("test");
+	TEST_CHECK(*p1 == string_view("test"));
+
+	aux::string_ptr p2(std::move(p1));
+
+	TEST_CHECK(*p2 == string_view("test"));
+
+	// moved-from state is empty
+	TEST_CHECK(*p1 == nullptr);
+}
+
+TORRENT_TEST(string_ptr_move_assign)
+{
+	aux::string_ptr p1("test");
+	TEST_CHECK(*p1 == string_view("test"));
+
+	aux::string_ptr p2("foobar");
+
+	p1 = std::move(p2);
+
+	TEST_CHECK(*p1 == string_view("foobar"));
+
+	// moved-from state is empty
+	TEST_CHECK(*p2 == nullptr);
+}
+