Add correct bool combiner for net signals #5859

pull sipa wants to merge 1 commits into bitcoin:master from sipa:combiner changing 2 files +34 −2

sipa commented at 11:49 AM on March 6, 2015: member

Extracted from #5843.
Add correct bool combiner for net signals 9519a9a420
sipa commented at 11:50 AM on March 6, 2015: member

@gavinandresen No clue how to use that boost::bind inside a type descriptor. Feel free to suggest code.
laanwj commented at 2:22 PM on March 6, 2015: member

ACK
laanwj added the label Bug on Mar 6, 2015
laanwj added the label Priority Medium on Mar 6, 2015

gavinandresen commented at 11:32 PM on March 6, 2015: contributor

Tested ACK.

My c++ kung fu fails me trying to use boost::bind instead of a CombinerAll class. But you could save a couple of lines of code like this:

diff --git a/src/net.h b/src/net.h
index e500536..81783ab 100644
--- a/src/net.h
+++ b/src/net.h
@@ -26,6 +26,7 @@
 #include <arpa/inet.h>
 #endif

+#include <boost/algorithm/cxx11/all_of.hpp>
 #include <boost/filesystem/path.hpp>
 #include <boost/foreach.hpp>
 #include <boost/signals2/signal.hpp>
@@ -83,11 +84,7 @@ struct CombinerAll
     template<typename I>
     bool operator()(I first, I last) const
     {
-        while (first != last) {
-            if (!(*first)) return false;
-            ++first;
-        }
-        return true;
+        return boost::algorithm::all_of_equal<I, bool>(first, last, true);
     }
 };

Take it or leave it, your original code is arguably more readable (doesn't require knowledge of what all_of_equal does).

laanwj commented at 10:56 AM on March 9, 2015: member

#include <boost/algorithm/cxx11/all_of.hpp>

I like the shorter, declarative code. But (looking at the include file name) does that require c++11?
gavinandresen commented at 12:52 PM on March 9, 2015: contributor

This is a boost implementation of functionality already part of C++11 for older C++ compilers.

Gavin Andresen

On Mar 9, 2015, at 6:56 AM, Wladimir J. van der Laan notifications@github.com wrote:

#include

I like the shorter, declarative code. But (looking at the include file name) does that require c++11?

— Reply to this email directly or view it on GitHub.
laanwj commented at 9:10 AM on March 11, 2015: member

Ah, right, a boost-ism that can be changed to a c++11-ism, that's great
jgarzik commented at 1:55 PM on March 11, 2015: contributor

ut ACK
laanwj merged this on Mar 12, 2015
laanwj closed this on Mar 12, 2015
laanwj referenced this in commit dd4ffcec0e on Mar 12, 2015
random-zebra referenced this in commit 8bbc0650e6 on Jul 1, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5843 Limit message sizes before receiving.