Skip to content

Commit d30b9ae

Browse files
committed
refactor: reduce ostringstream usage when constructing strings
1 parent 7200505 commit d30b9ae

File tree

7 files changed

+86
-130
lines changed

7 files changed

+86
-130
lines changed

src/governance/classes.cpp

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -24,55 +24,41 @@ CAmount ParsePaymentAmount(const std::string& strAmount)
2424
{
2525
CAmount nAmount = 0;
2626
if (strAmount.empty()) {
27-
std::ostringstream ostr;
28-
ostr << "ParsePaymentAmount: Amount is empty";
29-
throw std::runtime_error(ostr.str());
27+
throw std::runtime_error(strprintf("%s -- Amount is empty", __func__));
3028
}
3129
if (strAmount.size() > 20) {
3230
// String is much too long, the functions below impose stricter
3331
// requirements
34-
std::ostringstream ostr;
35-
ostr << "ParsePaymentAmount: Amount string too long";
36-
throw std::runtime_error(ostr.str());
32+
throw std::runtime_error(strprintf("%s -- Amount string too long", __func__));
3733
}
3834
// Make sure the string makes sense as an amount
3935
// Note: No spaces allowed
4036
// Also note: No scientific notation
4137
size_t pos = strAmount.find_first_not_of("0123456789.");
4238
if (pos != std::string::npos) {
43-
std::ostringstream ostr;
44-
ostr << "ParsePaymentAmount: Amount string contains invalid character";
45-
throw std::runtime_error(ostr.str());
39+
throw std::runtime_error(strprintf("%s -- Amount string contains invalid character", __func__));
4640
}
4741

4842
pos = strAmount.find('.');
4943
if (pos == 0) {
5044
// JSON doesn't allow values to start with a decimal point
51-
std::ostringstream ostr;
52-
ostr << "ParsePaymentAmount: Invalid amount string, leading decimal point not allowed";
53-
throw std::runtime_error(ostr.str());
45+
throw std::runtime_error(strprintf("%s -- Invalid amount string, leading decimal point not allowed", __func__));
5446
}
5547

5648
// Make sure there's no more than 1 decimal point
5749
if ((pos != std::string::npos) && (strAmount.find('.', pos + 1) != std::string::npos)) {
58-
std::ostringstream ostr;
59-
ostr << "ParsePaymentAmount: Invalid amount string, too many decimal points";
60-
throw std::runtime_error(ostr.str());
50+
throw std::runtime_error(strprintf("%s -- Invalid amount string, too many decimal points", __func__));
6151
}
6252

6353
// Note this code is taken from AmountFromValue in rpcserver.cpp
6454
// which is used for parsing the amounts in createrawtransaction.
6555
if (!ParseFixedPoint(strAmount, 8, &nAmount)) {
6656
nAmount = 0;
67-
std::ostringstream ostr;
68-
ostr << "ParsePaymentAmount: ParseFixedPoint failed for string: " << strAmount;
69-
throw std::runtime_error(ostr.str());
57+
throw std::runtime_error(strprintf("%s -- ParseFixedPoint failed for string \"%s\"", __func__, strAmount));
7058
}
7159
if (!MoneyRange(nAmount)) {
7260
nAmount = 0;
73-
std::ostringstream ostr;
74-
ostr << "ParsePaymentAmount: Invalid amount string, value outside of valid money range";
75-
throw std::runtime_error(ostr.str());
61+
throw std::runtime_error(strprintf("%s -- Invalid amount string, value outside of valid money range", __func__));
7662
}
7763

7864
return nAmount;
@@ -188,17 +174,15 @@ void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, c
188174
// IF THESE DON'T MATCH, SOMETHING IS WRONG
189175

190176
if (vecPaymentAddresses.size() != vecPaymentAmounts.size() || vecPaymentAddresses.size() != vecProposalHashes.size()) {
191-
std::ostringstream ostr;
192-
ostr << "CSuperblock::ParsePaymentSchedule -- Mismatched payments, amounts and proposalHashes";
193-
LogPrintf("%s\n", ostr.str());
194-
throw std::runtime_error(ostr.str());
177+
std::string msg{strprintf("CSuperblock::%s -- Mismatched payments, amounts and proposalHashes", __func__)};
178+
LogPrintf("%s\n", msg);
179+
throw std::runtime_error(msg);
195180
}
196181

197182
if (vecPaymentAddresses.empty()) {
198-
std::ostringstream ostr;
199-
ostr << "CSuperblock::ParsePaymentSchedule -- Error no payments";
200-
LogPrintf("%s\n", ostr.str());
201-
throw std::runtime_error(ostr.str());
183+
std::string msg{strprintf("CSuperblock::%s -- Error no payments", __func__)};
184+
LogPrintf("%s\n", msg);
185+
throw std::runtime_error(msg);
202186
}
203187

204188
// LOOP THROUGH THE ADDRESSES/AMOUNTS AND CREATE PAYMENTS
@@ -210,36 +194,33 @@ void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, c
210194
for (int i = 0; i < (int)vecPaymentAddresses.size(); i++) {
211195
CTxDestination dest = DecodeDestination(vecPaymentAddresses[i]);
212196
if (!IsValidDestination(dest)) {
213-
std::ostringstream ostr;
214-
ostr << "CSuperblock::ParsePaymentSchedule -- Invalid Dash Address : " << vecPaymentAddresses[i];
215-
LogPrintf("%s\n", ostr.str());
216-
throw std::runtime_error(ostr.str());
197+
std::string msg{strprintf("CSuperblock::%s -- Invalid Dash Address: %s", __func__, vecPaymentAddresses[i])};
198+
LogPrintf("%s\n", msg);
199+
throw std::runtime_error(msg);
217200
}
218201

219202
CAmount nAmount = ParsePaymentAmount(vecPaymentAmounts[i]);
220203

221204
uint256 proposalHash;
222205
if (!ParseHashStr(vecProposalHashes[i], proposalHash)) {
223-
std::ostringstream ostr;
224-
ostr << "CSuperblock::ParsePaymentSchedule -- Invalid proposal hash : " << vecProposalHashes[i];
225-
LogPrintf("%s\n", ostr.str());
226-
throw std::runtime_error(ostr.str());
206+
std::string msg{strprintf("CSuperblock::%s -- Invalid proposal hash: %s", __func__, vecProposalHashes[i])};
207+
LogPrintf("%s\n", msg);
208+
throw std::runtime_error(msg);
227209
}
228210

229211
LogPrint(BCLog::GOBJECT, /* Continued */
230-
"CSuperblock::ParsePaymentSchedule -- i = %d, amount string = %s, nAmount = %lld, proposalHash = %s\n",
212+
"CSuperblock::%s -- i = %d, amount string = %s, nAmount = %lld, proposalHash = %s\n", __func__,
231213
i, vecPaymentAmounts[i], nAmount, proposalHash.ToString());
232214

233215
CGovernancePayment payment(dest, nAmount, proposalHash);
234216
if (payment.IsValid()) {
235217
vecPayments.push_back(payment);
236218
} else {
237219
vecPayments.clear();
238-
std::ostringstream ostr;
239-
ostr << "CSuperblock::ParsePaymentSchedule -- Invalid payment found: address = " << EncodeDestination(dest)
240-
<< ", amount = " << nAmount;
241-
LogPrintf("%s\n", ostr.str());
242-
throw std::runtime_error(ostr.str());
220+
std::string msg{strprintf("CSuperblock::%s -- Invalid payment found: address = %s, amount = %d", __func__,
221+
EncodeDestination(dest), nAmount)};
222+
LogPrintf("%s\n", msg);
223+
throw std::runtime_error(msg);
243224
}
244225
}
245226
}

