-
Notifications
You must be signed in to change notification settings - Fork 37.7k
use const references where appropriate #6206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There are still a lot of these cases where a const reference can be used for strings throughout the source code. Instead of many little pulls I'd prefer (for post-0.11) a pull where they're all addressed at once. A quick
|
I'm going to update this :). |
ParseNetwork() isn't suitable, because we use |
@laanwj I catched all your results and also changed a few other occurances in BOOST_FOREACH loops and in the CNode constructor (unrelated to std::string). Mind reviewing it? |
@@ -1906,7 +1906,7 @@ bool CAddrDB::Read(CAddrMan& addr) | |||
unsigned int ReceiveFloodSize() { return 1000*GetArg("-maxreceivebuffer", 5*1000); } | |||
unsigned int SendBufferSize() { return 1000*GetArg("-maxsendbuffer", 1*1000); } | |||
|
|||
CNode::CNode(SOCKET hSocketIn, CAddress addrIn, std::string addrNameIn, bool fInboundIn) : | |||
CNode::CNode(const SOCKET& hSocketIn, const CAddress& addrIn, const std::string& addrNameIn, bool fInboundIn) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to do this for SOCKET, which is just an integer handle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, reverted for hSocketIn :), thanks.
@laanwj I'm going to attach a second squashme pull, which covers BOOST_FOREACH and updated to constant refs if appropriate, if you want me to do so? |
utACK |
Yes, why not |
@laanwj See 2nd commit. |
utACK^2 . WIll merge this after #6121 |
That's fine, could be I forgot a few cases in the GUI code, but that can be a seperate pull. |
utACK |
Will merge after rebase. |
@laanwj Fixed 2 merge conflicts, one in rpcnet.cpp and the other in utilstrencodings.cpp and added getValStr(). Should be ready. |
a9ac95c use const references where appropriate (Philip Kaufmann)
Bitcoin 0.12 misc PRs 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6198 - bitcoin/bitcoin#6206 - bitcoin/bitcoin#5927 - bitcoin/bitcoin#6213 - bitcoin/bitcoin#6061 - bitcoin/bitcoin#6283 (partial, remainder was pulled in #929) - bitcoin/bitcoin#6272 - bitcoin/bitcoin#6316 - bitcoin/bitcoin#6133 - bitcoin/bitcoin#6387 - bitcoin/bitcoin#6401 - bitcoin/bitcoin#6434 - bitcoin/bitcoin#6372 - bitcoin/bitcoin#6447 - bitcoin/bitcoin#6149 - bitcoin/bitcoin#6468 Part of #2074.
No description provided.