make the variables holding the source of external IPs type-safe

arvidn · arvidn · commit 9111d5977ef3 · 2017-10-10T22:38:36.000+02:00
diff --git a/include/libtorrent/aux_/session_impl.hpp b/include/libtorrent/aux_/session_impl.hpp
@@ -658,10 +658,10 @@ namespace aux {
 				, dht::msg const& request, entry& response) override;
 
 			void set_external_address(address const& ip
-				, int source_type, address const& source) override;
+				, ip_source_t source_type, address const& source) override;
 			void set_external_address(tcp::endpoint const& local_endpoint
 				, address const& ip
-				, int source_type, address const& source) override;
+				, ip_source_t source_type, address const& source) override;
 			external_ip external_address() const override;
 
 			// used when posting synchronous function
@@ -754,7 +754,7 @@ namespace aux {
 			void setup_socket_buffers(socket_type& s) override;
 
 			void set_external_address(std::shared_ptr<listen_socket_t> const& sock, address const& ip
-				, int const source_type, address const& source);
+				, ip_source_t const source_type, address const& source);
 
 			void interface_to_endpoints(std::string const& device, int const port
 				, bool const ssl, std::vector<listen_endpoint_t>& eps);
diff --git a/include/libtorrent/aux_/session_interface.hpp b/include/libtorrent/aux_/session_interface.hpp
@@ -44,6 +44,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/aux_/vector.hpp"
 #include "libtorrent/aux_/listen_socket_handle.hpp"
 #include "libtorrent/session_types.hpp"
+#include "libtorrent/flags.hpp"
 
 #include <functional>
 #include <memory>
