Skip to content

Commit 9eaef10

Browse files
author
MacroFake
committed
Merge bitcoin#25707: refactor: Make const references to avoid unnecessarily copying objects and enable two clang-tidy checks
ae7ae36 tidy: Enable two clang-tidy checks (Aurèle Oulès) 081b0e5 refactor: Make const refs vars where applicable (Aurèle Oulès) Pull request description: I added const references to some variables to avoid unnecessarily copying objects. Also added two clang-tidy checks : [performance-for-range-copy](https://releases.llvm.org/11.1.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-for-range-copy.html) and [performance-unnecessary-copy-initialization](https://releases.llvm.org/12.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.html). ACKs for top commit: vasild: ACK ae7ae36 MarcoFalke: review ACK ae7ae36 Tree-SHA512: f6ac6b0cd0eee1e0c34d2f186484bc0f7ec6071451cccb33fa88a67d93d92b304e2fac378b88f087e94657745bca4e966dbc443759587400eb01b1f3061fde8c
2 parents d480586 + ae7ae36 commit 9eaef10

28 files changed

+43
-39
lines changed

‎src/.clang-tidy

+4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ bugprone-argument-comment,
44
misc-unused-using-decls,
55
modernize-use-default-member-init,
66
modernize-use-nullptr,
7+
performance-for-range-copy,
8+
performance-unnecessary-copy-initialization,
79
readability-redundant-declaration,
810
readability-redundant-string-init,
911
'
@@ -12,6 +14,8 @@ bugprone-argument-comment,
1214
misc-unused-using-decls,
1315
modernize-use-default-member-init,
1416
modernize-use-nullptr,
17+
performance-for-range-copy,
18+
performance-unnecessary-copy-initialization,
1519
readability-redundant-declaration,
1620
readability-redundant-string-init,
1721
'

‎src/bitcoin-cli.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ static void GetWalletBalances(UniValue& result)
911911

912912
UniValue balances(UniValue::VOBJ);
913913
for (const UniValue& wallet : wallets.getValues()) {
914-
const std::string wallet_name = wallet.get_str();
914+
const std::string& wallet_name = wallet.get_str();
915915
const UniValue getbalances = ConnectAndCallRPC(&rh, "getbalances", /* args=*/{}, wallet_name);
916916
const UniValue& balance = find_value(getbalances, "result")["mine"]["trusted"];
917917
balances.pushKV(wallet_name, balance);

‎src/bitcoin-tx.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
596596
UniValue prevtxsObj = registers["prevtxs"];
597597
{
598598
for (unsigned int previdx = 0; previdx < prevtxsObj.size(); previdx++) {
599-
UniValue prevOut = prevtxsObj[previdx];
599+
const UniValue& prevOut = prevtxsObj[previdx];
600600
if (!prevOut.isObject())
601601
throw std::runtime_error("expected prevtxs internal object");
602602

‎src/blockfilter.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ const std::set<BlockFilterType>& AllBlockFilterTypes()
169169

170170
static std::once_flag flag;
171171
std::call_once(flag, []() {
172-
for (auto entry : g_filter_types) {
172+
for (const auto& entry : g_filter_types) {
173173
types.insert(entry.first);
174174
}
175175
});
@@ -185,7 +185,7 @@ const std::string& ListBlockFilterTypes()
185185
std::call_once(flag, []() {
186186
std::stringstream ret;
187187
bool first = true;
188-
for (auto entry : g_filter_types) {
188+
for (const auto& entry : g_filter_types) {
189189
if (!first) ret << ", ";
190190
ret << entry.second;
191191
first = false;

‎src/coins.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ bool CCoinsViewErrorCatcher::GetCoin(const COutPoint &outpoint, Coin &coin) cons
296296
try {
297297
return CCoinsViewBacked::GetCoin(outpoint, coin);
298298
} catch(const std::runtime_error& e) {
299-
for (auto f : m_err_callbacks) {
299+
for (const auto& f : m_err_callbacks) {
300300
f();
301301
}
302302
LogPrintf("Error reading from database: %s\n", e.what());

‎src/core_read.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ int ParseSighashString(const UniValue& sighash)
265265
{std::string("SINGLE"), int(SIGHASH_SINGLE)},
266266
{std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)},
267267
};
268-
std::string strHashType = sighash.get_str();
268+
const std::string& strHashType = sighash.get_str();
269269
const auto& it = map_sighash_values.find(strHashType);
270270
if (it != map_sighash_values.end()) {
271271
hash_type = it->second;

‎src/external_signer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector<ExternalS
2828
if (!result.isArray()) {
2929
throw std::runtime_error(strprintf("'%s' received invalid response, expected array of signers", command));
3030
}
31-
for (UniValue signer : result.getValues()) {
31+
for (const UniValue& signer : result.getValues()) {
3232
// Check for error
3333
const UniValue& error = find_value(signer, "error");
3434
if (!error.isNull()) {

‎src/init.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
10461046
static bool LockDataDirectory(bool probeOnly)
10471047
{
10481048
// Make sure only a single Bitcoin process is using the data directory.
1049-
fs::path datadir = gArgs.GetDataDirNet();
1049+
const fs::path& datadir = gArgs.GetDataDirNet();
10501050
if (!DirIsWritable(datadir)) {
10511051
return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir)));
10521052
}

‎src/logging.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ void BCLog::Logger::LogPrintStr(const std::string& str, const std::string& loggi
364364
}
365365

366366
if (m_log_threadnames && m_started_new_line) {
367-
const auto threadname = util::ThreadGetInternalName();
367+
const auto& threadname = util::ThreadGetInternalName();
368368
str_prefixed.insert(0, "[" + (threadname.empty() ? "unknown" : threadname) + "] ");
369369
}
370370

‎src/netbase.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
387387
return error("Error sending to proxy");
388388
}
389389
uint8_t pchRet1[2];
390-
if ((recvr = InterruptibleRecv(pchRet1, 2, g_socks5_recv_timeout, sock)) != IntrRecvError::OK) {
390+
if (InterruptibleRecv(pchRet1, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
391391
LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port);
392392
return false;
393393
}
@@ -410,7 +410,7 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
410410
}
411411
LogPrint(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password);
412412
uint8_t pchRetA[2];
413-
if ((recvr = InterruptibleRecv(pchRetA, 2, g_socks5_recv_timeout, sock)) != IntrRecvError::OK) {
413+
if (InterruptibleRecv(pchRetA, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
414414
return error("Error reading proxy authentication response");
415415
}
416416
if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) {
@@ -476,7 +476,7 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a
476476
if (recvr != IntrRecvError::OK) {
477477
return error("Error reading from proxy");
478478
}
479-
if ((recvr = InterruptibleRecv(pchRet3, 2, g_socks5_recv_timeout, sock)) != IntrRecvError::OK) {
479+
if (InterruptibleRecv(pchRet3, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) {
480480
return error("Error reading from proxy");
481481
}
482482
LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest);

‎src/node/blockstorage.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ void CleanupBlockRevFiles()
415415
// Remove the rev files immediately and insert the blk file paths into an
416416
// ordered map keyed by block file index.
417417
LogPrintf("Removing unusable blk?????.dat and rev?????.dat files for -reindex with -prune\n");
418-
fs::path blocksdir = gArgs.GetBlocksDirPath();
418+
const fs::path& blocksdir = gArgs.GetBlocksDirPath();
419419
for (fs::directory_iterator it(blocksdir); it != fs::directory_iterator(); it++) {
420420
const std::string path = fs::PathToString(it->path().filename());
421421
if (fs::is_regular_file(*it) &&

‎src/qt/splashscreen.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ SplashScreen::SplashScreen(const NetworkStyle* networkStyle)
4444
QString titleText = PACKAGE_NAME;
4545
QString versionText = QString("Version %1").arg(QString::fromStdString(FormatFullVersion()));
4646
QString copyrightText = QString::fromUtf8(CopyrightHolders(strprintf("\xc2\xA9 %u-%u ", 2009, COPYRIGHT_YEAR)).c_str());
47-
QString titleAddText = networkStyle->getTitleAddText();
47+
const QString& titleAddText = networkStyle->getTitleAddText();
4848

4949
QString font = QApplication::font().toString();
5050

‎src/rpc/mining.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ static RPCHelpMan getblocktemplate()
679679
if (lpval.isStr())
680680
{
681681
// Format: <hashBestChain><nTransactionsUpdatedLast>
682-
std::string lpstr = lpval.get_str();
682+
const std::string& lpstr = lpval.get_str();
683683

684684
hashWatchedChain = ParseHashV(lpstr.substr(0, 64), "longpollid");
685685
nTransactionsUpdatedLastLP = LocaleIndependentAtoi<int64_t>(lpstr.substr(64));

‎src/rpc/rawtransaction_util.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,14 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std::
160160
void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins)
161161
{
162162
if (!prevTxsUnival.isNull()) {
163-
UniValue prevTxs = prevTxsUnival.get_array();
163+
const UniValue& prevTxs = prevTxsUnival.get_array();
164164
for (unsigned int idx = 0; idx < prevTxs.size(); ++idx) {
165165
const UniValue& p = prevTxs[idx];
166166
if (!p.isObject()) {
167167
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "expected object with {\"txid'\",\"vout\",\"scriptPubKey\"}");
168168
}
169169

170-
UniValue prevOut = p.get_obj();
170+
const UniValue& prevOut = p.get_obj();
171171

172172
RPCTypeCheckObj(prevOut,
173173
{

‎src/rpc/util.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ CAmount AmountFromValue(const UniValue& value, int decimals)
9898

9999
uint256 ParseHashV(const UniValue& v, std::string strName)
100100
{
101-
std::string strHex(v.get_str());
101+
const std::string& strHex(v.get_str());
102102
if (64 != strHex.length())
103103
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d, for '%s')", strName, 64, strHex.length(), strHex));
104104
if (!IsHex(strHex)) // Note: IsHex("") is false

‎src/test/base58_tests.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ BOOST_AUTO_TEST_CASE(base58_EncodeBase58)
2525
{
2626
UniValue tests = read_json(std::string(json_tests::base58_encode_decode, json_tests::base58_encode_decode + sizeof(json_tests::base58_encode_decode)));
2727
for (unsigned int idx = 0; idx < tests.size(); idx++) {
28-
UniValue test = tests[idx];
28+
const UniValue& test = tests[idx];
2929
std::string strTest = test.write();
3030
if (test.size() < 2) // Allow for extra stuff (useful for comments)
3131
{
@@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58)
4747
std::vector<unsigned char> result;
4848

4949
for (unsigned int idx = 0; idx < tests.size(); idx++) {
50-
UniValue test = tests[idx];
50+
const UniValue& test = tests[idx];
5151
std::string strTest = test.write();
5252
if (test.size() < 2) // Allow for extra stuff (useful for comments)
5353
{

‎src/test/blockfilter_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(blockfilters_json_test)
137137

138138
const UniValue& tests = json.get_array();
139139
for (unsigned int i = 0; i < tests.size(); i++) {
140-
UniValue test = tests[i];
140+
const UniValue& test = tests[i];
141141
std::string strTest = test.write();
142142

143143
if (test.size() == 1) {

‎src/test/key_io_tests.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse)
2828
SelectParams(CBaseChainParams::MAIN);
2929

3030
for (unsigned int idx = 0; idx < tests.size(); idx++) {
31-
UniValue test = tests[idx];
31+
const UniValue& test = tests[idx];
3232
std::string strTest = test.write();
3333
if (test.size() < 3) { // Allow for extra stuff (useful for comments)
3434
BOOST_ERROR("Bad test: " << strTest);
@@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_gen)
8686
UniValue tests = read_json(std::string(json_tests::key_io_valid, json_tests::key_io_valid + sizeof(json_tests::key_io_valid)));
8787

8888
for (unsigned int idx = 0; idx < tests.size(); idx++) {
89-
UniValue test = tests[idx];
89+
const UniValue& test = tests[idx];
9090
std::string strTest = test.write();
9191
if (test.size() < 3) // Allow for extra stuff (useful for comments)
9292
{
@@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE(key_io_invalid)
126126
CTxDestination destination;
127127

128128
for (unsigned int idx = 0; idx < tests.size(); idx++) {
129-
UniValue test = tests[idx];
129+
const UniValue& test = tests[idx];
130130
std::string strTest = test.write();
131131
if (test.size() < 1) // Allow for extra stuff (useful for comments)
132132
{

‎src/test/script_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ BOOST_AUTO_TEST_CASE(script_json_test)
942942
UniValue tests = read_json(std::string(json_tests::script_tests, json_tests::script_tests + sizeof(json_tests::script_tests)));
943943

944944
for (unsigned int idx = 0; idx < tests.size(); idx++) {
945-
UniValue test = tests[idx];
945+
const UniValue& test = tests[idx];
946946
std::string strTest = test.write();
947947
CScriptWitness witness;
948948
CAmount nValue = 0;

‎src/test/sighash_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
165165
UniValue tests = read_json(std::string(json_tests::sighash, json_tests::sighash + sizeof(json_tests::sighash)));
166166

167167
for (unsigned int idx = 0; idx < tests.size(); idx++) {
168-
UniValue test = tests[idx];
168+
const UniValue& test = tests[idx];
169169
std::string strTest = test.write();
170170
if (test.size() < 1) // Allow for extra stuff (useful for comments)
171171
{

‎src/test/transaction_tests.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
194194
UniValue tests = read_json(std::string(json_tests::tx_valid, json_tests::tx_valid + sizeof(json_tests::tx_valid)));
195195

196196
for (unsigned int idx = 0; idx < tests.size(); idx++) {
197-
UniValue test = tests[idx];
197+
const UniValue& test = tests[idx];
198198
std::string strTest = test.write();
199199
if (test[0].isArray())
200200
{
@@ -214,7 +214,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
214214
fValid = false;
215215
break;
216216
}
217-
UniValue vinput = input.get_array();
217+
const UniValue& vinput = input.get_array();
218218
if (vinput.size() < 3 || vinput.size() > 4)
219219
{
220220
fValid = false;
@@ -282,7 +282,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
282282
UniValue tests = read_json(std::string(json_tests::tx_invalid, json_tests::tx_invalid + sizeof(json_tests::tx_invalid)));
283283

284284
for (unsigned int idx = 0; idx < tests.size(); idx++) {
285-
UniValue test = tests[idx];
285+
const UniValue& test = tests[idx];
286286
std::string strTest = test.write();
287287
if (test[0].isArray())
288288
{
@@ -302,7 +302,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
302302
fValid = false;
303303
break;
304304
}
305-
UniValue vinput = input.get_array();
305+
const UniValue& vinput = input.get_array();
306306
if (vinput.size() < 3 || vinput.size() > 4)
307307
{
308308
fValid = false;

‎src/test/validation_block_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
183183
}
184184

185185
// to make sure that eventually we process the full chain - do it here
186-
for (auto block : blocks) {
186+
for (const auto& block : blocks) {
187187
if (block->vtx.size() == 1) {
188188
bool processed = Assert(m_node.chainman)->ProcessNewBlock(block, true, &ignored);
189189
assert(processed);

‎src/wallet/bdb.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ void BerkeleyEnvironment::ReloadDbEnv()
432432
});
433433

434434
std::vector<fs::path> filenames;
435-
for (auto it : m_databases) {
435+
for (const auto& it : m_databases) {
436436
filenames.push_back(it.first);
437437
}
438438
// Close the individual Db's

‎src/wallet/receive.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ std::set< std::set<CTxDestination> > GetAddressGroupings(const CWallet& wallet)
416416

417417
std::set< std::set<CTxDestination>* > uniqueGroupings; // a set of pointers to groups of addresses
418418
std::map< CTxDestination, std::set<CTxDestination>* > setmap; // map addresses to the unique group containing it
419-
for (std::set<CTxDestination> _grouping : groupings)
419+
for (const std::set<CTxDestination>& _grouping : groupings)
420420
{
421421
// make a set of all the groups hit by this new group
422422
std::set< std::set<CTxDestination>* > hits;

‎src/wallet/rpc/spend.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1407,7 +1407,7 @@ RPCHelpMan sendall()
14071407
}
14081408

14091409
CAmount output_amounts_claimed{0};
1410-
for (CTxOut out : rawTx.vout) {
1410+
for (const CTxOut& out : rawTx.vout) {
14111411
output_amounts_claimed += out.nValue;
14121412
}
14131413

‎src/wallet/scriptpubkeyman.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1788,7 +1788,7 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const
17881788
AssertLockHeld(cs_desc_man);
17891789
if (m_storage.HasEncryptionKeys() && !m_storage.IsLocked()) {
17901790
KeyMap keys;
1791-
for (auto key_pair : m_map_crypted_keys) {
1791+
for (const auto& key_pair : m_map_crypted_keys) {
17921792
const CPubKey& pubkey = key_pair.second.first;
17931793
const std::vector<unsigned char>& crypted_secret = key_pair.second.second;
17941794
CKey key;

‎src/wallet/wallet.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -3474,7 +3474,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
34743474
const UniValue& descriptor_vals = find_value(signer_res, internal ? "internal" : "receive");
34753475
if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result");
34763476
for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) {
3477-
std::string desc_str = desc_val.getValStr();
3477+
const std::string& desc_str = desc_val.getValStr();
34783478
FlatSigningProvider keys;
34793479
std::string desc_error;
34803480
std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, desc_error, false);

‎src/wallet/walletdb.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -856,18 +856,18 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
856856
}
857857

858858
// Set the descriptor caches
859-
for (auto desc_cache_pair : wss.m_descriptor_caches) {
859+
for (const auto& desc_cache_pair : wss.m_descriptor_caches) {
860860
auto spk_man = pwallet->GetScriptPubKeyMan(desc_cache_pair.first);
861861
assert(spk_man);
862862
((DescriptorScriptPubKeyMan*)spk_man)->SetCache(desc_cache_pair.second);
863863
}
864864

865865
// Set the descriptor keys
866-
for (auto desc_key_pair : wss.m_descriptor_keys) {
866+
for (const auto& desc_key_pair : wss.m_descriptor_keys) {
867867
auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first);
868868
((DescriptorScriptPubKeyMan*)spk_man)->AddKey(desc_key_pair.first.second, desc_key_pair.second);
869869
}
870-
for (auto desc_key_pair : wss.m_descriptor_crypt_keys) {
870+
for (const auto& desc_key_pair : wss.m_descriptor_crypt_keys) {
871871
auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first);
872872
((DescriptorScriptPubKeyMan*)spk_man)->AddCryptedKey(desc_key_pair.first.second, desc_key_pair.second.first, desc_key_pair.second.second);
873873
}

0 commit comments

Comments
 (0)