src/governance/exceptions.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

55
#include <governance/exceptions.h>
6+
#include <tinyformat.h>
67

78
#include <iostream>
89
#include <sstream>
@@ -32,11 +33,8 @@ std::ostream& operator<<(std::ostream& os, governance_exception_type_enum_t eTyp
3233
CGovernanceException::CGovernanceException(const std::string& strMessageIn,
3334
governance_exception_type_enum_t eTypeIn,
3435
int nNodePenaltyIn) :
35-
strMessage(),
36-
eType(eTypeIn),
37-
nNodePenalty(nNodePenaltyIn)
36+
strMessage{strprintf("%s:%s", eTypeIn, strMessageIn)},
37+
eType{eTypeIn},
38+
nNodePenalty{nNodePenaltyIn}
3839
{
39-
std::ostringstream ostr;
40-
ostr << eType << ":" << strMessageIn;
41-
strMessage = ostr.str();
4240
}

src/governance/governance.cpp

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,44 +1131,43 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
11311131
uint256 nHashGovobj = vote.GetParentHash();
11321132

11331133
if (cmapVoteToObject.HasKey(nHashVote)) {
1134-
LogPrint(BCLog::GOBJECT, "CGovernanceObject::ProcessVote -- skipping known valid vote %s for object %s\n", nHashVote.ToString(), nHashGovobj.ToString());
1134+
LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- skipping known valid vote %s for object %s\n", __func__,
1135+
nHashVote.ToString(), nHashGovobj.ToString());
11351136
LEAVE_CRITICAL_SECTION(cs);
11361137
return false;
11371138
}
11381139

