range-based loops and const qualifications in net.cpp #10888

benma · 2017-07-20T09:44:48Z

Plus a use of std::copy() instead of manual copying.

(The loop on line 117 is already done in #10493).

jonasschnelli · 2017-07-20T11:27:10Z

src/net.cpp

    return NULL;
 }

 CNode* CConnman::FindNode(const CSubNet& subNet)
 {
    LOCK(cs_vNodes);
    for (CNode* pnode : vNodes)
-    if (subNet.Match((CNetAddr)pnode->addr))
-        return (pnode);
+        if (subNet.Match((CNetAddr)pnode->addr))


Please switch to brackets here.

practicalswift · 2017-07-20T11:38:33Z

Concept ACK (strong ACK! :-))

bytting · 2017-07-21T04:37:14Z

How about "improving" those FindNode functions to something like this,

auto iter = std::find_if(vNodes.begin(), vNodes.end(), [&](CNode *node) {
    return node->GetAddrName() == addrName;
});

return iter == vNodes.end() ? NULL : *iter;

I believe C++11 allows that

practicalswift · 2017-07-21T09:17:53Z

@corebob I guess it is a matter of taste, but personally I find the current range-based for loop form more straightforward and easier to read:

     for (CNode* pnode : vNodes) {
          if (pnode->GetAddrName() == addrName) {
              return pnode;
          }
      }
      return NULL;

bytting · 2017-07-21T09:44:56Z

@practicalswift
I pretty much agree. I think pro arguments could be made for getting rid of a raw loop, and getting rid of an extra exit point. Though probably not worth it in this case.

benma · 2017-07-22T07:39:23Z

I am usually in favor of using algorithm, as explicit names (find_if) and lambdas are more descriptive than loops etc., but in this case, I don't like the return value conversion, so it indeed looks more complicated than before. Too bad.

benma · 2017-09-11T13:46:01Z

Rebased.

benma · 2017-09-11T17:27:28Z

Can someone please retrigger CI?

sipa · 2017-09-11T23:50:08Z

utACK a9144d535122a4d08348e477315b5a94368e6dfc

Travis passed after restart.

maflcko

utACK a9144d535.

Since this is refactoring only you also might want to use clang format on the changed lines. (See the developer notes for a one-liner to fixup the formatting)

maflcko · 2017-09-12T03:02:25Z

src/net.cpp

@@ -135,11 +135,11 @@ static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn
    const int64_t nOneWeek = 7*24*60*60;
    std::vector<CAddress> vSeedsOut;
    vSeedsOut.reserve(vSeedsIn.size());
-    for (std::vector<SeedSpec6>::const_iterator i(vSeedsIn.begin()); i != vSeedsIn.end(); ++i)
+    for (const auto seed_in : vSeedsIn)


const SeedSpec6& seed_in makes more sense to me as it prevents a copy, no?

Correct. Done!

Plus a use of std::copy() instead of manual copying.

benma · 2017-09-12T07:12:36Z

@MarcoFalke I also applied clang-format. Nice tool. Maybe it should go into the CI for all future changes ;)

maflcko · 2017-09-12T07:17:16Z

re-utACK 05cae8a

benma · 2017-09-20T23:12:19Z

Can this be merged?

