diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index bf6d08515..42dc6f8eb 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -1207,15 +1207,12 @@ void create_bit_torrent_peer(tr_torrent* tor, std::shared_ptr io, tr_ /* FIXME: this is kind of a mess. */ [[nodiscard]] bool on_handshake_done(tr_peerMgr* const manager, tr_handshake::Result const& result) { + auto const lock = manager->unique_lock(); + TR_ASSERT(result.io != nullptr); - - bool const ok = result.is_connected; - - auto* const s = manager->get_existing_swarm(result.io->torrent_hash()); - auto const& socket_address = result.io->socket_address(); - - auto* const info_ptr = s != nullptr ? s->get_existing_peer_info(socket_address) : nullptr; + auto* const swarm = manager->get_existing_swarm(result.io->torrent_hash()); + auto* info_ptr = swarm != nullptr ? swarm->get_existing_peer_info(socket_address) : nullptr; if (result.io->is_incoming()) { @@ -1226,9 +1223,7 @@ void create_bit_torrent_peer(tr_torrent* tor, std::shared_ptr io, tr_ info_ptr->destroy_handshake(); } - auto const lock = manager->unique_lock(); - - if (!ok || s == nullptr || !s->is_running) + if (!result.is_connected || swarm == nullptr || !swarm->is_running) { if (info_ptr != nullptr && !info_ptr->is_connected()) { @@ -1237,7 +1232,7 @@ void create_bit_torrent_peer(tr_torrent* tor, std::shared_ptr io, tr_ if (!result.read_anything_from_peer) { tr_logAddTraceSwarm( - s, + swarm, fmt::format( "marking peer {} as unreachable... num_fails is {}", info_ptr->display_name(), @@ -1245,54 +1240,59 @@ void create_bit_torrent_peer(tr_torrent* tor, std::shared_ptr io, tr_ info_ptr->set_connectable(false); } } + + return false; } - else /* looking good */ + + if (info_ptr == nullptr && result.io->is_incoming()) { - // If this is an outgoing connection, then we are sure we already have the peer info object - auto& info = result.io->is_incoming() ? s->ensure_info_exists(socket_address, 0U, TR_PEER_FROM_INCOMING, false) : - *info_ptr; - - if (!result.io->is_incoming()) - { - info.set_connectable(); - } - - // If we're connected via µTP, then we know the peer supports µTP... - if (result.io->is_utp()) - { - info.set_utp_supported(); - } - - if (info.is_banned()) - { - tr_logAddTraceSwarm(s, fmt::format("banned peer {} tried to reconnect", info.display_name())); - } - else if (s->peerCount() >= s->tor->peer_limit()) - { - // too many peers already - } - else if (info.is_connected()) - { - // we're already connected to this peer; do nothing - } - else - { - auto client = tr_interned_string{}; - if (result.peer_id) - { - auto buf = std::array{}; - tr_clientForId(std::data(buf), sizeof(buf), *result.peer_id); - client = tr_interned_string{ tr_quark_new(std::data(buf)) }; - } - - result.io->set_bandwidth(&s->tor->bandwidth_); - create_bit_torrent_peer(s->tor, result.io, &info, client); - - return true; - } + info_ptr = &swarm->ensure_info_exists(socket_address, 0U, TR_PEER_FROM_INCOMING, false); } - return false; + if (info_ptr == nullptr) + { + return false; + } + + if (!result.io->is_incoming()) + { + info_ptr->set_connectable(); + } + + // If we're connected via µTP, then we know the peer supports µTP... + if (result.io->is_utp()) + { + info_ptr->set_utp_supported(); + } + + if (info_ptr->is_banned()) + { + tr_logAddTraceSwarm(swarm, fmt::format("banned peer {} tried to reconnect", info_ptr->display_name())); + return false; + } + + if (swarm->peerCount() >= swarm->tor->peer_limit()) // too many peers already + { + return false; + } + + if (info_ptr->is_connected()) // we're already connected to this peer; do nothing + { + return false; + } + + auto client = tr_interned_string{}; + if (result.peer_id) + { + auto buf = std::array{}; + tr_clientForId(std::data(buf), sizeof(buf), *result.peer_id); + client = tr_interned_string{ tr_quark_new(std::data(buf)) }; + } + + result.io->set_bandwidth(&swarm->tor->bandwidth_); + create_bit_torrent_peer(swarm->tor, result.io, info_ptr, client); + + return true; } } // namespace handshake_helpers } // namespace diff --git a/tests/libtransmission/file-piece-map-test.cc b/tests/libtransmission/file-piece-map-test.cc index 3521e5056..c6c94ce0c 100644 --- a/tests/libtransmission/file-piece-map-test.cc +++ b/tests/libtransmission/file-piece-map-test.cc @@ -171,14 +171,14 @@ TEST_F(FilePieceMapTest, priorities) { for (tr_file_index_t i = 0U; i < n_files; ++i) { - auto const expected = int{ expected_file_priorities[i] }; - auto const actual = int{ file_priorities.file_priority(i) }; + auto const expected = expected_file_priorities[i]; + auto const actual = file_priorities.file_priority(i); EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']'; } for (tr_piece_index_t i = 0U; i < block_info_.piece_count(); ++i) { - auto const expected = int{ expected_piece_priorities[i] }; - auto const actual = int{ file_priorities.piece_priority(i) }; + auto const expected = expected_piece_priorities[i]; + auto const actual = file_priorities.piece_priority(i); EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']'; } }; @@ -333,14 +333,14 @@ TEST_F(FilePieceMapTest, wanted) { for (tr_file_index_t i = 0U; i < n_files; ++i) { - auto const expected = int{ expected_files_wanted.test(i) }; - auto const actual = int{ files_wanted.file_wanted(i) }; + auto const expected = expected_files_wanted.test(i); + auto const actual = files_wanted.file_wanted(i); EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']'; } for (tr_piece_index_t i = 0U; i < block_info_.piece_count(); ++i) { - auto const expected = int{ expected_pieces_wanted.test(i) }; - auto const actual = int{ files_wanted.piece_wanted(i) }; + auto const expected = expected_pieces_wanted.test(i); + auto const actual = files_wanted.piece_wanted(i); EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']'; } };