From 7c5862a5f5db422896a708f77d681687659fa3c1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 11 Aug 2022 19:59:58 -0500 Subject: [PATCH] refactor: remove tr_new, tr_free pt. 1 (#3626) * refactor: tr_torrentGetMetadataPiece() returns an optional vector * refactor: use tr_pathbuf in create_temp_path() * refactor: use tr_pathbuf in win32 create_temp_path() * refactor: use std::vector in isPeerInteresting() * refactor: remove tr_new0 from tr_peerMgrPeerStats() * refactor: remove tr_new0 from rechokeUploads() * refactor: silence clang nullptr dereference warning * refactor: make tr_natpmp a C++ class * refactor: use std::string in tr_log_message --- daemon/daemon.cc | 3 +- gtk/MessageLogWindow.cc | 14 +-- libtransmission/file-posix.cc | 7 +- libtransmission/file-win32.cc | 11 +-- libtransmission/log.cc | 10 +- libtransmission/log.h | 9 +- libtransmission/natpmp.cc | 145 +++++++++++------------------ libtransmission/natpmp_local.h | 88 ++++++++++------- libtransmission/peer-mgr.cc | 102 ++++++++++---------- libtransmission/peer-msgs.cc | 9 +- libtransmission/port-forwarding.cc | 47 +++++----- libtransmission/port-forwarding.h | 6 +- libtransmission/session.cc | 10 +- libtransmission/session.h | 2 +- libtransmission/torrent-magnet.cc | 29 +++--- libtransmission/torrent-magnet.h | 6 +- libtransmission/torrent.cc | 2 +- macosx/MessageWindowController.mm | 7 +- 18 files changed, 239 insertions(+), 268 deletions(-) diff --git a/daemon/daemon.cc b/daemon/daemon.cc index 892fe9543..bbe3abeed 100644 --- a/daemon/daemon.cc +++ b/daemon/daemon.cc @@ -396,8 +396,7 @@ static void pumpLogMessages(tr_sys_file_t file) for (tr_log_message const* l = list; l != nullptr; l = l->next) { - auto const name = std::string_view{ l->name != nullptr ? l->name : "" }; - printMessage(file, l->level, name, l->message, l->file, l->line); + printMessage(file, l->level, l->name, l->message, l->file, l->line); } if (file != TR_BAD_SYS_FILE) diff --git a/gtk/MessageLogWindow.cc b/gtk/MessageLogWindow.cc index 7b6378d94..74250ea64 100644 --- a/gtk/MessageLogWindow.cc +++ b/gtk/MessageLogWindow.cc @@ -208,13 +208,7 @@ void MessageLogWindow::Impl::doSave(Gtk::Window& parent, Glib::ustring const& fi auto const it = level_names_.find(node->level); auto const* const level_str = it != std::end(level_names_) ? it->second : "???"; - fprintf( - fp, - "%s\t%s\t%s\t%s\n", - date.c_str(), - level_str, - node->name != nullptr ? node->name : "", - node->message != nullptr ? node->message : ""); + fprintf(fp, "%s\t%s\t%s\t%s\n", date.c_str(), level_str, node->name.c_str(), node->message.c_str()); } fclose(fp); @@ -356,7 +350,7 @@ tr_log_message* addMessages(Glib::RefPtr const& store, tr_log_me for (i = head; i != nullptr && i->next != nullptr; i = i->next) { - char const* name = i->name != nullptr ? i->name : default_name.c_str(); + char const* name = !std::empty(i->name) ? i->name.c_str() : default_name.c_str(); auto const row = *store->prepend(); row[message_log_cols.tr_msg] = i; @@ -369,9 +363,9 @@ tr_log_message* addMessages(Glib::RefPtr const& store, tr_log_me { auto gstr = gtr_sprintf("%s:%d %s", i->file, i->line, i->message); - if (i->name != nullptr) + if (!std::empty(i->name)) { - gstr += gtr_sprintf(" (%s)", i->name); + gstr += gtr_sprintf(" (%s)", i->name.c_str()); } g_warning("%s", gstr.c_str()); diff --git a/libtransmission/file-posix.cc b/libtransmission/file-posix.cc index 9365bfa84..252478cef 100644 --- a/libtransmission/file-posix.cc +++ b/libtransmission/file-posix.cc @@ -59,7 +59,7 @@ #include "log.h" #include "tr-assert.h" #include "tr-strbuf.h" -#include "utils.h" +#include "utils.h" // for _(), tr_strerror() #ifndef O_LARGEFILE #define O_LARGEFILE 0 @@ -192,10 +192,10 @@ static bool create_path_require_dir(char const* path, tr_error** error) static bool create_path(char const* path_in, int permissions, tr_error** error) { /* make a temporary copy of path */ - char* const path = tr_strdup(path_in); + auto path = tr_pathbuf{ path_in }; /* walk past the root */ - char* p = path; + char* p = std::data(path); while (*p == TR_PATH_DELIMITER) { @@ -283,7 +283,6 @@ CLEANUP: TR_ASSERT(my_error == nullptr); - tr_free(path); return ret; } diff --git a/libtransmission/file-win32.cc b/libtransmission/file-win32.cc index a6fb06633..88dd9e02e 100644 --- a/libtransmission/file-win32.cc +++ b/libtransmission/file-win32.cc @@ -15,6 +15,7 @@ #include #include "transmission.h" + #include "crypto-utils.h" /* tr_rand_int() */ #include "error.h" #include "file.h" @@ -298,8 +299,8 @@ static void create_temp_path( TR_ASSERT(path_template != nullptr); TR_ASSERT(callback != nullptr); - char* path = tr_strdup(path_template); - size_t path_size = strlen(path); + auto path = std::string{ path_template }; + auto path_size = std::size(path); TR_ASSERT(path_size > 0); @@ -320,7 +321,7 @@ static void create_temp_path( tr_error_clear(&my_error); - (*callback)(path, callback_param, &my_error); + (*callback)(path.c_str(), callback_param, &my_error); if (my_error == nullptr) { @@ -334,10 +335,8 @@ static void create_temp_path( } else { - memcpy(path_template, path, path_size); + std::copy_n(std::begin(path), path_size, path_template); } - - tr_free(path); } bool tr_sys_path_exists(char const* path, tr_error** error) diff --git a/libtransmission/log.cc b/libtransmission/log.cc index 61d1e37d8..81202edb1 100644 --- a/libtransmission/log.cc +++ b/libtransmission/log.cc @@ -133,13 +133,13 @@ void logAddImpl( if (tr_logGetQueueEnabled()) { - auto* const newmsg = tr_new0(tr_log_message, 1); + auto* const newmsg = new tr_log_message{}; newmsg->level = level; newmsg->when = tr_time(); - newmsg->message = tr_strvDup(msg); + newmsg->message = msg; newmsg->file = file; newmsg->line = line; - newmsg->name = tr_strvDup(name); + newmsg->name = name; *log_state.queue_tail_ = newmsg; log_state.queue_tail_ = &newmsg->next; @@ -221,9 +221,7 @@ void tr_logFreeQueue(tr_log_message* freeme) while (freeme != nullptr) { auto* const next = freeme->next; - tr_free(freeme->message); - tr_free(freeme->name); - tr_free(freeme); + delete freeme; freeme = next; } } diff --git a/libtransmission/log.h b/libtransmission/log.h index 0068ea5af..24dc18435 100644 --- a/libtransmission/log.h +++ b/libtransmission/log.h @@ -8,6 +8,7 @@ #include #include #include +#include #include /// @@ -50,17 +51,17 @@ struct tr_log_message tr_log_level level; // location in the source code - char const* file; - int line; + std::string_view file; + size_t line; // when the message was generated time_t when; // torrent name or code module name associated with the message - char* name; + std::string name; // the message - char* message; + std::string message; // linked list of messages struct tr_log_message* next; diff --git a/libtransmission/natpmp.cc b/libtransmission/natpmp.cc index 311b1637b..139968000 100644 --- a/libtransmission/natpmp.cc +++ b/libtransmission/natpmp.cc @@ -21,13 +21,6 @@ #include "port-forwarding.h" #include "utils.h" -static auto constexpr LifetimeSecs = uint32_t{ 3600 }; -static auto constexpr CommandWaitSecs = time_t{ 8 }; - -/** -*** -**/ - static void logVal(char const* func, int ret) { if (ret == NATPMP_TRYAGAIN) @@ -51,57 +44,33 @@ static void logVal(char const* func, int ret) } } -struct tr_natpmp* tr_natpmpInit() +bool tr_natpmp::canSendCommand() const { - auto* const nat = tr_new0(struct tr_natpmp, 1); - nat->state = TR_NATPMP_DISCOVER; - nat->public_port.clear(); - nat->private_port.clear(); - nat->natpmp.s = TR_BAD_SOCKET; /* socket */ - return nat; + return tr_time() >= command_time_; } -void tr_natpmpClose(tr_natpmp* nat) +void tr_natpmp::setCommandTime() { - if (nat != nullptr) + command_time_ = tr_time() + CommandWaitSecs; +} + +tr_port_forwarding tr_natpmp::pulse(tr_port private_port, bool is_enabled, tr_port* public_port, tr_port* real_private_port) +{ + if (is_enabled && state_ == TR_NATPMP_DISCOVER) { - closenatpmp(&nat->natpmp); - tr_free(nat); - } -} - -static bool canSendCommand(struct tr_natpmp const* nat) -{ - return tr_time() >= nat->command_time; -} - -static void setCommandTime(struct tr_natpmp* nat) -{ - nat->command_time = tr_time() + CommandWaitSecs; -} - -tr_port_forwarding tr_natpmpPulse( - struct tr_natpmp* nat, - tr_port private_port, - bool is_enabled, - tr_port* public_port, - tr_port* real_private_port) -{ - if (is_enabled && nat->state == TR_NATPMP_DISCOVER) - { - int val = initnatpmp(&nat->natpmp, 0, 0); + int val = initnatpmp(&natpmp_, 0, 0); logVal("initnatpmp", val); - val = sendpublicaddressrequest(&nat->natpmp); + val = sendpublicaddressrequest(&natpmp_); logVal("sendpublicaddressrequest", val); - nat->state = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_PUB; - nat->has_discovered = true; - setCommandTime(nat); + state_ = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_PUB; + has_discovered_ = true; + setCommandTime(); } - if (nat->state == TR_NATPMP_RECV_PUB && canSendCommand(nat)) + if (state_ == TR_NATPMP_RECV_PUB && canSendCommand()) { natpmpresp_t response; - int const val = readnatpmpresponseorretry(&nat->natpmp, &response); + int const val = readnatpmpresponseorretry(&natpmp_, &response); logVal("readnatpmpresponseorretry", val); if (val >= 0) @@ -109,37 +78,31 @@ tr_port_forwarding tr_natpmpPulse( char str[128]; evutil_inet_ntop(AF_INET, &response.pnu.publicaddress.addr, str, sizeof(str)); tr_logAddInfo(fmt::format(_("Found public address '{address}'"), fmt::arg("address", str))); - nat->state = TR_NATPMP_IDLE; + state_ = TR_NATPMP_IDLE; } else if (val != NATPMP_TRYAGAIN) { - nat->state = TR_NATPMP_ERR; + state_ = TR_NATPMP_ERR; } } - if ((nat->state == TR_NATPMP_IDLE || nat->state == TR_NATPMP_ERR) && (nat->is_mapped) && - (!is_enabled || nat->private_port != private_port)) + if ((state_ == TR_NATPMP_IDLE || state_ == TR_NATPMP_ERR) && is_mapped_ && (!is_enabled || private_port_ != private_port)) { - nat->state = TR_NATPMP_SEND_UNMAP; + state_ = TR_NATPMP_SEND_UNMAP; } - if (nat->state == TR_NATPMP_SEND_UNMAP && canSendCommand(nat)) + if (state_ == TR_NATPMP_SEND_UNMAP && canSendCommand()) { - int const val = sendnewportmappingrequest( - &nat->natpmp, - NATPMP_PROTOCOL_TCP, - nat->private_port.host(), - nat->public_port.host(), - 0); + int const val = sendnewportmappingrequest(&natpmp_, NATPMP_PROTOCOL_TCP, private_port_.host(), public_port_.host(), 0); logVal("sendnewportmappingrequest", val); - nat->state = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_UNMAP; - setCommandTime(nat); + state_ = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_UNMAP; + setCommandTime(); } - if (nat->state == TR_NATPMP_RECV_UNMAP) + if (state_ == TR_NATPMP_RECV_UNMAP) { natpmpresp_t resp; - int const val = readnatpmpresponseorretry(&nat->natpmp, &resp); + int const val = readnatpmpresponseorretry(&natpmp_, &resp); logVal("readnatpmpresponseorretry", val); if (val >= 0) @@ -148,72 +111,72 @@ tr_port_forwarding tr_natpmpPulse( tr_logAddInfo(fmt::format(_("Port {port} is no longer forwarded"), fmt::arg("port", unmapped_port.host()))); - if (nat->private_port == unmapped_port) + if (private_port_ == unmapped_port) { - nat->private_port.clear(); - nat->public_port.clear(); - nat->state = TR_NATPMP_IDLE; - nat->is_mapped = false; + private_port_.clear(); + public_port_.clear(); + state_ = TR_NATPMP_IDLE; + is_mapped_ = false; } } else if (val != NATPMP_TRYAGAIN) { - nat->state = TR_NATPMP_ERR; + state_ = TR_NATPMP_ERR; } } - if (nat->state == TR_NATPMP_IDLE) + if (state_ == TR_NATPMP_IDLE) { - if (is_enabled && !nat->is_mapped && nat->has_discovered) + if (is_enabled && !is_mapped_ && has_discovered_) { - nat->state = TR_NATPMP_SEND_MAP; + state_ = TR_NATPMP_SEND_MAP; } - else if (nat->is_mapped && tr_time() >= nat->renew_time) + else if (is_mapped_ && tr_time() >= renew_time_) { - nat->state = TR_NATPMP_SEND_MAP; + state_ = TR_NATPMP_SEND_MAP; } } - if (nat->state == TR_NATPMP_SEND_MAP && canSendCommand(nat)) + if (state_ == TR_NATPMP_SEND_MAP && canSendCommand()) { int const val = sendnewportmappingrequest( - &nat->natpmp, + &natpmp_, NATPMP_PROTOCOL_TCP, private_port.host(), private_port.host(), LifetimeSecs); logVal("sendnewportmappingrequest", val); - nat->state = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_MAP; - setCommandTime(nat); + state_ = val < 0 ? TR_NATPMP_ERR : TR_NATPMP_RECV_MAP; + setCommandTime(); } - if (nat->state == TR_NATPMP_RECV_MAP) + if (state_ == TR_NATPMP_RECV_MAP) { natpmpresp_t resp; - int const val = readnatpmpresponseorretry(&nat->natpmp, &resp); + int const val = readnatpmpresponseorretry(&natpmp_, &resp); logVal("readnatpmpresponseorretry", val); if (val >= 0) { - nat->state = TR_NATPMP_IDLE; - nat->is_mapped = true; - nat->renew_time = tr_time() + (resp.pnu.newportmapping.lifetime / 2); - nat->private_port = tr_port::fromHost(resp.pnu.newportmapping.privateport); - nat->public_port = tr_port::fromHost(resp.pnu.newportmapping.mappedpublicport); - tr_logAddInfo(fmt::format(_("Port {port} forwarded successfully"), fmt::arg("port", nat->private_port.host()))); + state_ = TR_NATPMP_IDLE; + is_mapped_ = true; + renew_time_ = tr_time() + (resp.pnu.newportmapping.lifetime / 2); + private_port_ = tr_port::fromHost(resp.pnu.newportmapping.privateport); + public_port_ = tr_port::fromHost(resp.pnu.newportmapping.mappedpublicport); + tr_logAddInfo(fmt::format(_("Port {port} forwarded successfully"), fmt::arg("port", private_port_.host()))); } else if (val != NATPMP_TRYAGAIN) { - nat->state = TR_NATPMP_ERR; + state_ = TR_NATPMP_ERR; } } - switch (nat->state) + switch (state_) { case TR_NATPMP_IDLE: - *public_port = nat->public_port; - *real_private_port = nat->private_port; - return nat->is_mapped ? TR_PORT_MAPPED : TR_PORT_UNMAPPED; + *public_port = public_port_; + *real_private_port = private_port_; + return is_mapped_ ? TR_PORT_MAPPED : TR_PORT_UNMAPPED; case TR_NATPMP_DISCOVER: return TR_PORT_UNMAPPED; diff --git a/libtransmission/natpmp_local.h b/libtransmission/natpmp_local.h index a49a185be..357876fb6 100644 --- a/libtransmission/natpmp_local.h +++ b/libtransmission/natpmp_local.h @@ -9,46 +9,62 @@ #error only libtransmission should #include this header. #endif -/** - * @addtogroup port_forwarding Port Forwarding - * @{ - */ - #include // time_t +#include "transmission.h" // tr_port_forwarding + #include "natpmp.h" #include "net.h" // tr_port -enum tr_natpmp_state +class tr_natpmp { - TR_NATPMP_IDLE, - TR_NATPMP_ERR, - TR_NATPMP_DISCOVER, - TR_NATPMP_RECV_PUB, - TR_NATPMP_SEND_MAP, - TR_NATPMP_RECV_MAP, - TR_NATPMP_SEND_UNMAP, - TR_NATPMP_RECV_UNMAP +public: + tr_natpmp() + { + natpmp_.s = TR_BAD_SOCKET; /* socket */ + } + + ~tr_natpmp() + { + closenatpmp(&natpmp_); + } + + [[nodiscard]] constexpr auto renewTime() const noexcept + { + return renew_time_; + } + + tr_port_forwarding pulse(tr_port port, bool is_enabled, tr_port* public_port, tr_port* real_private_port); + +private: + enum tr_natpmp_state + { + TR_NATPMP_IDLE, + TR_NATPMP_ERR, + TR_NATPMP_DISCOVER, + TR_NATPMP_RECV_PUB, + TR_NATPMP_SEND_MAP, + TR_NATPMP_RECV_MAP, + TR_NATPMP_SEND_UNMAP, + TR_NATPMP_RECV_UNMAP + }; + + static constexpr auto LifetimeSecs = uint32_t{ 3600 }; + static constexpr auto CommandWaitSecs = time_t{ 8 }; + + [[nodiscard]] bool canSendCommand() const; + + void setCommandTime(); + + natpmp_t natpmp_; + + tr_port public_port_ = {}; + tr_port private_port_ = {}; + + time_t renew_time_ = 0; + time_t command_time_ = 0; + tr_natpmp_state state_ = TR_NATPMP_DISCOVER; + + bool has_discovered_ = false; + bool is_mapped_ = false; }; - -struct tr_natpmp -{ - bool has_discovered; - bool is_mapped; - - tr_port public_port; - tr_port private_port; - - time_t renew_time; - time_t command_time; - tr_natpmp_state state; - natpmp_t natpmp; -}; - -tr_natpmp* tr_natpmpInit(void); - -void tr_natpmpClose(tr_natpmp*); - -tr_port_forwarding tr_natpmpPulse(tr_natpmp*, tr_port port, bool is_enabled, tr_port* public_port, tr_port* real_private_port); - -/* @} */ diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 76bc9a4db..992117ef5 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -1795,7 +1795,7 @@ tr_peer_stat* tr_peerMgrPeerStats(tr_torrent const* tor, int* setme_count) TR_ASSERT(tor->swarm->manager != nullptr); auto const n = tor->swarm->peerCount(); - auto* const ret = tr_new0(tr_peer_stat, n); + auto* const ret = new tr_peer_stat[n]; auto const now = tr_time(); auto const now_msec = tr_time_msec(); @@ -1826,7 +1826,7 @@ namespace /* does this peer have any pieces that we want? */ [[nodiscard]] bool isPeerInteresting( tr_torrent* const tor, - bool const* const piece_is_interesting, + std::vector const& piece_is_interesting, tr_peerMsgs const* const peer) { /* these cases should have already been handled by the calling code... */ @@ -1990,7 +1990,8 @@ void rechokeDownloads(tr_swarm* s) int const n = tor->pieceCount(); /* build a bitfield of interesting pieces... */ - auto* const piece_is_interesting = tr_new(bool, n); + auto piece_is_interesting = std::vector{}; + piece_is_interesting.resize(n); for (int i = 0; i < n; ++i) { @@ -2034,8 +2035,6 @@ void rechokeDownloads(tr_swarm* s) rechoke.emplace_back(peer, rechoke_state, salter()); } } - - tr_free(piece_is_interesting); } std::sort(std::begin(rechoke), std::end(rechoke)); @@ -2075,12 +2074,22 @@ namespace struct ChokeData { - bool isInterested; - bool wasChoked; - bool isChoked; + ChokeData(tr_peerMsgs* msgs_in, int rate_in, uint8_t salt_in, bool is_interested_in, bool was_choked_in, bool is_choked_in) + : msgs{ msgs_in } + , rate{ rate_in } + , salt{ salt_in } + , is_interested{ is_interested_in } + , was_choked{ was_choked_in } + , is_choked{ is_choked_in } + { + } + + tr_peerMsgs* msgs; int rate; uint8_t salt; - tr_peerMsgs* msgs; + bool is_interested; + bool was_choked; + bool is_choked; [[nodiscard]] constexpr auto compare(ChokeData const& that) const noexcept // <=> { @@ -2089,9 +2098,9 @@ struct ChokeData return this->rate > that.rate ? -1 : 1; } - if (this->wasChoked != that.wasChoked) // prefer unchoked + if (this->was_choked != that.was_choked) // prefer unchoked { - return this->wasChoked ? 1 : -1; + return this->was_choked ? 1 : -1; } if (this->salt != that.salt) // random order @@ -2146,7 +2155,8 @@ void rechokeUploads(tr_swarm* s, uint64_t const now) auto const peerCount = s->peerCount(); auto& peers = s->peers; - auto* const choke = tr_new0(struct ChokeData, peerCount); + auto choked = std::vector{}; + choked.reserve(peerCount); auto const* const session = s->manager->session; bool const chokeAll = !s->tor->clientCanUpload(); bool const isMaxedOut = isBandwidthMaxedOut(s->tor->bandwidth_, now, TR_UP); @@ -2162,8 +2172,6 @@ void rechokeUploads(tr_swarm* s, uint64_t const now) s->optimistic = nullptr; } - int size = 0; - /* sort the peers by preference and rate */ auto salter = tr_salt_shaker{}; for (auto* const peer : peers) @@ -2180,17 +2188,17 @@ void rechokeUploads(tr_swarm* s, uint64_t const now) } else if (peer != s->optimistic) { - struct ChokeData* n = &choke[size++]; - n->msgs = peer; - n->isInterested = peer->is_peer_interested(); - n->wasChoked = peer->is_peer_choked(); - n->rate = getRateBps(s->tor, peer, now); - n->salt = salter(); - n->isChoked = true; + choked.emplace_back( + peer, + getRateBps(s->tor, peer, now), + salter(), + peer->is_peer_interested(), + peer->is_peer_choked(), + true); } } - std::sort(choke, choke + size); + std::sort(std::begin(choked), std::end(choked)); /** * Reciprocation and number of uploads capping is managed by unchoking @@ -2207,57 +2215,57 @@ void rechokeUploads(tr_swarm* s, uint64_t const now) * * If our bandwidth is maxed out, don't unchoke any more peers. */ - int checkedChokeCount = 0; - int unchokedInterested = 0; + auto checked_choke_count = size_t{ 0U }; + auto unchoked_interested = size_t{ 0U }; - for (int i = 0; i < size && unchokedInterested < session->uploadSlotsPerTorrent; ++i) + for (auto& item : choked) { - choke[i].isChoked = isMaxedOut ? choke[i].wasChoked : false; - - ++checkedChokeCount; - - if (choke[i].isInterested) + if (unchoked_interested >= session->upload_slots_per_torrent) { - ++unchokedInterested; + break; + } + + item.is_choked = isMaxedOut ? item.was_choked : false; + + ++checked_choke_count; + + if (item.is_interested) + { + ++unchoked_interested; } } /* optimistic unchoke */ - if (s->optimistic == nullptr && !isMaxedOut && checkedChokeCount < size) + if (s->optimistic == nullptr && !isMaxedOut && checked_choke_count < std::size(choked)) { - auto randPool = std::vector{}; + auto rand_pool = std::vector{}; - for (int i = checkedChokeCount; i < size; ++i) + for (auto i = checked_choke_count, n = std::size(choked); i < n; ++i) { - if (choke[i].isInterested) + if (choked[i].is_interested) { - tr_peerMsgs const* msgs = choke[i].msgs; - int const x = isNew(msgs) ? 3 : 1; + int const x = isNew(choked[i].msgs) ? 3 : 1; for (int y = 0; y < x; ++y) { - randPool.push_back(&choke[i]); + rand_pool.push_back(&choked[i]); } } } - auto const n = std::size(randPool); - if (n != 0) + if (auto const n = std::size(rand_pool); n != 0) { - auto* c = randPool[tr_rand_int_weak(n)]; - c->isChoked = false; + auto* c = rand_pool[tr_rand_int_weak(n)]; + c->is_choked = false; s->optimistic = c->msgs; s->optimistic_unchoke_time_scaler = OptimisticUnchokeMultiplier; } } - for (int i = 0; i < size; ++i) + for (auto& item : choked) { - choke[i].msgs->set_choke(choke[i].isChoked); + item.msgs->set_choke(item.is_choked); } - - /* cleanup */ - tr_free(choke); } } // namespace rechoke_uploads_helpers diff --git a/libtransmission/peer-msgs.cc b/libtransmission/peer-msgs.cc index 7660761b8..b92ef537b 100644 --- a/libtransmission/peer-msgs.cc +++ b/libtransmission/peer-msgs.cc @@ -2225,9 +2225,7 @@ static size_t fillOutputBuffer(tr_peerMsgsImpl* msgs, time_t now) { auto ok = bool{ false }; - auto dataLen = size_t{}; - - if (auto* data = static_cast(tr_torrentGetMetadataPiece(msgs->torrent, piece, &dataLen)); data != nullptr) + if (auto const piece_data = tr_torrentGetMetadataPiece(msgs->torrent, piece); piece_data) { auto* const out = msgs->outMessages; @@ -2240,17 +2238,16 @@ static size_t fillOutputBuffer(tr_peerMsgsImpl* msgs, time_t now) evbuffer* const payload = tr_variantToBuf(&tmp, TR_VARIANT_FMT_BENC); /* write it out as a LTEP message to our outMessages buffer */ - evbuffer_add_uint32(out, 2 * sizeof(uint8_t) + evbuffer_get_length(payload) + dataLen); + evbuffer_add_uint32(out, 2 * sizeof(uint8_t) + evbuffer_get_length(payload) + std::size(*piece_data)); evbuffer_add_uint8(out, BtPeerMsgs::Ltep); evbuffer_add_uint8(out, msgs->ut_metadata_id); evbuffer_add_buffer(out, payload); - evbuffer_add(out, data, dataLen); + evbuffer_add(out, std::data(*piece_data), std::size(*piece_data)); msgs->pokeBatchPeriod(HighPriorityIntervalSecs); msgs->dbgOutMessageLen(); evbuffer_free(payload); tr_variantFree(&tmp); - tr_free(data); ok = true; } diff --git a/libtransmission/port-forwarding.cc b/libtransmission/port-forwarding.cc index a128014ea..00412e7de 100644 --- a/libtransmission/port-forwarding.cc +++ b/libtransmission/port-forwarding.cc @@ -24,12 +24,12 @@ using namespace std::literals; struct tr_shared { - tr_shared(tr_session* session_in) + tr_shared(tr_session& session_in) : session{ session_in } { } - tr_session* const session = nullptr; + tr_session& session; bool isEnabled = false; bool isShuttingDown = false; @@ -39,7 +39,7 @@ struct tr_shared tr_port_forwarding upnpStatus = TR_PORT_UNMAPPED; tr_upnp* upnp = nullptr; - tr_natpmp* natpmp = nullptr; + std::unique_ptr natpmp; std::unique_ptr timer; }; @@ -71,13 +71,13 @@ static char const* getNatStateStr(int state) static void natPulse(tr_shared* s, bool do_check) { - auto* session = s->session; - tr_port const private_peer_port = session->private_peer_port; + auto& session = s->session; + tr_port const private_peer_port = session.private_peer_port; bool const is_enabled = s->isEnabled && !s->isShuttingDown; - if (s->natpmp == nullptr) + if (!s->natpmp) { - s->natpmp = tr_natpmpInit(); + s->natpmp = std::make_unique(); } if (s->upnp == nullptr) @@ -89,19 +89,19 @@ static void natPulse(tr_shared* s, bool do_check) auto public_peer_port = tr_port{}; auto received_private_port = tr_port{}; - s->natpmpStatus = tr_natpmpPulse(s->natpmp, private_peer_port, is_enabled, &public_peer_port, &received_private_port); + s->natpmpStatus = s->natpmp->pulse(private_peer_port, is_enabled, &public_peer_port, &received_private_port); if (s->natpmpStatus == TR_PORT_MAPPED) { - session->public_peer_port = public_peer_port; - session->private_peer_port = received_private_port; + session.public_peer_port = public_peer_port; + session.private_peer_port = received_private_port; tr_logAddInfo(fmt::format( _("Mapped private port {private_port} to public port {public_port}"), - fmt::arg("public_port", session->public_peer_port.host()), - fmt::arg("private_port", session->private_peer_port.host()))); + fmt::arg("public_port", session.public_peer_port.host()), + fmt::arg("private_port", session.private_peer_port.host()))); } - s->upnpStatus = tr_upnpPulse(s->upnp, private_peer_port, is_enabled, do_check, session->bind_ipv4.readable()); + s->upnpStatus = tr_upnpPulse(s->upnp, private_peer_port, is_enabled, do_check, session.bind_ipv4.readable()); auto const new_status = tr_sharedTraversalStatus(s); @@ -129,9 +129,9 @@ static void restartTimer(tr_shared* s) // if we're mapped, everything is fine... check back at `renew_time` // to renew the port forwarding if it's expired s->doPortCheck = true; - if (auto const now = tr_time(); s->natpmp->renew_time > now) + if (auto const now = tr_time(); s->natpmp->renewTime() > now) { - timer->startSingleShot(std::chrono::seconds{ s->natpmp->renew_time - now }); + timer->startSingleShot(std::chrono::seconds{ s->natpmp->renewTime() - now }); } else // ??? { @@ -170,7 +170,7 @@ static void onTimer(void* vshared) **** ***/ -tr_shared* tr_sharedInit(tr_session* session) +tr_shared* tr_sharedInit(tr_session& session) { return new tr_shared{ session }; } @@ -185,8 +185,7 @@ static void stop_forwarding(tr_shared* s) tr_logAddTrace("stopped"); natPulse(s, false); - tr_natpmpClose(s->natpmp); - s->natpmp = nullptr; + s->natpmp.reset(); s->natpmpStatus = TR_PORT_UNMAPPED; tr_upnpClose(s->upnp); @@ -196,19 +195,19 @@ static void stop_forwarding(tr_shared* s) stop_timer(s); } -void tr_sharedClose(tr_session* session) +void tr_sharedClose(tr_session& session) { - tr_shared* shared = session->shared; + tr_shared* shared = session.shared; shared->isShuttingDown = true; stop_forwarding(shared); - shared->session->shared = nullptr; + shared->session.shared = nullptr; delete shared; } static void start_timer(tr_shared* s) { - s->timer = s->session->timerMaker().create(onTimer, s); + s->timer = s->session.timerMaker().create(onTimer, s); restartTimer(s); } @@ -226,9 +225,9 @@ void tr_sharedTraversalEnable(tr_shared* s, bool is_enable) } } -void tr_sharedPortChanged(tr_session* session) +void tr_sharedPortChanged(tr_session& session) { - tr_shared* s = session->shared; + auto* const s = session.shared; if (s->isEnabled) { diff --git a/libtransmission/port-forwarding.h b/libtransmission/port-forwarding.h index 615d43de6..6e13dc6c8 100644 --- a/libtransmission/port-forwarding.h +++ b/libtransmission/port-forwarding.h @@ -15,11 +15,11 @@ struct tr_bindsockets; struct tr_session; struct tr_shared; -tr_shared* tr_sharedInit(tr_session*); +tr_shared* tr_sharedInit(tr_session&); -void tr_sharedClose(tr_session*); +void tr_sharedClose(tr_session&); -void tr_sharedPortChanged(tr_session*); +void tr_sharedPortChanged(tr_session&); void tr_sharedTraversalEnable(tr_shared*, bool is_enabled); diff --git a/libtransmission/session.cc b/libtransmission/session.cc index e404e6ac1..8db965a83 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -458,7 +458,7 @@ void tr_sessionGetSettings(tr_session const* s, tr_variant* setme_dictionary) tr_variantDictAddInt(d, TR_KEY_speed_limit_up, tr_sessionGetSpeedLimit_KBps(s, TR_UP)); tr_variantDictAddBool(d, TR_KEY_speed_limit_up_enabled, tr_sessionIsSpeedLimited(s, TR_UP)); tr_variantDictAddStr(d, TR_KEY_umask, fmt::format("{:#o}", s->umask)); - tr_variantDictAddInt(d, TR_KEY_upload_slots_per_torrent, s->uploadSlotsPerTorrent); + tr_variantDictAddInt(d, TR_KEY_upload_slots_per_torrent, s->upload_slots_per_torrent); tr_variantDictAddStr(d, TR_KEY_bind_address_ipv4, s->bind_ipv4.readable()); tr_variantDictAddStr(d, TR_KEY_bind_address_ipv6, s->bind_ipv6.readable()); tr_variantDictAddBool(d, TR_KEY_start_added_torrents, !tr_sessionGetPaused(s)); @@ -722,7 +722,7 @@ static void tr_sessionInitImpl(init_data* data) session->peerMgr = tr_peerMgrNew(session); - session->shared = tr_sharedInit(session); + session->shared = tr_sharedInit(*session); /** *** Blocklist @@ -1016,7 +1016,7 @@ static void sessionSetImpl(struct init_data* const data) if (tr_variantDictFindInt(settings, TR_KEY_upload_slots_per_torrent, &i)) { - session->uploadSlotsPerTorrent = i; + session->upload_slots_per_torrent = i; } if (tr_variantDictFindInt(settings, TR_KEY_speed_limit_up, &i)) @@ -1254,7 +1254,7 @@ static void peerPortChanged(tr_session* const session) close_incoming_peer_port(session); open_incoming_peer_port(session); - tr_sharedPortChanged(session); + tr_sharedPortChanged(*session); for (auto* const tor : session->torrents()) { @@ -1855,7 +1855,7 @@ static void sessionCloseImplStart(tr_session* session) session->now_timer_.reset(); tr_verifyClose(session); - tr_sharedClose(session); + tr_sharedClose(*session); close_incoming_peer_port(session); session->rpc_server_.reset(); diff --git a/libtransmission/session.h b/libtransmission/session.h index 977290eec..d1d0af4b1 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -507,7 +507,7 @@ public: uint16_t peerLimit = 200; uint16_t peerLimitPerTorrent = 50; - int uploadSlotsPerTorrent = 0; + uint16_t upload_slots_per_torrent = 0; /* The UDP sockets used for the DHT and uTP. */ tr_port udp_port; diff --git a/libtransmission/torrent-magnet.cc b/libtransmission/torrent-magnet.cc index 1d0322fc6..f7df47519 100644 --- a/libtransmission/torrent-magnet.cc +++ b/libtransmission/torrent-magnet.cc @@ -107,50 +107,45 @@ bool tr_torrentSetMetadataSizeHint(tr_torrent* tor, int64_t size) return true; } -void* tr_torrentGetMetadataPiece(tr_torrent const* tor, int piece, size_t* len) +std::optional> tr_torrentGetMetadataPiece(tr_torrent const* tor, int piece) { TR_ASSERT(tr_isTorrent(tor)); TR_ASSERT(piece >= 0); - TR_ASSERT(len != nullptr); if (!tor->hasMetainfo()) { - return nullptr; + return {}; } auto const fd = tr_sys_file_open(tor->torrentFile(), TR_SYS_FILE_READ, 0); if (fd == TR_BAD_SYS_FILE) { - return nullptr; + return {}; } auto const info_dict_size = tor->infoDictSize(); TR_ASSERT(info_dict_size > 0); - char* ret = nullptr; if (size_t const o = piece * METADATA_PIECE_SIZE; tr_sys_file_seek(fd, tor->infoDictOffset() + o, TR_SEEK_SET, nullptr)) { - size_t const l = o + METADATA_PIECE_SIZE <= info_dict_size ? METADATA_PIECE_SIZE : info_dict_size - o; + size_t const piece_len = o + METADATA_PIECE_SIZE <= info_dict_size ? METADATA_PIECE_SIZE : info_dict_size - o; - if (0 < l && l <= METADATA_PIECE_SIZE) + if (piece_len <= METADATA_PIECE_SIZE) { - auto* buf = tr_new(char, l); + auto buf = std::vector{}; + buf.resize(piece_len); - if (auto n = uint64_t{}; tr_sys_file_read(fd, buf, l, &n) && n == l) + if (auto n_read = uint64_t{}; + tr_sys_file_read(fd, std::data(buf), std::size(buf), &n_read) && n_read == std::size(buf)) { - *len = l; - ret = buf; - buf = nullptr; + tr_sys_file_close(fd); + return buf; } - - tr_free(buf); } } tr_sys_file_close(fd); - - TR_ASSERT(ret == nullptr || *len > 0); - return ret; + return {}; } static int getPieceLength(struct tr_incomplete_metadata const* m, int piece) diff --git a/libtransmission/torrent-magnet.h b/libtransmission/torrent-magnet.h index 7523336cf..bdfae9f98 100644 --- a/libtransmission/torrent-magnet.h +++ b/libtransmission/torrent-magnet.h @@ -9,9 +9,11 @@ #error only libtransmission should #include this header. #endif -#include // int64_t #include // size_t +#include // int64_t #include // time_t +#include +#include #include "transmission.h" @@ -21,7 +23,7 @@ struct tr_torrent_metainfo; // defined by BEP #9 inline constexpr int METADATA_PIECE_SIZE = 1024 * 16; -void* tr_torrentGetMetadataPiece(tr_torrent const* tor, int piece, size_t* len); +std::optional> tr_torrentGetMetadataPiece(tr_torrent const* tor, int piece); void tr_torrentSetMetadataPiece(tr_torrent* tor, int piece, void const* data, int len); diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 413b9ceac..576aa78c4 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -1264,7 +1264,7 @@ tr_peer_stat* tr_torrentPeers(tr_torrent const* tor, int* peerCount) void tr_torrentPeersFree(tr_peer_stat* peers, int /*peerCount*/) { - tr_free(peers); + delete[] peers; } void tr_torrentAvailability(tr_torrent const* tor, int8_t* tab, int size) diff --git a/macosx/MessageWindowController.mm b/macosx/MessageWindowController.mm index baa2bc8a5..9d89c392f 100644 --- a/macosx/MessageWindowController.mm +++ b/macosx/MessageWindowController.mm @@ -252,12 +252,13 @@ for (tr_log_message* currentMessage = messages; currentMessage != NULL; currentMessage = currentMessage->next) { - NSString* name = currentMessage->name != NULL ? @(currentMessage->name) : NSProcessInfo.processInfo.processName; + NSString* name = !std::empty(currentMessage->name) ? @(currentMessage->name.c_str()) : NSProcessInfo.processInfo.processName; - NSString* file = [(@(currentMessage->file)).lastPathComponent stringByAppendingFormat:@":%d", currentMessage->line]; + auto const file_string = std::string{ currentMessage->file }; + NSString* file = [(@(file_string.c_str())).lastPathComponent stringByAppendingFormat:@":%d", currentMessage->line]; NSDictionary* message = @{ - @"Message" : @(currentMessage->message), + @"Message" : @(currentMessage->message.c_str()), @"Date" : [NSDate dateWithTimeIntervalSince1970:currentMessage->when], @"Index" : @(currentIndex++), //more accurate when sorting by date @"Level" : @(currentMessage->level),