05cae8a range-based loops and const qualifications in net.cpp (Marko Bencun) Pull request description: Plus a use of std::copy() instead of manual copying. (The loop on line 117 is already done in #10493). Tree-SHA512: d9839e330c71bb9781a4efa81ee353c9e3fd8a93c2120a309f7a0e516b119dd7abe0f0988546797801258b867a29581978515c05dda9e5b23097e15f705139b4

…t.cpp 05cae8a range-based loops and const qualifications in net.cpp (Marko Bencun) Pull request description: Plus a use of std::copy() instead of manual copying. (The loop on line 117 is already done in bitcoin#10493). Tree-SHA512: d9839e330c71bb9781a4efa81ee353c9e3fd8a93c2120a309f7a0e516b119dd7abe0f0988546797801258b867a29581978515c05dda9e5b23097e15f705139b4

7a5d181 Use the character based overload for std::string::find. (Alin Rus) c19401b Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (practicalswift) 4c5fe36 [Refactor] Remove unused fQuit var from checkqueue.h (donaloconnor) fda7a5f Cleanup: remove unneeded header includes (random-zebra) ac2476c Add test cases covering the relevant key length boundaries: 64 bytes +/- 1 byte for HMAC-SHA256 and 128 bytes +/- 1 byte for HMAC-SHA512 (practicalswift) a346262 Fix code constness in CBlockIndex::GetAncestor() overloads (Dan Raviv) 65e3f4e Refactor: More range-based for loops (random-zebra) dd3d3c4 Use range-based for loops (C++11) when looping over map elements (practicalswift) 5a7750a Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky) 402b4c4 Use compile-time constants instead of unnamed enumerations (practicalswift) bbac2d0 [Trivial] Fix indentation in coins.cpp (random-zebra) e539c62 Small refactor of CCoinsViewCache::BatchWrite() (Dan Raviv) ec91759 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (random-zebra) 93487b1 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (random-zebra) ff43d69 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) b4d9641 Use unique_ptr for dbenv (DbEnv) (practicalswift) 36108b9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) ff1c454 Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 93daf17 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 020ac16 Init: Remove redundant exit(EXIT_FAILURE) instances and replace with (random-zebra) b9f5d1f Remove duplicate uriParts.size() > 0 check (practicalswift) 440d961 Remove redundant check (!ecc is always true) (practicalswift) bfd295b Remove redundant NULL checks after new (practicalswift) 97aad32 Make fUseCrypto atomic (MeshCollider) 2711f78 Consistent parameter names in txdb.h (MeshCollider) d40df3a Fix race for mapBlockIndex in AppInitMain (random-zebra) 03b7766 Cleanup: remove unused functions to Hash the concat of 4 or more objects (random-zebra) c520e0f Remove some unused functions and methods (Pieter Wuille) 508f1a1 range-based loops and const qualifications in net.cpp (Marko Bencun) 79b1e50 Refactor: implement CPubKey::data() (random-zebra) 614d26c Refactor: more &vec[0] to vec.data() (random-zebra) 02b6337 Changing &vec[0] to vec.data(), what 9804 missed (MeshCollider) c1c8b05 Ensure that data types are consistent (jjz) 732bb9d Fix potential null dereferences (MeshCollider) 80f35f9 Remove unreachable code (practicalswift) Pull request description: This is a collection of simple refactorings coming from upstream Bitcoin v0.16 (adapting/extending to PIVX-specific code where needed). Pull requests backported: - bitcoin#10845 (practicalswift) - bitcoin#11238 (MeshCollider) - bitcoin#11232 (jjz) - bitcoin#10793 (MeshCollider) - bitcoin#10888 (benma) - ~~bitcoin#11351 (danra)~~ [edit: removed - included in #2423] - bitcoin#11385 (sipa) - bitcoin#11107 (MeshCollider) - bitcoin#10898 (practicalswift) - bitcoin#11511 (donaloconnor) - bitcoin#11043 (practicalswift) - bitcoin#11353 (danra) - bitcoin#10749 (practicalswift) - bitcoin#11603 (ryanofsky) - bitcoin#10493 (practicalswift) - bitcoin#11337 (danra) - bitcoin#11516 (practicalswift) - bitcoin#10574 (practicalswift) - bitcoin#12108 (donaloconnor) - bitcoin#10839 (practicalswift) - bitcoin#12159 (kekimusmaximus) ACKs for top commit: Fuzzbawls: ACK 7a5d181 furszy: re-ACK 7a5d181 after rebase, no code changes. Merging.. Tree-SHA512: d92f5df47f443391a6531274a2efb9a4882c62d32eff628f795b3abce703f108c8b40ec80ac841cde6c5fdd5c9ff2b6056a31546ac2edda279f5f18fccc99c32

benma force-pushed the netcpp_cosmetics2 branch from f4dba40 to ddbdf9f Compare July 20, 2017 09:46

jonasschnelli reviewed Jul 20, 2017

View reviewed changes

jonasschnelli added the Refactoring label Jul 20, 2017

benma force-pushed the netcpp_cosmetics2 branch from ddbdf9f to 44a91bc Compare July 20, 2017 16:56

dcousens approved these changes Jul 21, 2017

View reviewed changes

benma force-pushed the netcpp_cosmetics2 branch from 44a91bc to a9144d5 Compare September 11, 2017 13:45

maflcko reviewed Sep 12, 2017

View reviewed changes

benma force-pushed the netcpp_cosmetics2 branch from a9144d5 to 05cae8a Compare September 12, 2017 07:11

range-based loops and const qualifications in net.cpp

05cae8a

Plus a use of std::copy() instead of manual copying.

sipa merged commit 05cae8a into bitcoin:master Sep 20, 2017

codablock added a commit to codablock/dash that referenced this pull request Jan 14, 2020

More of bitcoin#10888

bedfb78

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 14, 2020

More of bitcoin#10888

78c06fa

random-zebra mentioned this pull request Jun 29, 2021

[Refactoring] Bitcoin 0.16 cherries PIVX-Project/PIVX#2462

Merged

gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021

More of bitcoin#10888

7cdd0f2

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

range-based loops and const qualifications in net.cpp #10888

range-based loops and const qualifications in net.cpp #10888

Uh oh!

Conversation

benma commented Jul 20, 2017

Uh oh!

jonasschnelli Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

benma Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Jul 20, 2017

Uh oh!

bytting commented Jul 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Jul 21, 2017

Uh oh!

bytting commented Jul 21, 2017

Uh oh!

benma commented Jul 22, 2017

Uh oh!

benma commented Sep 11, 2017

Uh oh!

benma commented Sep 11, 2017

Uh oh!

sipa commented Sep 11, 2017

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko Sep 12, 2017

Choose a reason for hiding this comment

Uh oh!

benma Sep 12, 2017

Choose a reason for hiding this comment

Uh oh!

benma commented Sep 12, 2017

Uh oh!

maflcko commented Sep 12, 2017

Uh oh!

benma commented Sep 20, 2017

Uh oh!

Uh oh!

bytting commented Jul 21, 2017 •

edited

Loading