From 5df6e4f88f0c6e89042c10b0cd9da2bd31fdb3c1 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 14 Oct 2021 09:22:28 -0500 Subject: [PATCH] fix: uninitialized variables in announcer (#1936) --- libtransmission/announcer-http.cc | 64 ++++++++++------------ libtransmission/announcer-udp.cc | 89 +++++++++--------------------- libtransmission/announcer.cc | 91 +++++++++++++------------------ 3 files changed, 93 insertions(+), 151 deletions(-) diff --git a/libtransmission/announcer-http.cc b/libtransmission/announcer-http.cc index 15de9d6a7..d53066d87 100644 --- a/libtransmission/announcer-http.cc +++ b/libtransmission/announcer-http.cc @@ -41,9 +41,7 @@ static char const* get_event_string(tr_announce_request const* req) static char* announce_url_new(tr_session const* session, tr_announce_request const* req) { - char const* str; - unsigned char const* ipv6; - struct evbuffer* buf = evbuffer_new(); + evbuffer* const buf = evbuffer_new(); char escaped_info_hash[SHA_DIGEST_LENGTH * 3 + 1]; tr_http_escape_sha1(escaped_info_hash, req->info_hash); @@ -85,15 +83,13 @@ static char* announce_url_new(tr_session const* session, tr_announce_request con evbuffer_add_printf(buf, "&corrupt=%" PRIu64, req->corrupt); } - str = get_event_string(req); - + char const* str = get_event_string(req); if (!tr_str_is_empty(str)) { evbuffer_add_printf(buf, "&event=%s", str); } str = req->tracker_id_str; - if (!tr_str_is_empty(str)) { evbuffer_add_printf(buf, "&trackerid=%s", str); @@ -108,8 +104,7 @@ static char* announce_url_new(tr_session const* session, tr_announce_request con announce twice. At any rate, we're already computing our IPv6 address (for the LTEP handshake), so this comes for free. */ - ipv6 = tr_globalIPv6(); - + unsigned char const* const ipv6 = tr_globalIPv6(); if (ipv6 != nullptr) { char ipv6_readable[INET6_ADDRSTRLEN]; @@ -129,26 +124,26 @@ static tr_pex* listToPex(tr_variant* peerList, size_t* setme_len) for (size_t i = 0; i < len; ++i) { - int64_t port; - char const* ip; - tr_address addr; - tr_variant* peer = tr_variantListChild(peerList, i); + tr_variant* const peer = tr_variantListChild(peerList, i); if (peer == nullptr) { continue; } + char const* ip = nullptr; if (!tr_variantDictFindStr(peer, TR_KEY_ip, &ip, nullptr)) { continue; } + auto addr = tr_address{}; if (!tr_address_from_string(&addr, ip)) { continue; } + auto port = int64_t{}; if (!tr_variantDictFindInt(peer, TR_KEY_port, &port)) { continue; @@ -207,10 +202,9 @@ static void on_announce_done( size_t msglen, void* vdata) { - tr_announce_response* response; auto* data = static_cast(vdata); - response = &data->response; + tr_announce_response* const response = &data->response; response->did_connect = did_connect; response->did_timeout = did_timeout; dbgmsg(data->log_name, "Got announce response"); @@ -234,7 +228,7 @@ static void on_announce_done( } else { - size_t len; + auto len = size_t{}; char* str = tr_variantToStr(&benc, TR_VARIANT_FMT_JSON, &len); fprintf(stderr, "%s", "Announce response:\n< "); @@ -250,11 +244,11 @@ static void on_announce_done( if (variant_loaded && tr_variantIsDict(&benc)) { - int64_t i; - size_t len; - tr_variant* tmp; - char const* str; - uint8_t const* raw; + auto i = int64_t{}; + auto len = size_t{}; + tr_variant* tmp = nullptr; + char const* str = nullptr; + uint8_t const* raw = nullptr; if (tr_variantDictFindStr(&benc, TR_KEY_failure_reason, &str, &len)) { @@ -329,10 +323,9 @@ void tr_tracker_http_announce( tr_announce_response_func response_func, void* response_func_user_data) { - struct announce_data* d; - char* url = announce_url_new(session, request); + char* const url = announce_url_new(session, request); - d = tr_new0(struct announce_data, 1); + auto* const d = tr_new0(announce_data, 1); d->response.seeders = -1; d->response.leechers = -1; d->response.downloads = -1; @@ -397,11 +390,8 @@ static void on_scrape_done( } else { - tr_variant top; - int64_t intVal; - tr_variant* files; - tr_variant* flags; - bool const variant_loaded = tr_variantFromBenc(&top, msg, msglen) == 0; + auto top = tr_variant{}; + auto const variant_loaded = tr_variantFromBenc(&top, msg, msglen) == 0; if (tr_env_key_exists("TR_CURL_VERBOSE")) { @@ -411,7 +401,7 @@ static void on_scrape_done( } else { - size_t len; + auto len = size_t{}; char* str = tr_variantToStr(&top, TR_VARIANT_FMT_JSON, &len); fprintf(stderr, "%s", "Scrape response:\n< "); @@ -427,23 +417,26 @@ static void on_scrape_done( if (variant_loaded) { - size_t len; - char const* str; + auto len = size_t{}; + char const* str = nullptr; if (tr_variantDictFindStr(&top, TR_KEY_failure_reason, &str, &len)) { response->errmsg = tr_strndup(str, len); } + tr_variant* flags = nullptr; + auto intVal = int64_t{}; if (tr_variantDictFindDict(&top, TR_KEY_flags, &flags) && tr_variantDictFindInt(flags, TR_KEY_min_request_interval, &intVal)) { response->min_request_interval = intVal; } + tr_variant* files = nullptr; if (tr_variantDictFindDict(&top, TR_KEY_files, &files)) { - tr_quark key; - tr_variant* val; + auto key = tr_quark{}; + tr_variant* val = nullptr; for (int i = 0; tr_variantDictChild(files, i, &key, &val); ++i) { @@ -489,11 +482,10 @@ static void on_scrape_done( static char* scrape_url_new(tr_scrape_request const* req) { - char delimiter; - struct evbuffer* buf = evbuffer_new(); + struct evbuffer* const buf = evbuffer_new(); evbuffer_add_printf(buf, "%s", req->url); - delimiter = strchr(req->url, '?') != nullptr ? '&' : '?'; + char delimiter = strchr(req->url, '?') != nullptr ? '&' : '?'; for (int i = 0; i < req->info_hash_count; ++i) { diff --git a/libtransmission/announcer-udp.cc b/libtransmission/announcer-udp.cc index f07b21956..91991f395 100644 --- a/libtransmission/announcer-udp.cc +++ b/libtransmission/announcer-udp.cc @@ -48,7 +48,7 @@ static void tau_sockaddr_setport(struct sockaddr* sa, tr_port port) static int tau_sendto(tr_session const* session, struct evutil_addrinfo* ai, tr_port port, void const* buf, size_t buflen) { - tr_socket_t sockfd; + auto sockfd = tr_socket_t{}; if (ai->ai_addr->sa_family == AF_INET) { @@ -79,14 +79,14 @@ static int tau_sendto(tr_session const* session, struct evutil_addrinfo* ai, tr_ static uint32_t evbuffer_read_ntoh_32(struct evbuffer* buf) { - uint32_t val; + auto val = uint32_t{}; evbuffer_remove(buf, &val, sizeof(uint32_t)); return ntohl(val); } static uint64_t evbuffer_read_ntoh_64(struct evbuffer* buf) { - uint64_t val; + auto val = uint64_t{}; evbuffer_remove(buf, &val, sizeof(uint64_t)); return tr_ntohll(val); } @@ -106,7 +106,7 @@ using tau_transaction_t = uint32_t; static tau_transaction_t tau_transaction_new(void) { - tau_transaction_t tmp; + auto tmp = tau_transaction_t{}; tr_rand_buffer(&tmp, sizeof(tau_transaction_t)); return tmp; } @@ -242,35 +242,24 @@ static void on_scrape_response(struct tau_scrape_request* request, tau_action_t { for (int i = 0; i < request->response.row_count; ++i) { - struct tr_scrape_response_row* row; - if (evbuffer_get_length(buf) < sizeof(uint32_t) * 3) { break; } - row = &request->response.rows[i]; - row->seeders = evbuffer_read_ntoh_32(buf); - row->downloads = evbuffer_read_ntoh_32(buf); - row->leechers = evbuffer_read_ntoh_32(buf); + struct tr_scrape_response_row& row = request->response.rows[i]; + row.seeders = evbuffer_read_ntoh_32(buf); + row.downloads = evbuffer_read_ntoh_32(buf); + row.leechers = evbuffer_read_ntoh_32(buf); } tau_scrape_request_finished(request); } else { - char* errmsg; size_t const buflen = evbuffer_get_length(buf); - - if (action == TAU_ACTION_ERROR && buflen > 0) - { - errmsg = tr_strndup(evbuffer_pullup(buf, -1), buflen); - } - else - { - errmsg = tr_strdup(_("Unknown error")); - } - + char* const errmsg = action == TAU_ACTION_ERROR && buflen > 0 ? tr_strndup(evbuffer_pullup(buf, -1), buflen) : + tr_strdup(_("Unknown error")); tau_scrape_request_fail(request, true, false, errmsg); tr_free(errmsg); } @@ -415,17 +404,8 @@ static void on_announce_response(struct tau_announce_request* request, tau_actio } else { - char* errmsg; - - if (action == TAU_ACTION_ERROR && buflen > 0) - { - errmsg = tr_strndup(evbuffer_pullup(buf, -1), buflen); - } - else - { - errmsg = tr_strdup(_("Unknown error")); - } - + char* const errmsg = action == TAU_ACTION_ERROR && buflen > 0 ? tr_strndup(evbuffer_pullup(buf, -1), buflen) : + tr_strdup(_("Unknown error")); tau_announce_request_fail(request, true, false, errmsg); tr_free(errmsg); } @@ -613,17 +593,10 @@ static void on_tracker_connection_response(struct tau_tracker* tracker, tau_acti } else { - char* errmsg; size_t const buflen = buf != nullptr ? evbuffer_get_length(buf) : 0; - if (action == TAU_ACTION_ERROR && buflen > 0) - { - errmsg = tr_strndup(evbuffer_pullup(buf, -1), buflen); - } - else - { - errmsg = tr_strdup(_("Connection failed")); - } + char* const errmsg = action == TAU_ACTION_ERROR && buflen > 0 ? tr_strndup(evbuffer_pullup(buf, -1), buflen) : + tr_strdup(_("Connection failed")); dbgmsg(tracker->key, "%s", errmsg); tau_tracker_fail_all(tracker, true, false, errmsg); @@ -635,7 +608,6 @@ static void on_tracker_connection_response(struct tau_tracker* tracker, tau_acti static void tau_tracker_timeout_reqs(struct tau_tracker* tracker) { - tr_ptrArray* reqs; time_t const now = time(nullptr); bool const cancel_all = tracker->close_at != 0 && (tracker->close_at <= now); @@ -644,7 +616,7 @@ static void tau_tracker_timeout_reqs(struct tau_tracker* tracker) on_tracker_connection_response(tracker, TAU_ACTION_ERROR, nullptr); } - reqs = &tracker->announces; + tr_ptrArray* reqs = &tracker->announces; for (int i = 0, n = tr_ptrArraySize(reqs); i < n; ++i) { @@ -665,7 +637,7 @@ static void tau_tracker_timeout_reqs(struct tau_tracker* tracker) for (int i = 0, n = tr_ptrArraySize(reqs); i < n; ++i) { - auto* req = static_cast(tr_ptrArrayNth(reqs, i)); + auto* const req = static_cast(tr_ptrArrayNth(reqs, i)); if (cancel_all || req->created_at + TAU_REQUEST_TTL < now) { @@ -778,14 +750,12 @@ struct tr_announcer_udp static struct tr_announcer_udp* announcer_udp_get(tr_session* session) { - struct tr_announcer_udp* tau; - if (session->announcer_udp != nullptr) { return session->announcer_udp; } - tau = tr_new0(struct tr_announcer_udp, 1); + auto* const tau = tr_new0(tr_announcer_udp, 1); tau->trackers = {}; tau->session = session; session->announcer_udp = tau; @@ -796,15 +766,13 @@ static struct tr_announcer_udp* announcer_udp_get(tr_session* session) If it doesn't exist yet, create one. */ static struct tau_tracker* tau_session_get_tracker(struct tr_announcer_udp* tau, char const* url) { - int port; - char* host; - char* key; - struct tau_tracker* tracker = nullptr; - /* see if we've already got a tracker that matches this host + port */ + auto port = int{}; + char* host = nullptr; tr_urlParse(url, TR_BAD_SIZE, nullptr, &host, &port, nullptr); - key = tr_strdup_printf("%s:%d", host, port); + char* const key = tr_strdup_printf("%s:%d", host, port); + tau_tracker* tracker = nullptr; for (int i = 0, n = tr_ptrArraySize(&tau->trackers); tracker == nullptr && i < n; ++i) { auto* tmp = static_cast(tr_ptrArrayNth(&tau->trackers, i)); @@ -908,10 +876,6 @@ void tr_tracker_udp_start_shutdown(tr_session* session) * @return true if msg was a tracker response; false otherwise */ bool tau_handle_message(tr_session* session, uint8_t const* msg, size_t msglen) { - struct tr_announcer_udp* tau; - tau_transaction_t transaction_id; - struct evbuffer* buf; - if (session == nullptr || session->announcer_udp == nullptr) { return false; @@ -923,7 +887,7 @@ bool tau_handle_message(tr_session* session, uint8_t const* msg, size_t msglen) } /* extract the action_id and see if it makes sense */ - buf = evbuffer_new(); + struct evbuffer* const buf = evbuffer_new(); evbuffer_add_reference(buf, msg, msglen, nullptr, nullptr); auto const action_id = tau_action_t(evbuffer_read_ntoh_32(buf)); @@ -934,12 +898,11 @@ bool tau_handle_message(tr_session* session, uint8_t const* msg, size_t msglen) } /* extract the transaction_id and look for a match */ - tau = session->announcer_udp; - transaction_id = evbuffer_read_ntoh_32(buf); + struct tr_announcer_udp* const tau = session->announcer_udp; + tau_transaction_t const transaction_id = evbuffer_read_ntoh_32(buf); for (int i = 0, n = tr_ptrArraySize(&tau->trackers); i < n; ++i) { - tr_ptrArray* reqs; auto* tracker = static_cast(tr_ptrArrayNth(&tau->trackers, i)); /* is it a connection response? */ @@ -952,7 +915,7 @@ bool tau_handle_message(tr_session* session, uint8_t const* msg, size_t msglen) } /* is it a response to one of this tracker's announces? */ - reqs = &tracker->announces; + tr_ptrArray* reqs = &tracker->announces; for (int j = 0, jn = tr_ptrArraySize(reqs); j < jn; ++j) { @@ -974,7 +937,7 @@ bool tau_handle_message(tr_session* session, uint8_t const* msg, size_t msglen) for (int j = 0, jn = tr_ptrArraySize(reqs); j < jn; ++j) { - auto* req = static_cast(tr_ptrArrayNth(reqs, j)); + auto* const req = static_cast(tr_ptrArrayNth(reqs, j)); if (req->sent_at != 0 && transaction_id == req->transaction_id) { diff --git a/libtransmission/announcer.cc b/libtransmission/announcer.cc index f730a19bd..eadc86213 100644 --- a/libtransmission/announcer.cc +++ b/libtransmission/announcer.cc @@ -228,13 +228,16 @@ struct tr_tracker /* format: host+':'+ port */ static char* getKey(char const* url) { - char* ret; char* scheme = nullptr; char* host = nullptr; int port = 0; tr_urlParse(url, TR_BAD_SIZE, &scheme, &host, &port, nullptr); - ret = tr_strdup_printf("%s://%s:%d", scheme != nullptr ? scheme : "invalid", host != nullptr ? host : "invalid", port); + char* const ret = tr_strdup_printf( + "%s://%s:%d", + scheme != nullptr ? scheme : "invalid", + host != nullptr ? host : "invalid", + port); tr_free(host); tr_free(scheme); @@ -318,25 +321,20 @@ struct tr_tier static time_t get_next_scrape_time(tr_session const* session, tr_tier const* tier, int interval) { - time_t ret; - time_t const now = tr_time(); - /* Maybe don't scrape paused torrents */ if (!tier->isRunning && !session->scrapePausedTorrents) { - ret = 0; + return 0; } + /* Add the interval, and then increment to the nearest 10th second. * The latter step is to increase the odds of several torrents coming * due at the same time to improve multiscrape. */ - else + time_t const now = tr_time(); + time_t ret = now + interval; + while (ret % 10 != 0) { - ret = now + interval; - - while (ret % 10 != 0) - { - ++ret; - } + ++ret; } return ret; @@ -566,7 +564,6 @@ static int filter_trackers_compare_func(void const* va, void const* vb) static tr_tracker_info* filter_trackers(tr_tracker_info const* input, int input_count, int* setme_count) { int n = 0; - struct tr_tracker_info* ret; struct ann_tracker_info* tmp = tr_new0(struct ann_tracker_info, input_count); /* build a list of valid trackers */ @@ -574,17 +571,17 @@ static tr_tracker_info* filter_trackers(tr_tracker_info const* input, int input_ { if (tr_urlIsValidTracker(input[i].announce)) { - int port; - char* scheme; - char* host; - char* path; - bool is_duplicate = false; + int port = 0; + char* scheme = nullptr; + char* host = nullptr; + char* path = nullptr; tr_urlParse(input[i].announce, TR_BAD_SIZE, &scheme, &host, &port, &path); /* weed out one common source of duplicates: * "http://tracker/announce" + * "http://tracker:80/announce" */ + bool is_duplicate = false; for (int j = 0; !is_duplicate && j < n; ++j) { is_duplicate = tmp[j].port == port && strcmp(tmp[j].scheme, scheme) == 0 && strcmp(tmp[j].host, host) == 0 && @@ -630,7 +627,7 @@ static tr_tracker_info* filter_trackers(tr_tracker_info const* input, int input_ /* build the output */ *setme_count = n; - ret = tr_new0(tr_tracker_info, n); + struct tr_tracker_info* const ret = tr_new0(tr_tracker_info, n); for (int i = 0; i < n; ++i) { @@ -652,9 +649,7 @@ static tr_tracker_info* filter_trackers(tr_tracker_info const* input, int input_ static void addTorrentToTier(tr_torrent_tiers* tt, tr_torrent* tor) { - int n; - int tier_count; - tr_tier* tier; + auto n = int{}; tr_tracker_info* infos = filter_trackers(tor->info.trackers, tor->info.trackerCount, &n); /* build the array of trackers */ @@ -667,8 +662,7 @@ static void addTorrentToTier(tr_torrent_tiers* tt, tr_torrent* tor) } /* count how many tiers there are */ - tier_count = 0; - + auto tier_count = int{}; for (int i = 0; i < n; ++i) { if (i == 0 || infos[i].tier != infos[i - 1].tier) @@ -678,7 +672,7 @@ static void addTorrentToTier(tr_torrent_tiers* tt, tr_torrent* tor) } /* build the array of tiers */ - tier = nullptr; + tr_tier* tier = nullptr; tt->tiers = tr_new0(tr_tier, tier_count); tt->tier_count = 0; @@ -770,7 +764,6 @@ static void dbgmsg_tier_announce_queue(tr_tier const* tier) if (tr_logGetDeepEnabled()) { char name[128]; - char* message; struct evbuffer* buf = evbuffer_new(); tier_build_log_name(tier, name, sizeof(name)); @@ -782,7 +775,7 @@ static void dbgmsg_tier_announce_queue(tr_tier const* tier) evbuffer_add_printf(buf, "[%d:%s]", i, str); } - message = evbuffer_free_to_str(buf, nullptr); + char* const message = evbuffer_free_to_str(buf, nullptr); tr_logAddDeep(__FILE__, __LINE__, name, "announce queue is %s", message); tr_free(message); } @@ -1029,8 +1022,6 @@ struct announce_data static void on_announce_error(tr_tier* tier, char const* err, tr_announce_event e) { - int interval; - /* increment the error count */ if (tier->currentTracker != nullptr) { @@ -1046,7 +1037,7 @@ static void on_announce_error(tr_tier* tier, char const* err, tr_announce_event tierIncrementTracker(tier); /* schedule a reannounce */ - interval = getRetryInterval(tier->currentTracker); + int const interval = getRetryInterval(tier->currentTracker); dbgmsg(tier, "Retrying announce in %d seconds.", interval); tr_logAddTorInfo(tier->tor, "Retrying announce in %d seconds.", interval); tier_announce_event_push(tier, e, tr_time() + interval); @@ -1062,8 +1053,6 @@ static void on_announce_done(tr_announce_response const* response, void* vdata) if (tier != nullptr) { - tr_tracker* tracker; - dbgmsg( tier, "Got announce response: " @@ -1121,16 +1110,15 @@ static void on_announce_done(tr_announce_response const* response, void* vdata) } else { - int i; - char const* str; - int scrape_fields = 0; - int seeders = 0; - int leechers = 0; - bool const isStopped = event == TR_ANNOUNCE_EVENT_STOPPED; + auto const isStopped = event == TR_ANNOUNCE_EVENT_STOPPED; + auto leechers = int{}; + auto scrape_fields = int{}; + auto seeders = int{}; publishErrorClear(tier); - if ((tracker = tier->currentTracker) != nullptr) + tr_tracker* const tracker = tier->currentTracker; + if (tracker != nullptr) { tracker->consecutiveFailures = 0; @@ -1152,14 +1140,15 @@ static void on_announce_done(tr_announce_response const* response, void* vdata) ++scrape_fields; } - if ((str = response->tracker_id_str) != nullptr) + if (response->tracker_id_str != nullptr) { tr_free(tracker->tracker_id_str); - tracker->tracker_id_str = tr_strdup(str); + tracker->tracker_id_str = tr_strdup(response->tracker_id_str); } } - if ((str = response->warning) != nullptr) + char const* const str = response->warning; + if (str != nullptr) { tr_strlcpy(tier->lastAnnounceStr, str, sizeof(tier->lastAnnounceStr)); dbgmsg(tier, "tracker gave \"%s\"", str); @@ -1170,14 +1159,14 @@ static void on_announce_done(tr_announce_response const* response, void* vdata) tr_strlcpy(tier->lastAnnounceStr, _("Success"), sizeof(tier->lastAnnounceStr)); } - if ((i = response->min_interval) != 0) + if (response->min_interval != 0) { - tier->announceMinIntervalSec = i; + tier->announceMinIntervalSec = response->min_interval; } - if ((i = response->interval) != 0) + if (response->interval != 0) { - tier->announceIntervalSec = i; + tier->announceIntervalSec = response->interval; } if (response->pex_count > 0) @@ -1228,7 +1217,7 @@ static void on_announce_done(tr_announce_response const* response, void* vdata) if (!isStopped && tier->announce_event_count == 0) { /* the queue is empty, so enqueue a perodic update */ - i = tier->announceIntervalSec; + int const i = tier->announceIntervalSec; dbgmsg(tier, "Sending periodic reannounce in %d seconds", i); tier_announce_event_push(tier, TR_ANNOUNCE_EVENT_NONE, now + i); } @@ -1337,8 +1326,6 @@ static bool multiscrape_too_big(char const* errmsg) static void on_scrape_error(tr_session const* session, tr_tier* tier, char const* errmsg) { - int interval; - /* increment the error count */ if (tier->currentTracker != nullptr) { @@ -1354,7 +1341,7 @@ static void on_scrape_error(tr_session const* session, tr_tier* tier, char const tierIncrementTracker(tier); /* schedule a rescrape */ - interval = getRetryInterval(tier->currentTracker); + int const interval = getRetryInterval(tier->currentTracker); dbgmsg(tier, "Retrying scrape in %zu seconds.", (size_t)interval); tr_logAddTorInfo(tier->tor, "Retrying scrape in %zu seconds.", (size_t)interval); tier->lastScrapeSucceeded = false; @@ -1490,7 +1477,7 @@ static void on_scrape_done(tr_scrape_response const* response, void* vsession) { char* scheme = nullptr; char* host = nullptr; - int port; + auto port = int{}; if (tr_urlParse(std::data(url), std::size(url), &scheme, &host, &port, nullptr)) { /* don't log the full URL, since that might have a personal announce id */