From 4d4eb66c8b1726fa8be055d79d19b652e543011a Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Sun, 11 Sep 2016 01:58:48 -0400 Subject: [PATCH] some refactor, more const refs and span use (#1078) some refactor, more const refs and span use --- include/libtorrent/bdecode.hpp | 1 - include/libtorrent/bencode.hpp | 3 --- include/libtorrent/create_torrent.hpp | 2 +- include/libtorrent/kademlia/dht_tracker.hpp | 6 ++--- include/libtorrent/kademlia/find_data.hpp | 6 ++--- include/libtorrent/kademlia/get_peers.hpp | 2 +- include/libtorrent/kademlia/node.hpp | 6 +++-- include/libtorrent/kademlia/refresh.hpp | 3 +-- .../kademlia/traversal_algorithm.hpp | 2 +- include/libtorrent/settings_pack.hpp | 3 +-- simulation/test_dht_rate_limit.cpp | 2 +- src/kademlia/dht_tracker.cpp | 24 ++++++++++-------- src/kademlia/find_data.cpp | 2 +- src/kademlia/get_peers.cpp | 2 +- src/kademlia/node.cpp | 9 ++++--- src/kademlia/refresh.cpp | 2 +- src/kademlia/traversal_algorithm.cpp | 2 +- src/session.cpp | 6 ++--- src/session_impl.cpp | 6 ++--- src/settings_pack.cpp | 18 ++++++------- test/test_settings_pack.cpp | 25 ++++++++++++++++++- 21 files changed, 77 insertions(+), 55 deletions(-) diff --git a/include/libtorrent/bdecode.hpp b/include/libtorrent/bdecode.hpp index 7063f0c87..704b80d0d 100644 --- a/include/libtorrent/bdecode.hpp +++ b/include/libtorrent/bdecode.hpp @@ -418,4 +418,3 @@ TORRENT_EXPORT int bdecode(char const* start, char const* end, bdecode_node& ret } #endif // TORRENT_BDECODE_HPP - diff --git a/include/libtorrent/bencode.hpp b/include/libtorrent/bencode.hpp index a4f4cb420..651a74dcf 100644 --- a/include/libtorrent/bencode.hpp +++ b/include/libtorrent/bencode.hpp @@ -30,12 +30,9 @@ POSSIBILITY OF SUCH DAMAGE. */ - #ifndef TORRENT_BENCODE_HPP_INCLUDED #define TORRENT_BENCODE_HPP_INCLUDED - - // OVERVIEW // // Bencoding is a common representation in bittorrent used for for dictionary, diff --git a/include/libtorrent/create_torrent.hpp b/include/libtorrent/create_torrent.hpp index f3dad5fc8..37ca0b688 100644 --- a/include/libtorrent/create_torrent.hpp +++ b/include/libtorrent/create_torrent.hpp @@ -322,7 +322,7 @@ namespace libtorrent mutable std::vector m_merkle_tree; // dht nodes to add to the routing table/bootstrap from - typedef std::vector > nodes_t; + using nodes_t = std::vector>; nodes_t m_nodes; // the hash that identifies this torrent diff --git a/include/libtorrent/kademlia/dht_tracker.hpp b/include/libtorrent/kademlia/dht_tracker.hpp index f23fb5985..8821ab197 100644 --- a/include/libtorrent/kademlia/dht_tracker.hpp +++ b/include/libtorrent/kademlia/dht_tracker.hpp @@ -64,8 +64,8 @@ namespace libtorrent { namespace dht : udp_socket_interface , std::enable_shared_from_this { - typedef std::function, error_code&, int)> send_fun_t; + using send_fun_t = std::function, error_code&, int)>; dht_tracker(dht_observer* observer , io_service& ios @@ -129,7 +129,7 @@ namespace libtorrent { namespace dht void update_stats_counters(counters& c) const; void incoming_error(error_code const& ec, udp::endpoint const&); - bool incoming_packet(udp::endpoint const&, char const* buf, int size); + bool incoming_packet(udp::endpoint const& ep, span buf); private: diff --git a/include/libtorrent/kademlia/find_data.hpp b/include/libtorrent/kademlia/find_data.hpp index a496139aa..bd8f0368d 100644 --- a/include/libtorrent/kademlia/find_data.hpp +++ b/include/libtorrent/kademlia/find_data.hpp @@ -55,9 +55,9 @@ class node; struct find_data : traversal_algorithm { - typedef std::function > const&)> nodes_callback; + typedef std::function> const&)> nodes_callback; - find_data(node& node, node_id target + find_data(node& node, node_id const& target , nodes_callback const& ncallback); void got_write_token(node_id const& n, std::string write_token); @@ -66,7 +66,7 @@ struct find_data : traversal_algorithm virtual char const* name() const; - node_id const target() const { return m_target; } + node_id const& target() const { return m_target; } protected: diff --git a/include/libtorrent/kademlia/get_peers.hpp b/include/libtorrent/kademlia/get_peers.hpp index 9a0d38131..cfb95f28f 100644 --- a/include/libtorrent/kademlia/get_peers.hpp +++ b/include/libtorrent/kademlia/get_peers.hpp @@ -44,7 +44,7 @@ struct get_peers : find_data void got_peers(std::vector const& peers); - get_peers(node& dht_node, node_id target + get_peers(node& dht_node, node_id const& target , data_callback const& dcallback , nodes_callback const& ncallback , bool noseeds); diff --git a/include/libtorrent/kademlia/node.hpp b/include/libtorrent/kademlia/node.hpp index f7d94fcdc..323049192 100644 --- a/include/libtorrent/kademlia/node.hpp +++ b/include/libtorrent/kademlia/node.hpp @@ -99,7 +99,8 @@ class TORRENT_EXTRA_EXPORT node : boost::noncopyable { public: node(udp proto, udp_socket_interface* sock - , libtorrent::dht_settings const& settings, node_id nid + , libtorrent::dht_settings const& settings + , node_id const& nid , dht_observer* observer, counters& cnt , std::map const& nodes , dht_storage_interface& storage); @@ -247,7 +248,6 @@ private: public: routing_table m_table; rpc_manager m_rpc; - std::map const& m_nodes; private: #ifdef _MSC_VER @@ -269,6 +269,8 @@ private: #endif static protocol_descriptor const& map_protocol_to_descriptor(udp protocol); + std::map const& m_nodes; + dht_observer* m_observer; protocol_descriptor const& m_protocol; diff --git a/include/libtorrent/kademlia/refresh.hpp b/include/libtorrent/kademlia/refresh.hpp index c39150479..73a1880e1 100644 --- a/include/libtorrent/kademlia/refresh.hpp +++ b/include/libtorrent/kademlia/refresh.hpp @@ -48,7 +48,7 @@ class bootstrap : public get_peers public: typedef get_peers::nodes_callback done_callback; - bootstrap(node& dht_node, node_id target + bootstrap(node& dht_node, node_id const& target , done_callback const& callback); virtual char const* name() const; @@ -68,4 +68,3 @@ protected: } } // namespace libtorrent::dht #endif // REFRESH_050324_HPP - diff --git a/include/libtorrent/kademlia/traversal_algorithm.hpp b/include/libtorrent/kademlia/traversal_algorithm.hpp index f69fea1c7..22c0763c6 100644 --- a/include/libtorrent/kademlia/traversal_algorithm.hpp +++ b/include/libtorrent/kademlia/traversal_algorithm.hpp @@ -73,7 +73,7 @@ struct TORRENT_EXTRA_EXPORT traversal_algorithm : boost::noncopyable void resort_results(); void add_entry(node_id const& id, udp::endpoint const& addr, unsigned char flags); - traversal_algorithm(node & node, node_id target); + traversal_algorithm(node& node, node_id const& target); int invoke_count() const { return m_invoke_count; } int branch_factor() const { return m_branch_factor; } diff --git a/include/libtorrent/settings_pack.hpp b/include/libtorrent/settings_pack.hpp index 6fa51dd4b..2cf2666d0 100644 --- a/include/libtorrent/settings_pack.hpp +++ b/include/libtorrent/settings_pack.hpp @@ -56,7 +56,7 @@ namespace libtorrent struct settings_pack; struct bdecode_node; - TORRENT_EXTRA_EXPORT std::shared_ptr load_pack_from_dict(bdecode_node const& settings); + TORRENT_EXTRA_EXPORT settings_pack load_pack_from_dict(bdecode_node const& settings); TORRENT_EXTRA_EXPORT void save_settings_to_dict(aux::session_settings const& s, entry::dictionary_type& sett); TORRENT_EXTRA_EXPORT void apply_pack(settings_pack const* pack, aux::session_settings& sett , aux::session_impl* ses = nullptr); @@ -244,7 +244,6 @@ namespace libtorrent // ``send_redundant_have`` controls if have messages will be sent to // peers that already have the piece. This is typically not necessary, // but it might be necessary for collecting statistics in some cases. - // Default is false. send_redundant_have, #ifndef TORRENT_NO_DEPRECATE diff --git a/simulation/test_dht_rate_limit.cpp b/simulation/test_dht_rate_limit.cpp index 47d27fb67..2c7e16d2d 100644 --- a/simulation/test_dht_rate_limit.cpp +++ b/simulation/test_dht_rate_limit.cpp @@ -123,7 +123,7 @@ TORRENT_TEST(dht_rate_limit) udp_socket::packet p; error_code err; int const num = int(sock.read(lt::span(&p, 1), err)); - if (num) dht->incoming_packet(p.from, p.data.data(), int(p.data.size())); + if (num) dht->incoming_packet(p.from, p.data); if (stop || err) return; sock.async_read(on_read); }; diff --git a/src/kademlia/dht_tracker.cpp b/src/kademlia/dht_tracker.cpp index 89386e049..8f2d48834 100644 --- a/src/kademlia/dht_tracker.cpp +++ b/src/kademlia/dht_tracker.cpp @@ -136,10 +136,10 @@ namespace libtorrent { namespace dht #ifndef TORRENT_DISABLE_LOGGING m_log->log(dht_logger::tracker, "starting IPv4 DHT tracker with node id: %s" , aux::to_hex(m_dht.nid()).c_str()); - #if TORRENT_USE_IPV6 +#if TORRENT_USE_IPV6 m_log->log(dht_logger::tracker, "starting IPv6 DHT tracker with node id: %s" , aux::to_hex(m_dht6.nid()).c_str()); - #endif +#endif #endif } @@ -504,11 +504,14 @@ namespace libtorrent { namespace dht } bool dht_tracker::incoming_packet(udp::endpoint const& ep - , char const* buf, int size) + , span const buf) { - if (size <= 20 || *buf != 'd' || buf[size - 1] != 'e') return false; + int const buf_size = int(buf.size()); + if (buf_size <= 20 + || buf.front() != 'd' + || buf.back() != 'e') return false; - m_counters.inc_stats_counter(counters::dht_bytes_in, size); + m_counters.inc_stats_counter(counters::dht_bytes_in, buf_size); // account for IP and UDP overhead m_counters.inc_stats_counter(counters::recv_ip_overhead_bytes , ep.address().is_v6() ? 48 : 28); @@ -539,16 +542,16 @@ namespace libtorrent { namespace dht using libtorrent::entry; using libtorrent::bdecode; - TORRENT_ASSERT(size > 0); + TORRENT_ASSERT(buf_size > 0); int pos; error_code err; - int ret = bdecode(buf, buf + size, m_msg, err, &pos, 10, 500); + int ret = bdecode(buf.data(), buf.data() + buf_size, m_msg, err, &pos, 10, 500); if (ret != 0) { m_counters.inc_stats_counter(counters::dht_messages_in_dropped); #ifndef TORRENT_DISABLE_LOGGING - m_log->log_packet(dht_logger::incoming_message, buf, size, ep); + m_log->log_packet(dht_logger::incoming_message, buf.data(), buf_size, ep); #endif return false; } @@ -556,15 +559,14 @@ namespace libtorrent { namespace dht if (m_msg.type() != bdecode_node::dict_t) { #ifndef TORRENT_DISABLE_LOGGING - m_log->log_packet(dht_logger::incoming_message, buf, size, ep); + m_log->log_packet(dht_logger::incoming_message, buf.data(), buf_size, ep); #endif // it's not a good idea to send a response to an invalid messages return false; } #ifndef TORRENT_DISABLE_LOGGING - m_log->log_packet(dht_logger::incoming_message, buf - , size, ep); + m_log->log_packet(dht_logger::incoming_message, buf.data(), buf_size, ep); #endif libtorrent::dht::msg m(m_msg, ep); diff --git a/src/kademlia/find_data.cpp b/src/kademlia/find_data.cpp index 41f07f2eb..b29c8e49f 100644 --- a/src/kademlia/find_data.cpp +++ b/src/kademlia/find_data.cpp @@ -80,7 +80,7 @@ void find_data_observer::reply(msg const& m) find_data::find_data( node& dht_node - , node_id target + , node_id const& target , nodes_callback const& ncallback) : traversal_algorithm(dht_node, target) , m_nodes_callback(ncallback) diff --git a/src/kademlia/get_peers.cpp b/src/kademlia/get_peers.cpp index 0a7337392..9ec81f29b 100644 --- a/src/kademlia/get_peers.cpp +++ b/src/kademlia/get_peers.cpp @@ -122,7 +122,7 @@ void get_peers::got_peers(std::vector const& peers) get_peers::get_peers( node& dht_node - , node_id target + , node_id const& target , data_callback const& dcallback , nodes_callback const& ncallback , bool noseeds) diff --git a/src/kademlia/node.cpp b/src/kademlia/node.cpp index a99286832..a2774d943 100644 --- a/src/kademlia/node.cpp +++ b/src/kademlia/node.cpp @@ -75,16 +75,16 @@ void nop() {} node_id calculate_node_id(node_id const& nid, dht_observer* observer, udp protocol) { address external_address; - if (observer) external_address = observer->external_address(protocol); + if (observer != nullptr) external_address = observer->external_address(protocol); // if we don't have an observer, don't pretend that external_address is valid // generating an ID based on 0.0.0.0 would be terrible. random is better - if (!observer || external_address.is_unspecified()) + if (observer == nullptr || external_address.is_unspecified()) { return generate_random_id(); } - if (nid == (node_id::min)() || !verify_id(nid, external_address)) + if (nid == node_id::min() || !verify_id(nid, external_address)) return generate_id(external_address); return nid; @@ -93,7 +93,8 @@ node_id calculate_node_id(node_id const& nid, dht_observer* observer, udp protoc } // anonymous namespace node::node(udp proto, udp_socket_interface* sock - , dht_settings const& settings, node_id nid + , dht_settings const& settings + , node_id const& nid , dht_observer* observer , counters& cnt , std::map const& nodes diff --git a/src/kademlia/refresh.cpp b/src/kademlia/refresh.cpp index d4f89dce7..ae7e2fd05 100644 --- a/src/kademlia/refresh.cpp +++ b/src/kademlia/refresh.cpp @@ -73,7 +73,7 @@ bool bootstrap::invoke(observer_ptr o) bootstrap::bootstrap( node& dht_node - , node_id target + , node_id const& target , done_callback const& callback) : get_peers(dht_node, target, get_peers::data_callback(), callback, false) { diff --git a/src/kademlia/traversal_algorithm.cpp b/src/kademlia/traversal_algorithm.cpp index e01d4d47e..f5a9e63d1 100644 --- a/src/kademlia/traversal_algorithm.cpp +++ b/src/kademlia/traversal_algorithm.cpp @@ -86,7 +86,7 @@ observer_ptr traversal_algorithm::new_observer(udp::endpoint const& ep traversal_algorithm::traversal_algorithm( node& dht_node - , node_id target) + , node_id const& target) : m_node(dht_node) , m_target(target) , m_invoke_count(0) diff --git a/src/session.cpp b/src/session.cpp index feab7641d..73819a605 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -252,7 +252,7 @@ namespace libtorrent // this function returns a session_settings object // which will optimize libtorrent for minimum memory // usage, with no consideration of performance. - TORRENT_EXPORT session_settings min_memory_usage() + session_settings min_memory_usage() { aux::session_settings def; initialize_default_settings(def); @@ -264,7 +264,7 @@ namespace libtorrent return ret; } - TORRENT_EXPORT session_settings high_performance_seed() + session_settings high_performance_seed() { aux::session_settings def; initialize_default_settings(def); @@ -285,7 +285,7 @@ namespace libtorrent // on the configuration. The session.hpp file will reference // it and if the library and the client are built with different // configurations this will give a link error - void TORRENT_EXPORT TORRENT_CFG() {} + void TORRENT_CFG() {} void session::start(session_params params, io_service* ios) { diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 1f0d64baa..bad9dd3ef 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -704,8 +704,8 @@ namespace aux { if (settings) { // apply_settings_pack will update dht and proxy - std::shared_ptr pack = load_pack_from_dict(settings); - apply_settings_pack(pack); + settings_pack pack = load_pack_from_dict(settings); + apply_settings_pack_impl(pack); #ifndef TORRENT_DISABLE_DHT need_update_dht = false; #endif @@ -2306,7 +2306,7 @@ namespace aux { #ifndef TORRENT_DISABLE_DHT if (m_dht && buf.size() > 20 && buf.front() == 'd' && buf.back() == 'e') { - handled = m_dht->incoming_packet(packet.from, buf.data(), int(buf.size())); + handled = m_dht->incoming_packet(packet.from, buf); } #endif diff --git a/src/settings_pack.cpp b/src/settings_pack.cpp index 957042ef1..2782517a4 100644 --- a/src/settings_pack.cpp +++ b/src/settings_pack.cpp @@ -388,9 +388,9 @@ namespace libtorrent return ""; } - std::shared_ptr load_pack_from_dict(bdecode_node const& settings) + settings_pack load_pack_from_dict(bdecode_node const& settings) { - std::shared_ptr pack = std::make_shared(); + settings_pack pack; for (int i = 0; i < settings.dict_size(); ++i) { @@ -405,27 +405,27 @@ namespace libtorrent case bdecode_node::int_t: { bool found = false; - for (int k = 0; k < sizeof(int_settings)/sizeof(int_settings[0]); ++k) + for (int k = 0; k < sizeof(int_settings) / sizeof(int_settings[0]); ++k) { if (key != int_settings[k].name) continue; - pack->set_int(settings_pack::int_type_base + k, val.int_value()); + pack.set_int(settings_pack::int_type_base + k, val.int_value()); found = true; break; } if (found) continue; - for (int k = 0; k < sizeof(bool_settings)/sizeof(bool_settings[0]); ++k) + for (int k = 0; k < sizeof(bool_settings) / sizeof(bool_settings[0]); ++k) { if (key != bool_settings[k].name) continue; - pack->set_bool(settings_pack::bool_type_base + k, val.int_value() != 0); + pack.set_bool(settings_pack::bool_type_base + k, val.int_value() != 0); break; } } break; case bdecode_node::string_t: - for (int k = 0; k < sizeof(str_settings)/sizeof(str_settings[0]); ++k) + for (int k = 0; k < sizeof(str_settings) / sizeof(str_settings[0]); ++k) { if (key != str_settings[k].name) continue; - pack->set_str(settings_pack::string_type_base + k, val.string_value().to_string()); + pack.set_str(settings_pack::string_type_base + k, val.string_value().to_string()); break; } break; @@ -547,7 +547,7 @@ namespace libtorrent #include "libtorrent/aux_/disable_warnings_pop.hpp" -#endif +#endif // TORRENT_NO_DEPRECATE void initialize_default_settings(aux::session_settings& s) { diff --git a/test/test_settings_pack.cpp b/test/test_settings_pack.cpp index 97549f423..3f2b2759c 100644 --- a/test/test_settings_pack.cpp +++ b/test/test_settings_pack.cpp @@ -35,6 +35,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/aux_/session_settings.hpp" #include "libtorrent/entry.hpp" #include "libtorrent/bencode.hpp" +#include "libtorrent/bdecode.hpp" #include using namespace libtorrent; @@ -142,5 +143,27 @@ TORRENT_TEST(duplicates) TEST_EQUAL(p.get_str(settings_pack::peer_fingerprint), "hij"); } -// TODO: load_pack_from_dict +TORRENT_TEST(load_pack_from_dict) +{ + aux::session_settings p1; + p1.set_str(settings_pack::peer_fingerprint, "abc"); + p1.set_int(settings_pack::max_out_request_queue, 1337); + p1.set_bool(settings_pack::send_redundant_have, false); + entry e; + save_settings_to_dict(p1, e.dict()); + + std::string s; + bencode(std::back_inserter(s), e); + + bdecode_node n; + error_code ec; + int ret = bdecode(s.data(), s.data() + int(s.size()), n, ec); + TEST_EQUAL(ret, 0); + TEST_CHECK(!ec); + + settings_pack p2 = load_pack_from_dict(n); + TEST_EQUAL(p2.get_str(settings_pack::peer_fingerprint), "abc"); + TEST_EQUAL(p2.get_int(settings_pack::max_out_request_queue), 1337); + TEST_EQUAL(p2.get_bool(settings_pack::send_redundant_have), false); +}