From 69485558efd2b316ecad25e6e188588ad81e48b8 Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 9 Nov 2018 18:48:33 +0100 Subject: [PATCH] factor out duplicate code in piece_picker, upnp and lazy_bdecode --- include/libtorrent/piece_picker.hpp | 2 + include/libtorrent/upnp.hpp | 2 + src/lazy_bdecode.cpp | 66 +++++++++++++++-------------- src/piece_picker.cpp | 39 +++-------------- src/upnp.cpp | 66 +++++++++++------------------ 5 files changed, 70 insertions(+), 105 deletions(-) diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index afd3933b4..e25d706d2 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -714,8 +714,10 @@ namespace libtorrent { private: +#if TORRENT_USE_ASSERTS || TORRENT_USE_INVARIANT_CHECKS index_range categories() const { return {{}, piece_picker::piece_pos::num_download_categories}; } +#endif // the following vectors are mutable because they sometimes may // be updated lazily, triggered by const functions diff --git a/include/libtorrent/upnp.hpp b/include/libtorrent/upnp.hpp index b52d83156..b24f5e261 100644 --- a/include/libtorrent/upnp.hpp +++ b/include/libtorrent/upnp.hpp @@ -209,6 +209,8 @@ private: void next(rootdevice& d, port_mapping_t i); void update_map(rootdevice& d, port_mapping_t i); + void connect(rootdevice& d); + void on_upnp_xml(error_code const& e , libtorrent::http_parser const& p, rootdevice& d , http_connection& c); diff --git a/src/lazy_bdecode.cpp b/src/lazy_bdecode.cpp index 62f0ed19c..731f44de8 100644 --- a/src/lazy_bdecode.cpp +++ b/src/lazy_bdecode.cpp @@ -80,6 +80,36 @@ namespace { return start; } + char const* parse_string(char const* start, char const* end + , bdecode_errors::error_code_enum& e, std::int64_t& len) + { + start = parse_int(start, end, ':', len, e); + if (e) return start; + if (start == end) + { + e = bdecode_errors::expected_colon; + } + else + { + // remaining buffer size excluding ':' + const ptrdiff_t buff_size = end - start - 1; + if (len > buff_size) + { + e = bdecode_errors::unexpected_eof; + } + else if (len < 0) + { + e = bdecode_errors::overflow; + } + else + { + ++start; + if (start >= end) e = bdecode_errors::unexpected_eof; + } + } + return start; + } + } // anonymous namespace #if TORRENT_ABI_VERSION == 1 @@ -130,22 +160,9 @@ namespace { if (!is_digit(t)) TORRENT_FAIL_BDECODE(bdecode_errors::expected_digit); std::int64_t len = t - '0'; bdecode_errors::error_code_enum e = bdecode_errors::no_error; - start = parse_int(start, end, ':', len, e); - if (e) - TORRENT_FAIL_BDECODE(e); - if (start == end) - TORRENT_FAIL_BDECODE(bdecode_errors::expected_colon); + start = parse_string(start, end, e, len); + if (e) TORRENT_FAIL_BDECODE(e); - // remaining buffer size excluding ':' - const ptrdiff_t buff_size = end - start - 1; - if (len > buff_size) - TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); - - if (len < 0) - TORRENT_FAIL_BDECODE(bdecode_errors::overflow); - - ++start; - if (start == end) TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); lazy_entry* ent = top->dict_append(start); if (ent == nullptr) TORRENT_FAIL_BDECODE(boost::system::errc::not_enough_memory); start += len; @@ -199,26 +216,13 @@ namespace { } default: { - if (!is_digit(t)) - TORRENT_FAIL_BDECODE(bdecode_errors::expected_value); + if (!is_digit(t)) TORRENT_FAIL_BDECODE(bdecode_errors::expected_value); std::int64_t len = t - '0'; bdecode_errors::error_code_enum e = bdecode_errors::no_error; - start = parse_int(start, end, ':', len, e); - if (e) - TORRENT_FAIL_BDECODE(e); - if (start == end) - TORRENT_FAIL_BDECODE(bdecode_errors::expected_colon); + start = parse_string(start, end, e, len); + if (e) TORRENT_FAIL_BDECODE(e); - // remaining buffer size excluding ':' - const ptrdiff_t buff_size = end - start - 1; - if (len > buff_size) - TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); - if (len < 0) - TORRENT_FAIL_BDECODE(bdecode_errors::overflow); - - ++start; - if (start == end) TORRENT_FAIL_BDECODE(bdecode_errors::unexpected_eof); top->construct_string(start, int(len)); start += len; stack.pop_back(); diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index d406bee25..627a3b254 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -381,8 +381,7 @@ namespace libtorrent { for (auto const k : categories()) { if (m_downloads[k].empty()) continue; - for (std::vector::const_iterator i = m_downloads[k].begin(); - i != m_downloads[k].end() - 1; ++i) + for (auto i = m_downloads[k].begin(); i != m_downloads[k].end() - 1; ++i) { downloading_piece const& dp = *i; downloading_piece const& next = *(i + 1); @@ -391,13 +390,11 @@ namespace libtorrent { + m_blocks_per_piece <= int(m_block_info.size())); for (auto const& bl : blocks_for_piece(dp)) { - if (bl.peer) - { - torrent_peer* p = bl.peer; - TORRENT_ASSERT(p->in_use); - TORRENT_ASSERT(p->connection == nullptr - || static_cast(p->connection)->m_in_use); - } + if (!bl.peer) continue; + torrent_peer* p = bl.peer; + TORRENT_ASSERT(p->in_use); + TORRENT_ASSERT(p->connection == nullptr + || static_cast(p->connection)->m_in_use); } } } @@ -472,29 +469,7 @@ namespace libtorrent { last = b; } - for (auto const k : categories()) - { - if (m_downloads[k].empty()) continue; - for (std::vector::const_iterator i = m_downloads[k].begin(); - i != m_downloads[k].end() - 1; ++i) - { - downloading_piece const& dp = *i; - downloading_piece const& next = *(i + 1); - TORRENT_ASSERT(dp.index < next.index); - TORRENT_ASSERT(int(dp.info_idx) * m_blocks_per_piece - + m_blocks_per_piece <= int(m_block_info.size())); -#if TORRENT_USE_ASSERTS - for (auto const& bl : blocks_for_piece(dp)) - { - if (!bl.peer) continue; - torrent_peer* p = bl.peer; - TORRENT_ASSERT(p->in_use); - TORRENT_ASSERT(p->connection == nullptr - || static_cast(p->connection)->m_in_use); - } -#endif - } - } + check_piece_state(); if (t != nullptr) TORRENT_ASSERT(num_pieces() == t->torrent_file().num_pieces()); diff --git a/src/upnp.cpp b/src/upnp.cpp index bdd425c76..cbe789267 100644 --- a/src/upnp.cpp +++ b/src/upnp.cpp @@ -344,29 +344,33 @@ void upnp::resend_request(error_code const& ec) // we don't have a WANIP or WANPPP url for this device, // ask for it - rootdevice& d = const_cast(dev); - TORRENT_ASSERT(d.magic == 1337); - TORRENT_TRY - { + connect(const_cast(dev)); + } +} + +void upnp::connect(rootdevice& d) +{ + TORRENT_ASSERT(d.magic == 1337); + TORRENT_TRY + { #ifndef TORRENT_DISABLE_LOGGING - log("connecting to: %s", d.url.c_str()); + log("connecting to: %s", d.url.c_str()); #endif - if (d.upnp_connection) d.upnp_connection->close(); - d.upnp_connection = std::make_shared(m_io_service - , m_resolver - , std::bind(&upnp::on_upnp_xml, self(), _1, _2 - , std::ref(d), _4)); - d.upnp_connection->get(d.url, seconds(30), 1); - } - TORRENT_CATCH (std::exception const& exc) - { - TORRENT_DECLARE_DUMMY(std::exception, exc); - TORRENT_UNUSED(exc); + if (d.upnp_connection) d.upnp_connection->close(); + d.upnp_connection = std::make_shared(m_io_service + , m_resolver + , std::bind(&upnp::on_upnp_xml, self(), _1, _2 + , std::ref(d), _4)); + d.upnp_connection->get(d.url, seconds(30), 1); + } + TORRENT_CATCH (std::exception const& exc) + { + TORRENT_DECLARE_DUMMY(std::exception, exc); + TORRENT_UNUSED(exc); #ifndef TORRENT_DISABLE_LOGGING - log("connection failed to: %s %s", d.url.c_str(), exc.what()); + log("connection failed to: %s %s", d.url.c_str(), exc.what()); #endif - d.disabled = true; - } + d.disabled = true; } } @@ -687,29 +691,7 @@ void upnp::try_map_upnp(bool const timer) { // we don't have a WANIP or WANPPP url for this device, // ask for it - rootdevice& d = const_cast(*i); - TORRENT_ASSERT(d.magic == 1337); - TORRENT_TRY - { -#ifndef TORRENT_DISABLE_LOGGING - log("connecting to: %s", d.url.c_str()); -#endif - if (d.upnp_connection) d.upnp_connection->close(); - d.upnp_connection = std::make_shared(m_io_service - , m_resolver - , std::bind(&upnp::on_upnp_xml, self(), _1, _2 - , std::ref(d), _4)); - d.upnp_connection->get(d.url, seconds(30), 1); - } - TORRENT_CATCH (std::exception const& exc) - { - TORRENT_DECLARE_DUMMY(std::exception, exc); - TORRENT_UNUSED(exc); -#ifndef TORRENT_DISABLE_LOGGING - log("connection failed to: %s %s", d.url.c_str(), exc.what()); -#endif - d.disabled = true; - } + connect(const_cast(*i)); } } }