11391140
if (cmapInvalidVotes.HasKey(nHashVote)) {
1140-
std::ostringstream ostr;
1141-
ostr << "CGovernanceManager::ProcessVote -- Old invalid vote "
1142-
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort()
1143-
<< ", governance object hash = " << nHashGovobj.ToString();
1144-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
1145-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
1141+
std::string msg{strprintf("CGovernanceManager::%s -- Old invalid vote, MN outpoint = %s, governance object hash = %s",
1142+
__func__, vote.GetMasternodeOutpoint().ToStringShort(), nHashGovobj.ToString())};
1143+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
1144+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
11461145
LEAVE_CRITICAL_SECTION(cs);
11471146
return false;
11481147
}
11491148

11501149
auto it = mapObjects.find(nHashGovobj);
11511150
if (it == mapObjects.end()) {
1152-
std::ostringstream ostr;
1153-
ostr << "CGovernanceManager::ProcessVote -- Unknown parent object " << nHashGovobj.ToString()
1154-
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort();
1155-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_WARNING);
1151+
std::string msg{strprintf("CGovernanceManager::%s -- Unknown parent object %s, MN outpoint = %s", __func__,
1152+
nHashGovobj.ToString(), vote.GetMasternodeOutpoint().ToStringShort())};
1153+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING);
11561154
if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, GetTime<std::chrono::seconds>().count() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) {
11571155
LEAVE_CRITICAL_SECTION(cs);
11581156
RequestGovernanceObject(pfrom, nHashGovobj, connman);
1159-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
1157+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
11601158
return false;
11611159
}
11621160

1163-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
1161+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
11641162
LEAVE_CRITICAL_SECTION(cs);
11651163
return false;
11661164
}
11671165

11681166
CGovernanceObject& govobj = it->second;
11691167

11701168
if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) {
1171-
LogPrint(BCLog::GOBJECT, "CGovernanceObject::ProcessVote -- ignoring vote for expired or deleted object, hash = %s\n", nHashGovobj.ToString());
1169+
LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- ignoring vote for expired or deleted object, hash = %s\n",
1170+
__func__, nHashGovobj.ToString());
11721171
LEAVE_CRITICAL_SECTION(cs);
11731172
return false;
11741173
}

src/governance/object.cpp

