From 2b65b985cf8b4f90725e5f526f7b898b872e8f3e Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 26 Oct 2021 19:16:56 -0500 Subject: [PATCH] refactor: cppcoreguidelines-init-variables pt. 14 (#2049) * refactor: fix uninit var warnings in ptrarray * refactor: fix uninit var warnings in bitfield * refactor: fix uninit var warnings in handshake * refactor: fix uninit var warnings in tr-dht * refactor: fix uninit var warnings in natpmp * refactor: fix uninit var warnings in tr-dht * refactor: fix uninit var warnings in crypto-utils-openssl * refactor: fix uninit var warnings in handshake * refactor: fix uninit var warnings in crypto-utils * refactor: fix uninit var warnings in crypto * Revert "refactor: fix uninit var warnings in handshake" This reverts commit 5aaa9cc30a7cdfc247af5e67558cee507b22816f. * refactor: fix uninit var warnings in crypto-utils-ccrypto * refactor: fix uninit var warnings in crypto-utils-polarssl * refactor: fix uninit var warnings in crypto-utils-cyassl * fixup! refactor: fix uninit var warnings in crypto-utils-cyassl * fixup! refactor: cppcoreguidelines-init-variables pt. 13 (#2043) --- libtransmission/crypto-utils-ccrypto.cc | 6 +- libtransmission/crypto-utils-cyassl.cc | 14 +- libtransmission/crypto-utils-openssl.cc | 31 ++-- libtransmission/crypto-utils-polarssl.cc | 9 +- libtransmission/crypto-utils.cc | 23 +-- libtransmission/crypto.cc | 3 +- libtransmission/session.h | 7 - libtransmission/tr-dht.cc | 173 +++++++++++------------ 8 files changed, 113 insertions(+), 153 deletions(-) diff --git a/libtransmission/crypto-utils-ccrypto.cc b/libtransmission/crypto-utils-ccrypto.cc index 9988aeefb..c445ab08b 100644 --- a/libtransmission/crypto-utils-ccrypto.cc +++ b/libtransmission/crypto-utils-ccrypto.cc @@ -215,7 +215,7 @@ tr_dh_ctx_t tr_dh_new( TR_ASSERT(generator_num != nullptr); auto handle = std::make_unique(); - CCStatus status; + auto status = CCStatus{}; handle->p = CCBigNumPtr(CCBigNumFromData(&status, prime_num, prime_num_length)); if (!check_pointer(handle->p.get(), &status)) @@ -243,7 +243,7 @@ bool tr_dh_make_key(tr_dh_ctx_t raw_handle, size_t private_key_length, uint8_t* TR_ASSERT(public_key != nullptr); auto& handle = *static_cast(raw_handle); - CCStatus status; + auto status = CCStatus{}; handle.private_key = CCBigNumPtr(CCBigNumCreateRandom(&status, private_key_length * 8, private_key_length * 8, 0)); if (!check_pointer(handle.private_key.get(), &status)) @@ -286,7 +286,7 @@ tr_dh_secret_t tr_dh_agree(tr_dh_ctx_t raw_handle, uint8_t const* other_public_k TR_ASSERT(other_public_key != nullptr); auto const& handle = *static_cast(raw_handle); - CCStatus status; + auto status = CCStatus{}; auto const other_key = CCBigNumPtr(CCBigNumFromData(&status, other_public_key, other_public_key_length)); if (!check_pointer(other_key.get(), &status)) diff --git a/libtransmission/crypto-utils-cyassl.cc b/libtransmission/crypto-utils-cyassl.cc index d7b396209..43d14cdb5 100644 --- a/libtransmission/crypto-utils-cyassl.cc +++ b/libtransmission/crypto-utils-cyassl.cc @@ -59,7 +59,7 @@ static void log_cyassl_error(int error_code, char const* file, int line) #elif API_VERSION_HEX >= 0x03000002 char const* error_message = CTaoCryptGetErrorString(error_code); #else - char error_message[CYASSL_MAX_ERROR_SZ]; + char error_message[CYASSL_MAX_ERROR_SZ] = {}; CTaoCryptErrorString(error_code, error_message); #endif @@ -211,8 +211,6 @@ bool tr_dh_make_key(tr_dh_ctx_t raw_handle, size_t /*private_key_length*/, uint8 TR_ASSERT(public_key != nullptr); auto* handle = static_cast(raw_handle); - word32 my_private_key_length; - word32 my_public_key_length; tr_lock* rng_lock = get_rng_lock(); if (handle->private_key == nullptr) @@ -222,6 +220,8 @@ bool tr_dh_make_key(tr_dh_ctx_t raw_handle, size_t /*private_key_length*/, uint8 tr_lockLock(rng_lock); + auto my_private_key_length = word32{}; + auto my_public_key_length = word32{}; if (!check_result(API(DhGenerateKeyPair)( &handle->dh, get_rng(), @@ -254,11 +254,10 @@ tr_dh_secret_t tr_dh_agree(tr_dh_ctx_t raw_handle, uint8_t const* other_public_k TR_ASSERT(other_public_key != nullptr); auto* handle = static_cast(raw_handle); - struct tr_dh_secret* ret; - word32 my_secret_key_length; - ret = tr_dh_secret_new(handle->key_length); + tr_dh_secret* ret = tr_dh_secret_new(handle->key_length); + auto my_secret_key_length = word32{}; if (check_result(API(DhAgree)( &handle->dh, ret->key, @@ -287,11 +286,10 @@ bool tr_rand_buffer(void* buffer, size_t length) { TR_ASSERT(buffer != nullptr); - bool ret; tr_lock* rng_lock = get_rng_lock(); tr_lockLock(rng_lock); - ret = check_result(API(RNG_GenerateBlock)(get_rng(), static_cast(buffer), length)); + bool const ret = check_result(API(RNG_GenerateBlock)(get_rng(), static_cast(buffer), length)); tr_lockUnlock(rng_lock); return ret; diff --git a/libtransmission/crypto-utils-openssl.cc b/libtransmission/crypto-utils-openssl.cc index cec236b80..8a7d87f2c 100644 --- a/libtransmission/crypto-utils-openssl.cc +++ b/libtransmission/crypto-utils-openssl.cc @@ -140,7 +140,7 @@ bool tr_sha1_final(tr_sha1_ctx_t raw_handle, uint8_t* hash) { TR_ASSERT(handle != nullptr); - unsigned int hash_length; + unsigned int hash_length = 0; ret = check_result(EVP_DigestFinal_ex(handle, hash, &hash_length)); @@ -258,11 +258,9 @@ tr_dh_ctx_t tr_dh_new( TR_ASSERT(generator_num != nullptr); DH* handle = DH_new(); - BIGNUM* p; - BIGNUM* g; - p = BN_bin2bn(prime_num, prime_num_length, nullptr); - g = BN_bin2bn(generator_num, generator_num_length, nullptr); + BIGNUM* const p = BN_bin2bn(prime_num, prime_num_length, nullptr); + BIGNUM* const g = BN_bin2bn(generator_num, generator_num_length, nullptr); if (!check_pointer(p) || !check_pointer(g) || DH_set0_pqg(handle, p, nullptr, g) == 0) { @@ -293,9 +291,6 @@ bool tr_dh_make_key(tr_dh_ctx_t raw_handle, size_t private_key_length, uint8_t* TR_ASSERT(public_key != nullptr); auto* handle = static_cast(raw_handle); - int dh_size; - int my_public_key_length; - BIGNUM const* my_public_key; DH_set_length(handle, private_key_length * 8); @@ -304,10 +299,11 @@ bool tr_dh_make_key(tr_dh_ctx_t raw_handle, size_t private_key_length, uint8_t* return false; } + BIGNUM const* my_public_key = nullptr; DH_get0_key(handle, &my_public_key, nullptr); - my_public_key_length = BN_bn2bin(my_public_key, public_key); - dh_size = DH_size(handle); + int const my_public_key_length = BN_bn2bin(my_public_key, public_key); + int const dh_size = DH_size(handle); tr_dh_align_key(public_key, my_public_key_length, dh_size); @@ -326,20 +322,15 @@ tr_dh_secret_t tr_dh_agree(tr_dh_ctx_t raw_handle, uint8_t const* other_public_k TR_ASSERT(handle != nullptr); TR_ASSERT(other_public_key != nullptr); - struct tr_dh_secret* ret; - int dh_size; - int secret_key_length; - BIGNUM* other_key; - - if (!check_pointer(other_key = BN_bin2bn(other_public_key, other_public_key_length, nullptr))) + BIGNUM* const other_key = BN_bin2bn(other_public_key, other_public_key_length, nullptr); + if (!check_pointer(other_key)) { return nullptr; } - dh_size = DH_size(handle); - ret = tr_dh_secret_new(dh_size); - - secret_key_length = DH_compute_key(ret->key, other_key, handle); + int const dh_size = DH_size(handle); + tr_dh_secret* ret = tr_dh_secret_new(dh_size); + int const secret_key_length = DH_compute_key(ret->key, other_key, handle); if (check_result_neq(secret_key_length, -1)) { diff --git a/libtransmission/crypto-utils-polarssl.cc b/libtransmission/crypto-utils-polarssl.cc index 5f85c3a9a..f6ae03b42 100644 --- a/libtransmission/crypto-utils-polarssl.cc +++ b/libtransmission/crypto-utils-polarssl.cc @@ -245,17 +245,15 @@ tr_dh_secret_t tr_dh_agree(tr_dh_ctx_t raw_handle, uint8_t const* other_public_k TR_ASSERT(other_public_key != nullptr); auto* handle = static_cast(raw_handle); - struct tr_dh_secret* ret; - size_t secret_key_length; if (!check_result(API(dhm_read_public)(handle, other_public_key, other_public_key_length))) { return nullptr; } - ret = tr_dh_secret_new(handle->len); + tr_dh_secret* const ret = tr_dh_secret_new(handle->len); - secret_key_length = handle->len; + size_t secret_key_length = handle->len; #if API_VERSION_NUMBER >= 0x02000000 @@ -285,11 +283,10 @@ bool tr_rand_buffer(void* buffer, size_t length) { TR_ASSERT(buffer != nullptr); - bool ret; tr_lock* rng_lock = get_rng_lock(); tr_lockLock(rng_lock); - ret = check_result(API(ctr_drbg_random)(get_rng(), static_cast(buffer), length)); + bool const ret = check_result(API(ctr_drbg_random)(get_rng(), static_cast(buffer), length)); tr_lockUnlock(rng_lock); return ret; diff --git a/libtransmission/crypto-utils.cc b/libtransmission/crypto-utils.cc index 39600d12f..9e9f1e21d 100644 --- a/libtransmission/crypto-utils.cc +++ b/libtransmission/crypto-utils.cc @@ -48,9 +48,8 @@ void tr_dh_align_key(uint8_t* key_buffer, size_t key_size, size_t buffer_size) bool tr_sha1(uint8_t* hash, void const* data1, int data1_length, ...) { - tr_sha1_ctx_t sha; - - if ((sha = tr_sha1_init()) == nullptr) + tr_sha1_ctx_t sha = tr_sha1_init(); + if (sha == nullptr) { return false; } @@ -58,10 +57,9 @@ bool tr_sha1(uint8_t* hash, void const* data1, int data1_length, ...) if (tr_sha1_update(sha, data1, data1_length)) { va_list vl; - void const* data; - va_start(vl, data1_length); + void const* data = nullptr; while ((data = va_arg(vl, void const*)) != nullptr) { int const data_length = va_arg(vl, int); @@ -94,8 +92,7 @@ int tr_rand_int(int upper_bound) { TR_ASSERT(upper_bound > 0); - unsigned int noise; - + unsigned int noise = 0; if (tr_rand_buffer(&noise, sizeof(noise))) { return noise % upper_bound; @@ -191,7 +188,7 @@ bool tr_ssha1_matches(char const* ssha1, char const* plain_text) void* tr_base64_encode(void const* input, size_t input_length, size_t* output_length) { - char* ret; + char* ret = nullptr; if (input != nullptr) { @@ -223,10 +220,6 @@ void* tr_base64_encode(void const* input, size_t input_length, size_t* output_le ret = tr_strdup(""); } - else - { - ret = nullptr; - } if (output_length != nullptr) { @@ -243,7 +236,7 @@ void* tr_base64_encode_str(char const* input, size_t* output_length) void* tr_base64_decode(void const* input, size_t input_length, size_t* output_length) { - char* ret; + char* ret = nullptr; if (input != nullptr) { @@ -269,10 +262,6 @@ void* tr_base64_decode(void const* input, size_t input_length, size_t* output_le ret = tr_strdup(""); } - else - { - ret = nullptr; - } if (output_length != nullptr) { diff --git a/libtransmission/crypto.cc b/libtransmission/crypto.cc index 92c97bd9a..cfd59398c 100644 --- a/libtransmission/crypto.cc +++ b/libtransmission/crypto.cc @@ -44,8 +44,7 @@ static void ensureKeyExists(tr_crypto* crypto) { if (crypto->dh == nullptr) { - size_t public_key_length; - + size_t public_key_length = 0; crypto->dh = tr_dh_new(dh_P, sizeof(dh_P), dh_G, sizeof(dh_G)); tr_dh_make_key(crypto->dh, DH_PRIVKEY_LEN, crypto->myPublicKey, &public_key_length); diff --git a/libtransmission/session.h b/libtransmission/session.h index 3fd717d7b..c1c42682e 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -31,13 +31,6 @@ #include "utils.h" #include "variant.h" -enum tr_tristate_t -{ - TR_NET_OK, - TR_NET_ERROR, - TR_NET_WAIT -}; - enum tr_auto_switch_state_t { TR_AUTO_SWITCH_UNUSED, diff --git a/libtransmission/tr-dht.cc b/libtransmission/tr-dht.cc index 12164bd30..01d3f8ba2 100644 --- a/libtransmission/tr-dht.cc +++ b/libtransmission/tr-dht.cc @@ -112,18 +112,16 @@ static int bootstrap_af(tr_session* session) static void bootstrap_from_name(char const* name, tr_port port, int af) { - struct addrinfo hints; - struct addrinfo* info; - char pp[10]; - - memset(&hints, 0, sizeof(hints)); + auto hints = addrinfo{}; hints.ai_socktype = SOCK_DGRAM; hints.ai_family = af; + /* No, just passing p + 1 to gai won't work. */ + char pp[10]; tr_snprintf(pp, sizeof(pp), "%d", (int)port); + addrinfo* info = nullptr; int const rc = getaddrinfo(name, pp, &hints, &info); - if (rc != 0) { tr_logAddNamedError("DHT", "%s:%s: %s", name, pp, gai_strerror(rc)); @@ -173,8 +171,8 @@ static void dht_bootstrap(void* closure) { if (i < num && !bootstrap_done(cl->session, AF_INET)) { - tr_port port; - struct tr_address addr; + auto port = tr_port{}; + auto addr = tr_address{}; memset(&addr, 0, sizeof(addr)); addr.type = TR_AF_INET; @@ -186,8 +184,8 @@ static void dht_bootstrap(void* closure) if (i < num6 && !bootstrap_done(cl->session, AF_INET6)) { - tr_port port; - struct tr_address addr; + auto port = tr_port{}; + auto addr = tr_address{}; memset(&addr, 0, sizeof(addr)); addr.type = TR_AF_INET6; @@ -308,15 +306,6 @@ static void dht_bootstrap(void* closure) int tr_dhtInit(tr_session* ss) { - tr_variant benc; - bool have_id = false; - uint8_t* nodes = nullptr; - uint8_t* nodes6 = nullptr; - uint8_t const* raw; - size_t len = 0; - size_t len6 = 0; - struct bootstrap_closure* cl; - if (session_ != nullptr) /* already initialized */ { return -1; @@ -330,11 +319,18 @@ int tr_dhtInit(tr_session* ss) } char* const dat_file = tr_buildPath(ss->configDir, "dht.dat", nullptr); + auto benc = tr_variant{}; int rc = tr_variantFromFile(&benc, TR_VARIANT_FMT_BENC, dat_file, nullptr) ? 0 : -1; tr_free(dat_file); + bool have_id = false; + uint8_t* nodes = nullptr; + uint8_t* nodes6 = nullptr; + size_t len = 0; + size_t len6 = 0; if (rc == 0) { + uint8_t const* raw = nullptr; have_id = tr_variantDictFindRaw(&benc, TR_KEY_id, &raw, &len); if (have_id && len == 20) @@ -345,26 +341,24 @@ int tr_dhtInit(tr_session* ss) if (ss->udp_socket != TR_BAD_SOCKET && tr_variantDictFindRaw(&benc, TR_KEY_nodes, &raw, &len) && len % 6 == 0) { nodes = static_cast(tr_memdup(raw, len)); + if (nodes == nullptr) + { + len = 0; + } } if (ss->udp6_socket != TR_BAD_SOCKET && tr_variantDictFindRaw(&benc, TR_KEY_nodes6, &raw, &len6) && len6 % 18 == 0) { nodes6 = static_cast(tr_memdup(raw, len6)); + if (nodes6 == nullptr) + { + len6 = 0; + } } tr_variantFree(&benc); } - if (nodes == nullptr) - { - len = 0; - } - - if (nodes6 == nullptr) - { - len6 = 0; - } - if (have_id) { tr_logAddNamedInfo("DHT", "Reusing old id"); @@ -381,12 +375,17 @@ int tr_dhtInit(tr_session* ss) if (rc < 0) { - goto fail; + tr_free(nodes6); + tr_free(nodes); + + tr_logAddNamedDbg("DHT", "DHT initialization failed (errno = %d)", errno); + session_ = nullptr; + return -1; } session_ = ss; - cl = tr_new(struct bootstrap_closure, 1); + auto* const cl = tr_new(struct bootstrap_closure, 1); cl->session = session_; cl->nodes = nodes; cl->nodes6 = nodes6; @@ -400,14 +399,6 @@ int tr_dhtInit(tr_session* ss) tr_logAddNamedDbg("DHT", "DHT initialized"); return 1; - -fail: - tr_free(nodes6); - tr_free(nodes); - - tr_logAddNamedDbg("DHT", "DHT initialization failed (errno = %d)", errno); - session_ = nullptr; - return -1; } void tr_dhtUninit(tr_session* ss) @@ -508,10 +499,10 @@ struct getstatus_closure static void getstatus(void* cl) { auto* closure = static_cast(cl); - int good; - int dubious; - int incoming; + int good = 0; + int dubious = 0; + int incoming = 0; dht_nodes(closure->af, &good, &dubious, nullptr, &incoming); closure->count = good + dubious; @@ -676,59 +667,29 @@ static void callback(void* /*ignore*/, int event, unsigned char const* info_hash } } -static int tr_dhtAnnounce(tr_torrent* tor, int af, bool announce) +enum class AnnounceResult { - int numnodes; - int ret = 0; + INVALID, + OK, + FAILED +}; +static AnnounceResult tr_dhtAnnounce(tr_torrent* tor, int af, bool announce) +{ if (!tr_torrentAllowsDHT(tor)) { - return -1; + return AnnounceResult::INVALID; } + int numnodes = 0; int const status = tr_dhtStatus(tor->session, af, &numnodes); - if (status == TR_DHT_STOPPED) { - /* Let the caller believe everything is all right. */ - return 1; + // let the caller believe everything is all right. + return AnnounceResult::OK; } - if (status >= TR_DHT_POOR) - { - int const rc = dht_search(tor->info.hash, announce ? tr_sessionGetPeerPort(session_) : 0, af, callback, nullptr); - if (rc >= 0) - { - tr_logAddTorInfo( - tor, - "Starting %s DHT announce (%s, %d nodes)", - af == AF_INET6 ? "IPv6" : "IPv4", - tr_dhtPrintableStatus(status), - numnodes); - - if (af == AF_INET) - { - tor->dhtAnnounceInProgress = true; - } - else - { - tor->dhtAnnounce6InProgress = true; - } - - ret = 1; - } - else - { - tr_logAddTorErr( - tor, - "%s DHT announce failed (%s, %d nodes): %s", - af == AF_INET6 ? "IPv6" : "IPv4", - tr_dhtPrintableStatus(status), - numnodes, - tr_strerror(errno)); - } - } - else + if (status < TR_DHT_POOR) { tr_logAddTorDbg( tor, @@ -736,9 +697,39 @@ static int tr_dhtAnnounce(tr_torrent* tor, int af, bool announce) af == AF_INET6 ? "IPv6" : "IPv4", tr_dhtPrintableStatus(status), numnodes); + return AnnounceResult::FAILED; } - return ret; + int const rc = dht_search(tor->info.hash, announce ? tr_sessionGetPeerPort(session_) : 0, af, callback, nullptr); + if (rc < 0) + { + tr_logAddTorErr( + tor, + "%s DHT announce failed (%s, %d nodes): %s", + af == AF_INET6 ? "IPv6" : "IPv4", + tr_dhtPrintableStatus(status), + numnodes, + tr_strerror(errno)); + return AnnounceResult::FAILED; + } + + tr_logAddTorInfo( + tor, + "Starting %s DHT announce (%s, %d nodes)", + af == AF_INET6 ? "IPv6" : "IPv4", + tr_dhtPrintableStatus(status), + numnodes); + + if (af == AF_INET) + { + tor->dhtAnnounceInProgress = true; + } + else + { + tor->dhtAnnounce6InProgress = true; + } + + return AnnounceResult::OK; } void tr_dhtUpkeep(tr_session* session) @@ -754,16 +745,18 @@ void tr_dhtUpkeep(tr_session* session) if (tor->dhtAnnounceAt <= now) { - int const rc = tr_dhtAnnounce(tor, AF_INET, true); + auto const rc = tr_dhtAnnounce(tor, AF_INET, true); - tor->dhtAnnounceAt = now + ((rc == 0) ? 5 + tr_rand_int_weak(5) : 25 * 60 + tr_rand_int_weak(3 * 60)); + tor->dhtAnnounceAt = now + + ((rc == AnnounceResult::FAILED) ? 5 + tr_rand_int_weak(5) : 25 * 60 + tr_rand_int_weak(3 * 60)); } if (tor->dhtAnnounce6At <= now) { - int const rc = tr_dhtAnnounce(tor, AF_INET6, true); + auto const rc = tr_dhtAnnounce(tor, AF_INET6, true); - tor->dhtAnnounce6At = now + ((rc == 0) ? 5 + tr_rand_int_weak(5) : 25 * 60 + tr_rand_int_weak(3 * 60)); + tor->dhtAnnounce6At = now + + ((rc == AnnounceResult::FAILED) ? 5 + tr_rand_int_weak(5) : 25 * 60 + tr_rand_int_weak(3 * 60)); } } } @@ -777,7 +770,7 @@ void tr_dhtCallback(unsigned char* buf, int buflen, struct sockaddr* from, sockl return; } - time_t tosleep; + time_t tosleep = 0; int rc = dht_periodic(buf, buflen, from, fromlen, &tosleep, callback, nullptr); if (rc < 0)