@@ -101,6 +102,9 @@ namespace libtorrent { namespace aux {
 	struct proxy_settings;
 	struct session_settings;
 
+	struct ip_source_tag;
+	using ip_source_t = flags::bitfield_flag<std::uint8_t, ip_source_tag>;
+
 #if !defined TORRENT_DISABLE_LOGGING || TORRENT_USE_ASSERTS
 	// This is the basic logging and debug interface offered by the session.
 	// a release build with logging disabled (which is the default) will
@@ -131,21 +135,23 @@ namespace libtorrent { namespace aux {
 		: session_logger
 #endif
 	{
+
 		// TODO: 2 the IP voting mechanism should be factored out
 		// to its own class, not part of the session
-		enum
-		{
-			source_dht = 1,
-			source_peer = 2,
-			source_tracker = 4,
-			source_router = 8
-		};
+		// and these constants should move too
+
+		// the logic in ip_voter relies on more reliable sources are represented
+		// by more significant bits
+		static constexpr ip_source_t source_dht = 1_bit;
+		static constexpr ip_source_t source_peer = 2_bit;
+		static constexpr ip_source_t source_tracker = 3_bit;
+		static constexpr ip_source_t source_router = 4_bit;
 
 		virtual void set_external_address(address const& ip
-			, int source_type, address const& source) = 0;
+			, ip_source_t source_type, address const& source) = 0;
 		virtual void set_external_address(tcp::endpoint const& local_endpoint
 			, address const& ip
-			, int source_type, address const& source) = 0;
+			, ip_source_t source_type, address const& source) = 0;
 		virtual external_ip external_address() const = 0;
 
 		virtual disk_interface& disk_thread() = 0;
diff --git a/include/libtorrent/ip_voter.hpp b/include/libtorrent/ip_voter.hpp
@@ -37,6 +37,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/address.hpp"
 #include "libtorrent/bloom_filter.hpp"
 #include "libtorrent/time.hpp" // for time_point
+#include "libtorrent/aux_/session_interface.hpp" // for ip_source_t
 
 namespace libtorrent {
 
@@ -48,7 +49,7 @@ namespace libtorrent {
 
 		// returns true if a different IP is the top vote now
 		// i.e. we changed our idea of what our external IP is
-		bool cast_vote(address const& ip, int source_type, address const& source);
+		bool cast_vote(address const& ip, aux::ip_source_t source_type, address const& source);
 
 		address external_address() const { return m_external_address; }
 
@@ -58,16 +59,14 @@ namespace libtorrent {
 
 		struct external_ip_t
 		{
-			external_ip_t(): sources(0), num_votes(0) {}
-
-			bool add_vote(sha1_hash const& k, int type);
+			bool add_vote(sha1_hash const& k, aux::ip_source_t type);
 
 			// we want to sort descending
 			bool operator<(external_ip_t const& rhs) const
 			{
 				if (num_votes > rhs.num_votes) return true;
 				if (num_votes < rhs.num_votes) return false;
-				return sources > rhs.sources;
+				return static_cast<std::uint8_t>(sources) > static_cast<std::uint8_t>(rhs.sources);
 			}
 
 			// this is a bloom filter of the IPs that have
@@ -76,9 +75,9 @@ namespace libtorrent {
 			// this is the actual external address
 			address addr;
 			// a bitmask of sources the reporters have come from
-			std::uint16_t sources;
+			aux::ip_source_t sources{};
 			// the total number of votes for this IP
-			std::uint16_t num_votes;
+			std::uint16_t num_votes = 0;
 		};
 
 		// this is a bloom filter of all the IPs that have
diff --git a/simulation/setup_dht.cpp b/simulation/setup_dht.cpp
@@ -80,7 +80,8 @@ namespace {
 	std::shared_ptr<lt::aux::listen_socket_t> sim_listen_socket(tcp::endpoint ep)
 	{
 		auto ls = std::make_shared<lt::aux::listen_socket_t>();
-		ls->external_address.cast_vote(ep.address(), 1, lt::address());
+		ls->external_address.cast_vote(ep.address()
+			, lt::aux::session_interface::source_dht, lt::address());
 		ls->local_endpoint = ep;
 		return ls;
 	}
diff --git a/simulation/test_dht_rate_limit.cpp b/simulation/test_dht_rate_limit.cpp
@@ -105,7 +105,8 @@ TORRENT_TEST(dht_rate_limit)
 	lt::udp_socket sock(dht_ios);
 	obs o;
 	auto ls = std::make_shared<lt::aux::listen_socket_t>();
-	ls->external_address.cast_vote(address_v4::from_string("40.30.20.10"), 1, lt::address());
+	ls->external_address.cast_vote(address_v4::from_string("40.30.20.10")
+		, lt::aux::session_interface::source_dht, lt::address());
 	ls->local_endpoint = tcp::endpoint(address_v4::from_string("40.30.20.10"), 8888);
 	error_code ec;
 	sock.bind(udp::endpoint(address_v4::from_string("40.30.20.10"), 8888), ec);
@@ -233,7 +234,8 @@ TORRENT_TEST(dht_delete_socket)
 
 	obs o;
 	auto ls = std::make_shared<lt::aux::listen_socket_t>();
-	ls->external_address.cast_vote(address_v4::from_string("40.30.20.10"), 1, lt::address());
+	ls->external_address.cast_vote(address_v4::from_string("40.30.20.10")
+		, lt::aux::session_interface::source_dht, lt::address());
 	ls->local_endpoint = tcp::endpoint(address_v4::from_string("40.30.20.10"), 8888);
 	dht::dht_settings dhtsett;
 	counters cnt;
diff --git a/src/ip_voter.cpp b/src/ip_voter.cpp
@@ -98,7 +98,7 @@ namespace libtorrent {
 	}
 
 	bool ip_voter::cast_vote(address const& ip
-		, int const source_type, address const& source)
+		, aux::ip_source_t const source_type, address const& source)
 	{
 		if (is_any(ip)) return false;
 		if (is_local(ip)) return false;
@@ -164,7 +164,8 @@ namespace libtorrent {
 		return true;
 	}
 
-	bool ip_voter::external_ip_t::add_vote(sha1_hash const& k, int type)
+	bool ip_voter::external_ip_t::add_vote(sha1_hash const& k
+		, aux::ip_source_t const type)
 	{
 		sources |= type;
 		if (voters.find(k)) return false;
diff --git a/src/session_impl.cpp b/src/session_impl.cpp
@@ -189,6 +189,11 @@ namespace libtorrent {
 
 namespace aux {
 
+	constexpr ip_source_t session_interface::source_dht;
+	constexpr ip_source_t session_interface::source_peer;
+	constexpr ip_source_t session_interface::source_tracker;
+	constexpr ip_source_t session_interface::source_router;
+
 	std::vector<std::shared_ptr<listen_socket_t>>::iterator partition_listen_sockets(
 		std::vector<listen_endpoint_t>& eps
 		, std::vector<std::shared_ptr<listen_socket_t>>& sockets)
@@ -6638,13 +6643,14 @@ namespace {
 	}
 
 	// this is the DHT observer version. DHT is the implied source
-	void session_impl::set_external_address(aux::listen_socket_handle const& iface, address const& ip
-		, address const& source)
+	void session_impl::set_external_address(aux::listen_socket_handle const& iface
+		, address const& ip, address const& source)
 	{
 		auto i = iface.m_sock.lock();
 		TORRENT_ASSERT(i);
 		if (!i) return;
-		set_external_address(std::static_pointer_cast<listen_socket_t>(i), ip, source_dht, source);
+		set_external_address(std::static_pointer_cast<listen_socket_t>(i), ip
+			, source_dht, source);
 	}
 
 	void session_impl::get_peers(sha1_hash const& ih)
@@ -6738,7 +6744,7 @@ namespace {
 	}
 
 	void session_impl::set_external_address(address const& ip
-		, int const source_type, address const& source)
+		, ip_source_t const source_type, address const& source)
 	{
 		// for now, just pick the first socket with a matching address family
 		// TODO: remove this function once all callers are updated to specify a listen socket
@@ -6754,7 +6760,7 @@ namespace {
 
 	void session_impl::set_external_address(
 		tcp::endpoint const& local_endpoint, address const& ip
-		, int const source_type, address const& source)
+		, ip_source_t const source_type, address const& source)
 	{
 		auto sock = std::find_if(m_listen_sockets.begin(), m_listen_sockets.end()
 			, [&](std::shared_ptr<listen_socket_t> const& v) { return v->local_endpoint == local_endpoint; });
@@ -6764,13 +6770,15 @@ namespace {
 	}
 
 	void session_impl::set_external_address(std::shared_ptr<listen_socket_t> const& sock
-		, address const& ip, int const source_type, address const& source)
+		, address const& ip, ip_source_t const source_type, address const& source)
 	{
 #ifndef TORRENT_DISABLE_LOGGING
 		if (should_log())
 		{
-			session_log(": set_external_address(%s, %d, %s)", print_address(ip).c_str()
-				, source_type, print_address(source).c_str());
+			session_log(": set_external_address(%s, %d, %s)"
+				, print_address(ip).c_str()
+				, static_cast<std::uint8_t>(source_type)
+				, print_address(source).c_str());
 		}
 #endif
 
diff --git a/test/test_dht.cpp b/test/test_dht.cpp
@@ -128,15 +128,17 @@ std::shared_ptr<aux::listen_socket_t> dummy_listen_socket(udp::endpoint src)
 {
 	auto ret = std::make_shared<aux::listen_socket_t>();
 	ret->local_endpoint = tcp::endpoint(src.address(), src.port());
-	ret->external_address.cast_vote(src.address(), 1, rand_v4());
+	ret->external_address.cast_vote(src.address()
+		, aux::session_interface::source_dht, rand_v4());
 	return ret;
 }
 
 std::shared_ptr<aux::listen_socket_t> dummy_listen_socket4()
 {
 	auto ret = std::make_shared<aux::listen_socket_t>();
 	ret->local_endpoint = tcp::endpoint(addr4("192.168.4.1"), 6881);
-	ret->external_address.cast_vote(addr4("236.0.0.1"), 1, rand_v4());
+	ret->external_address.cast_vote(addr4("236.0.0.1")
+		, aux::session_interface::source_dht, rand_v4());
 	return ret;
 }
 
@@ -145,7 +147,8 @@ std::shared_ptr<aux::listen_socket_t> dummy_listen_socket6()
 {
 	auto ret = std::make_shared<aux::listen_socket_t>();
 	ret->local_endpoint = tcp::endpoint(addr6("2002::1"), 6881);
-	ret->external_address.cast_vote(addr6("2002::1"), 1, rand_v6());
+	ret->external_address.cast_vote(addr6("2002::1")
+		, aux::session_interface::source_dht, rand_v6());
 	return ret;
 }
 #endif
@@ -519,7 +522,8 @@ struct obs : dht::dht_observer
 	void set_external_address(aux::listen_socket_handle const& s, address const& addr
 		, address const& source) override
 	{
-		s.get()->external_address.cast_vote(addr, 1, rand_v4());
+		s.get()->external_address.cast_vote(addr
+			, aux::session_interface::source_dht, rand_v4());
 	}
 
 	void get_peers(sha1_hash const& ih) override {}
diff --git a/test/test_ip_voter.cpp b/test/test_ip_voter.cpp
@@ -44,7 +44,7 @@ using namespace lt;
 
 bool cast_vote(ip_voter& ipv, address ext_ip, address voter)
 {
-	bool new_ip = ipv.cast_vote(ext_ip, 1, voter);
+	bool new_ip = ipv.cast_vote(ext_ip, aux::session_interface::source_dht, voter);
 	std::printf("%15s -> %-15s\n"
 		, print_address(voter).c_str()
 		, print_address(ext_ip).c_str());

Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,8 @@ namespace {`
`80`	`80`	`std::shared_ptr<lt::aux::listen_socket_t> sim_listen_socket(tcp::endpoint ep)`
`81`	`81`	`{`
`82`	`82`	`auto ls = std::make_shared<lt::aux::listen_socket_t>();`
`83`		`- ls->external_address.cast_vote(ep.address(), 1, lt::address());`
	`83`	`+ ls->external_address.cast_vote(ep.address()`
	`84`	`+ , lt::aux::session_interface::source_dht, lt::address());`
`84`	`85`	`ls->local_endpoint = ep;`
`85`	`86`	`return ls;`
`86`	`87`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ namespace libtorrent {`
`98`	`98`	`}`
`99`	`99`
`100`	`100`	`bool ip_voter::cast_vote(address const& ip`
`101`		`- , int const source_type, address const& source)`
	`101`	`+ , aux::ip_source_t const source_type, address const& source)`
`102`	`102`	`{`
`103`	`103`	`if (is_any(ip)) return false;`
`104`	`104`	`if (is_local(ip)) return false;`
`@@ -164,7 +164,8 @@ namespace libtorrent {`
`164`	`164`	`return true;`
`165`	`165`	`}`
`166`	`166`
`167`		`- bool ip_voter::external_ip_t::add_vote(sha1_hash const& k, int type)`
	`167`	`+ bool ip_voter::external_ip_t::add_vote(sha1_hash const& k`
	`168`	`+ , aux::ip_source_t const type)`
`168`	`169`	`{`
`169`	`170`	`sources \|= type;`
`170`	`171`	`if (voters.find(k)) return false;`
Original file line number	Diff line number	Diff line change
`@@ -128,15 +128,17 @@ std::shared_ptr<aux::listen_socket_t> dummy_listen_socket(udp::endpoint src)`
`128`	`128`	`{`
`129`	`129`	`auto ret = std::make_shared<aux::listen_socket_t>();`
`130`	`130`	`ret->local_endpoint = tcp::endpoint(src.address(), src.port());`
`131`		`- ret->external_address.cast_vote(src.address(), 1, rand_v4());`
	`131`	`+ ret->external_address.cast_vote(src.address()`
	`132`	`+ , aux::session_interface::source_dht, rand_v4());`
`132`	`133`	`return ret;`
`133`	`134`	`}`
`134`	`135`
`135`	`136`	`std::shared_ptr<aux::listen_socket_t> dummy_listen_socket4()`
`136`	`137`	`{`
`137`	`138`	`auto ret = std::make_shared<aux::listen_socket_t>();`
`138`	`139`	`ret->local_endpoint = tcp::endpoint(addr4("192.168.4.1"), 6881);`
`139`		`- ret->external_address.cast_vote(addr4("236.0.0.1"), 1, rand_v4());`
	`140`	`+ ret->external_address.cast_vote(addr4("236.0.0.1")`
	`141`	`+ , aux::session_interface::source_dht, rand_v4());`
`140`	`142`	`return ret;`
`141`	`143`	`}`
`142`	`144`
`@@ -145,7 +147,8 @@ std::shared_ptr<aux::listen_socket_t> dummy_listen_socket6()`
`145`	`147`	`{`
`146`	`148`	`auto ret = std::make_shared<aux::listen_socket_t>();`
`147`	`149`	`ret->local_endpoint = tcp::endpoint(addr6("2002::1"), 6881);`
`148`		`- ret->external_address.cast_vote(addr6("2002::1"), 1, rand_v6());`
	`150`	`+ ret->external_address.cast_vote(addr6("2002::1")`
	`151`	`+ , aux::session_interface::source_dht, rand_v6());`
`149`	`152`	`return ret;`
`150`	`153`	`}`
`151`	`154`	`#endif`
`@@ -519,7 +522,8 @@ struct obs : dht::dht_observer`
`519`	`522`	`void set_external_address(aux::listen_socket_handle const& s, address const& addr`
`520`	`523`	`, address const& source) override`
`521`	`524`	`{`
`522`		`- s.get()->external_address.cast_vote(addr, 1, rand_v4());`
	`525`	`+ s.get()->external_address.cast_vote(addr`
	`526`	`+ , aux::session_interface::source_dht, rand_v4());`
`523`	`527`	`}`
`524`	`528`
`525`	`529`	`void get_peers(sha1_hash const& ih) override {}`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ using namespace lt;`
`44`	`44`
`45`	`45`	`bool cast_vote(ip_voter& ipv, address ext_ip, address voter)`
`46`	`46`	`{`
`47`		`- bool new_ip = ipv.cast_vote(ext_ip, 1, voter);`
	`47`	`+ bool new_ip = ipv.cast_vote(ext_ip, aux::session_interface::source_dht, voter);`
`48`	`48`	`std::printf("%15s -> %-15s\n"`
`49`	`49`	`, print_address(voter).c_str()`
`50`	`50`	`, print_address(ext_ip).c_str());`