Lines changed: 39 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -66,77 +66,71 @@ bool CGovernanceObject::ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceM
6666
// do not process already known valid votes twice
6767
if (fileVotes.HasVote(vote.GetHash())) {
6868
// nothing to do here, not an error
69-
std::ostringstream ostr;
70-
ostr << "CGovernanceObject::ProcessVote -- Already known valid vote";
71-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
72-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_NONE);
69+
std::string msg{strprintf("CGovernanceObject::%s -- Already known valid vote", __func__)};
70+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
71+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_NONE);
7372
return false;
7473
}
7574

7675
auto dmn = tip_mn_list.GetMNByCollateral(vote.GetMasternodeOutpoint());
7776
if (!dmn) {
78-
std::ostringstream ostr;
79-
ostr << "CGovernanceObject::ProcessVote -- Masternode " << vote.GetMasternodeOutpoint().ToStringShort() << " not found";
80-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
77+
std::string msg{strprintf("CGovernanceObject::%s -- Masternode %s not found", __func__,
78+
vote.GetMasternodeOutpoint().ToStringShort())};
79+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
8180
return false;
8281
}
8382

8483
auto it = mapCurrentMNVotes.emplace(vote_m_t::value_type(vote.GetMasternodeOutpoint(), vote_rec_t())).first;
8584
vote_rec_t& voteRecordRef = it->second;
8685
vote_signal_enum_t eSignal = vote.GetSignal();
8786
if (eSignal == VOTE_SIGNAL_NONE) {
88-
std::ostringstream ostr;
89-
ostr << "CGovernanceObject::ProcessVote -- Vote signal: none";
90-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
91-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_WARNING);
87+
std::string msg{strprintf("CGovernanceObject::%s -- Vote signal: none", __func__)};
88+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
89+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING);
9290
return false;
9391
}
9492
if (eSignal < VOTE_SIGNAL_NONE || eSignal >= VOTE_SIGNAL_UNKNOWN) {
95-
std::ostringstream ostr;
96-
ostr << "CGovernanceObject::ProcessVote -- Unsupported vote signal: " << CGovernanceVoting::ConvertSignalToString(vote.GetSignal());
97-
LogPrintf("%s\n", ostr.str());
98-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
93+
std::string msg{strprintf("CGovernanceObject::%s -- Unsupported vote signal: %s", __func__,
94+
CGovernanceVoting::ConvertSignalToString(vote.GetSignal()))};
95+
LogPrintf("%s\n", msg);
96+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
9997
return false;
10098
}
10199
auto it2 = voteRecordRef.mapInstances.emplace(vote_instance_m_t::value_type(int(eSignal), vote_instance_t())).first;
102100
vote_instance_t& voteInstanceRef = it2->second;
103101

104102
// Reject obsolete votes
105103
if (vote.GetTimestamp() < voteInstanceRef.nCreationTime) {
106-
std::ostringstream ostr;
107-
ostr << "CGovernanceObject::ProcessVote -- Obsolete vote";
108-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
109-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_NONE);
104+
std::string msg{strprintf("CGovernanceObject::%s -- Obsolete vote", __func__)};
105+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
106+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_NONE);
110107
return false;
111108
} else if (vote.GetTimestamp() == voteInstanceRef.nCreationTime) {
112109
// Someone is doing something fishy, there can be no two votes from the same masternode
113110
// with the same timestamp for the same object and signal and yet different hash/outcome.
114-
std::ostringstream ostr;
115-
ostr << "CGovernanceObject::ProcessVote -- Invalid vote, same timestamp for the different outcome";
111+
std::string msg{strprintf("CGovernanceObject::%s -- Invalid vote, same timestamp for the different outcome", __func__)};
116112
if (vote.GetOutcome() < voteInstanceRef.eOutcome) {
117113
// This is an arbitrary comparison, we have to agree on some way
118114
// to pick the "winning" vote.
119-
ostr << ", rejected";
120-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
121-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_NONE);
115+
msg += ", rejected";
116+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
117+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_NONE);
122118
return false;
123119
}
124-
ostr << ", accepted";
125-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
120+
msg += ", accepted";
121+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
126122
}
127123

128124
int64_t nNow = GetAdjustedTime();
129125
int64_t nVoteTimeUpdate = voteInstanceRef.nTime;
130126
if (govman.AreRateChecksEnabled()) {
131127
int64_t nTimeDelta = nNow - voteInstanceRef.nTime;
132128
if (nTimeDelta < GOVERNANCE_UPDATE_MIN) {
133-
std::ostringstream ostr;
134-
ostr << "CGovernanceObject::ProcessVote -- Masternode voting too often"
135-
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort()
136-
<< ", governance object hash = " << GetHash().ToString()
137-
<< ", time delta = " << nTimeDelta;
138-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
139-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_TEMPORARY_ERROR);
129+
std::string msg{strprintf("CGovernanceObject::%s -- Masternode voting too often, MN outpoint = %s, "
130+
"governance object hash = %s, time delta = %d",
131+
__func__, vote.GetMasternodeOutpoint().ToStringShort(), GetHash().ToString(), nTimeDelta)};
132+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
133+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_TEMPORARY_ERROR);
140134
return false;
141135
}
142136
nVoteTimeUpdate = nNow;
@@ -146,24 +140,21 @@ bool CGovernanceObject::ProcessVote(CMasternodeMetaMan& mn_metaman, CGovernanceM
146140

147141
// Finally check that the vote is actually valid (done last because of cost of signature verification)
148142
if (!vote.IsValid(tip_mn_list, onlyVotingKeyAllowed)) {
149-
std::ostringstream ostr;
150-
ostr << "CGovernanceObject::ProcessVote -- Invalid vote"
151-
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort()
152-
<< ", governance object hash = " << GetHash().ToString()
153-
<< ", vote hash = " << vote.GetHash().ToString();
154-
LogPrintf("%s\n", ostr.str());
155-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
143+
std::string msg{strprintf("CGovernanceObject::%s -- Invalid vote, MN outpoint = %s, governance object hash = %s, "
144+
"vote hash = %s",
145+
__func__, vote.GetMasternodeOutpoint().ToStringShort(), GetHash().ToString(), vote.GetHash().ToString())};
146+
LogPrintf("%s\n", msg);
147+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
156148
govman.AddInvalidVote(vote);
157149
return false;
158150
}
159151

160152
if (!mn_metaman.AddGovernanceVote(dmn->proTxHash, vote.GetParentHash())) {
161-
std::ostringstream ostr;
162-
ostr << "CGovernanceObject::ProcessVote -- Unable to add governance vote"
163-
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort()
164-
<< ", governance object hash = " << GetHash().ToString();
165-
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
166-
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
153+
std::string msg{strprintf("CGovernanceObject::%s -- Unable to add governance vote, MN outpoint = %s, "
154+
"governance object hash = %s",
155+
__func__, vote.GetMasternodeOutpoint().ToStringShort(), GetHash().ToString())};
156+
LogPrint(BCLog::GOBJECT, "%s\n", msg);
157+
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR);
167158
return false;
168159
}
169160

@@ -323,16 +314,11 @@ void CGovernanceObject::LoadData()
323314
m_obj.type = GovernanceObject(obj["type"].get_int());
324315
} catch (std::exception& e) {
325316
fUnparsable = true;
326-
std::ostringstream ostr;
327-
ostr << "CGovernanceObject::LoadData Error parsing JSON"
328-
<< ", e.what() = " << e.what();
329-
LogPrintf("%s\n", ostr.str());
317+
LogPrintf("%s\n", strprintf("CGovernanceObject::LoadData -- Error parsing JSON, e.what() = %s", e.what()));
330318
return;
331319
} catch (...) {
332320
fUnparsable = true;
333-
std::ostringstream ostr;
334-
ostr << "CGovernanceObject::LoadData Unknown Error parsing JSON";
335-
LogPrintf("%s\n", ostr.str());
321+
LogPrintf("%s\n", strprintf("CGovernanceObject::LoadData -- Unknown Error parsing JSON"));
336322
return;
337323
}
338324
}

0 commit comments

Comments
 (0)