Fixed 'if' statement consistency issues in the code base

added failure short circuits for validation functions
used sizeof instead of a magic number for a string length
simplified a nullptr test
removed the use of auto with pod types

audit/audit.cpp

@@ -61,7 +61,7 @@ audit::start()
                                                             , shared_from_this()
                                                             , std::placeholders::_1
                                                             , std::placeholders::_2));
-        if(this->monitor_endpoint)
+        if (this->monitor_endpoint)
         {
             LOG(info) << boost::format("Audit module running, will send stats to %1%:%2%")
                 % this->monitor_endpoint->address().to_string()
@@ -75,7 +75,7 @@ audit::start()
         LOG(info) << "Audit module running";
 
 
-        if(this->use_pbft)
+        if (this->use_pbft)
         {
             this->pbft_specific_init();
         }
@@ -123,7 +123,7 @@ audit::reset_leader_alive_timer()
 void
 audit::handle_leader_alive_timeout(const boost::system::error_code& ec)
 {
-    if(ec)
+    if (ec)
     {
         LOG(debug) << "Leader alive timeout canceled " << ec.message();
         return;
@@ -139,7 +139,7 @@ audit::handle_leader_alive_timeout(const boost::system::error_code& ec)
 void
 audit::handle_primary_alive_timeout(const boost::system::error_code& ec)
 {
-    if(ec)
+    if (ec)
     {
         LOG(debug) << "Primary alive timeout canceled " << ec.message();
         return;
@@ -169,7 +169,7 @@ audit::clear_leader_progress_timer()
 
 void audit::handle_leader_progress_timeout(const boost::system::error_code& ec)
 {
-    if(ec)
+    if (ec)
     {
         LOG(debug) << "Leader progress timeout canceled " << ec.message();
         return;
@@ -196,7 +196,7 @@ audit::report_error(const std::string& metric_name, const std::string& descripti
 void
 audit::send_to_monitor(const std::string& stat)
 {
-    if(!this->monitor_endpoint)
+    if (!this->monitor_endpoint)
     {
         return;
     }
@@ -225,23 +225,23 @@ audit::handle(const bzn::message& json, std::shared_ptr<bzn::session_base> sessi
 
     LOG(debug) << "Got audit message" << message.DebugString();
 
-    if(message.has_raft_commit())
+    if (message.has_raft_commit())
     {
         this->handle_raft_commit(message.raft_commit());
     }
-    else if(message.has_leader_status())
+    else if (message.has_leader_status())
     {
         this->handle_leader_status(message.leader_status());
     }
-    else if(message.has_pbft_commit())
+    else if (message.has_pbft_commit())
     {
         this->handle_pbft_commit(message.pbft_commit());
     }
-    else if(message.has_primary_status())
+    else if (message.has_primary_status())
     {
         this->handle_primary_status(message.primary_status());
     }
-    else if(message.has_failure_detected())
+    else if (message.has_failure_detected())
     {
         this->handle_failure_detected(message.failure_detected());
     }
@@ -255,22 +255,22 @@ audit::handle(const bzn::message& json, std::shared_ptr<bzn::session_base> sessi
 
 void audit::handle_primary_status(const primary_status& primary_status)
 {
-    if(!this->use_pbft)
+    if (!this->use_pbft)
     {
         LOG(debug) << "audit ignoring primary status message because we are in raft mode";
         return;
     }
 
     std::lock_guard<std::mutex> lock(this->audit_lock);
 
-    if(this->recorded_primaries.count(primary_status.view()) == 0)
+    if (this->recorded_primaries.count(primary_status.view()) == 0)
     {
         LOG(info) << "audit recording that primary of view " << primary_status.view() << " is '" << primary_status.primary() << "'";
         this->send_to_monitor(bzn::PRIMARY_HEARD_METRIC_NAME+bzn::STATSD_COUNTER_FORMAT);
         this->recorded_primaries[primary_status.view()] = primary_status.primary();
         this->trim();
     }
-    else if(this->recorded_primaries[primary_status.view()] != primary_status.primary())
+    else if (this->recorded_primaries[primary_status.view()] != primary_status.primary())
     {
         std::string err = str(boost::format(
                 "Conflicting primary elected! '%1%' is the recorded primary of view %2%, but '%3%' claims to be the primary of the same view.")
@@ -286,22 +286,22 @@ void audit::handle_primary_status(const primary_status& primary_status)
 void
 audit::handle_leader_status(const leader_status& leader_status)
 {
-    if(this->use_pbft)
+    if (this->use_pbft)
     {
         LOG(debug) << "audit ignoring leader status message because we are in pbft mode";
         return;
     }
 
     std::lock_guard<std::mutex> lock(this->audit_lock);
 
-    if(this->recorded_leaders.count(leader_status.term()) == 0)
+    if (this->recorded_leaders.count(leader_status.term()) == 0)
     {
         LOG(info) << "audit recording that leader of term " << leader_status.term() << " is '" << leader_status.leader() << "'";
         this->send_to_monitor(bzn::NEW_LEADER_METRIC_NAME+bzn::STATSD_COUNTER_FORMAT);
         this->recorded_leaders[leader_status.term()] = leader_status.leader();
         this->trim();
     }
-    else if(this->recorded_leaders[leader_status.term()] != leader_status.leader())
+    else if (this->recorded_leaders[leader_status.term()] != leader_status.leader())
     {
         std::string err = str(boost::format(
                 "Conflicting leader elected! '%1%' is the recorded leader of term %2%, but '%3%' claims to be the leader of the same term.")
@@ -325,7 +325,7 @@ audit::handle_leader_data(const leader_status& leader_status)
     // - The timer must be reset, but not halted, when the leader commits some but not all of the uncommitted entries
     // - The timer must be restarted from full when the leader gets an uncommitted entry where it previously had none
 
-    if(leader_status.leader() != this->last_leader)
+    if (leader_status.leader() != this->last_leader)
     {
         // Leader has changed - halt or restart timer
         this->last_leader = leader_status.leader();
@@ -350,7 +350,7 @@ void audit::handle_leader_made_progress(const leader_status& leader_status)
     // Extracted common logic in the case where we know that the leader has made some progress,
     // but may or may not still have more messages to commit
 
-    if(leader_status.current_commit_index() == leader_status.current_log_index())
+    if (leader_status.current_commit_index() == leader_status.current_log_index())
     {
         this->clear_leader_progress_timer();
         this->leader_has_uncommitted_entries = false;
@@ -367,21 +367,21 @@ audit::handle_raft_commit(const raft_commit_notification& commit)
 {
     std::lock_guard<std::mutex> lock(this->audit_lock);
 
-    if(this->use_pbft)
+    if (this->use_pbft)
     {
         LOG(debug) << "audit ignoring raft commit message because we are in pbft mode";
         return;
     }
 
     this->send_to_monitor(bzn::RAFT_COMMIT_METRIC_NAME + bzn::STATSD_COUNTER_FORMAT);
 
-    if(this->recorded_raft_commits.count(commit.log_index()) == 0)
+    if (this->recorded_raft_commits.count(commit.log_index()) == 0)
     {
         LOG(info) << "audit recording that message '" << commit.operation() << "' is committed at index " << commit.log_index();
         this->recorded_raft_commits[commit.log_index()] = commit.operation();
         this->trim();
     }
-    else if(this->recorded_raft_commits[commit.log_index()] != commit.operation())
+    else if (this->recorded_raft_commits[commit.log_index()] != commit.operation())
     {
         std::string err = str(boost::format(
                 "Conflicting commit detected! '%1%' is the recorded entry at index %2%, but '%3%' has been committed with the same index.")
@@ -397,21 +397,21 @@ audit::handle_pbft_commit(const pbft_commit_notification& commit)
 {
     std::lock_guard<std::mutex> lock(this->audit_lock);
 
-    if(!this->use_pbft)
+    if (!this->use_pbft)
     {
         LOG(debug) << "audit ignoring pbft commit message because we are in raft mode";
         return;
     }
 
     this->send_to_monitor(bzn::PBFT_COMMIT_METRIC_NAME + bzn::STATSD_COUNTER_FORMAT);
 
-    if(this->recorded_pbft_commits.count(commit.sequence_number()) == 0)
+    if (this->recorded_pbft_commits.count(commit.sequence_number()) == 0)
     {
         LOG(info) << "audit recording that message '" << commit.operation() << "' is committed at sequence " << commit.sequence_number();
         this->recorded_pbft_commits[commit.sequence_number()] = commit.operation();
         this->trim();
     }
-    else if(this->recorded_pbft_commits[commit.sequence_number()] != commit.operation())
+    else if (this->recorded_pbft_commits[commit.sequence_number()] != commit.operation())
     {
         std::string err = str(boost::format(
                 "Conflicting commit detected! '%1%' is the recorded entry at sequence %2%, but '%3%' has been committed with the same sequence.")
@@ -428,7 +428,7 @@ audit::handle_failure_detected(const failure_detected& /*failure*/)
     // TODO KEP-539: more info in this message
     std::lock_guard<std::mutex> lock(this->audit_lock);
 
-    if(!this->use_pbft)
+    if (!this->use_pbft)
     {
         LOG(debug) << "audit ignoring pbft failure detected message because we are in raft mode";
         return;

bootstrap/bootstrap_peers.cpp

@@ -110,7 +110,7 @@ bootstrap_peers::initialize_peer_list(const Json::Value& root, bzn::peers_list_t
             // At this point we cannot validate uuids as we do not have
             // signatures for all of them, so we simply ignore blacklisted
             // uuids
-            if(bzn::utils::blacklist::is_blacklisted(uuid))
+            if (bzn::utils::blacklist::is_blacklisted(uuid))
             {
                 LOG(warning) << "Ignoring blacklisted node with uuid: [" << uuid << "]";
                 continue;

bootstrap/test/bootstrap_test.cpp

@@ -97,8 +97,8 @@ TEST_F(bootstrap_file_test, test_valid_peers)
 
     for (const bzn::peer_address_t& p : bootstrap_peers.get_peers())
     {
-        if(p.port == 12345) seen_peer1 = true;
-        if(p.port == 54321) seen_peer2 = true;
+        if (p.port == 12345) seen_peer1 = true;
+        if (p.port == 54321) seen_peer2 = true;
     }
 
     ASSERT_TRUE(seen_peer1 && seen_peer2);
@@ -116,8 +116,8 @@ TEST_F(bootstrap_file_test, test_unnamed_peers)
 
     for (const bzn::peer_address_t& p : bootstrap_peers.get_peers())
     {
-        if(p.name == "peer1") seen_name1 = true;
-        if(p.name == "unknown") seen_name2 = true;
+        if (p.name == "peer1") seen_name1 = true;
+        if (p.name == "unknown") seen_name2 = true;
     }
 
     ASSERT_TRUE(seen_name1 && seen_name2);

crud/crud.cpp

@@ -57,7 +57,7 @@ crud::start()
 
                     // This is to avoid trying to process encoded protobuf messages where we just want to commit this
                     // index, but not alter storage at all.
-                    if(!ws_msg["msg"].isString())
+                    if (!ws_msg["msg"].isString())
                     {
                         return true;
                     }

http/connection.cpp

@@ -91,7 +91,7 @@ connection::do_read_request()
     this->http_socket->async_read(this->buffer, this->request,
         [self = shared_from_this()](boost::beast::error_code ec, std::size_t /*bytes_transferred*/)
         {
-            if(!ec)
+            if (!ec)
             {
                 self->deadline_timer->cancel();
 

options/options.cpp

@@ -270,7 +270,7 @@ options::validate()
         return false;
     }
 
-    if(this->config_data.isMember(MONITOR_PORT_KEY) && this->config_data.isMember(MONITOR_ADDRESS_KEY))
+    if (this->config_data.isMember(MONITOR_PORT_KEY) && this->config_data.isMember(MONITOR_ADDRESS_KEY))
     {
         LOG(info) << "No monitor address provided; will not send monitor packets";
     }

options/test/options_test.cpp

@@ -154,7 +154,7 @@ TEST_F(options_file_test, test_max_storage_parsing)
                       {
                           std::string max_storage{boost::lexical_cast<std::string>(expected / p.second)};
                           max_storage = max_storage + p.first;
-                          if(p.first!='B')
+                          if (p.first!='B')
                           {
                               max_storage = max_storage.append("B");
                           }

pbft/pbft.cpp

@@ -110,7 +110,7 @@ pbft::start()
                         [weak_this = this->weak_from_this()]()
                         {
                             auto strong_this = weak_this.lock();
-                            if(strong_this)
+                            if (strong_this)
                             {
                                 strong_this->handle_failure();
                             }
@@ -165,7 +165,7 @@ pbft::handle_message(const pbft_msg& msg)
         return;
     }
 
-    if(msg.has_request())
+    if (msg.has_request())
     {
         this->failure_detector->request_seen(msg.request());
     }
@@ -443,7 +443,7 @@ pbft::wrap_message(const pbft_msg& msg, const std::string& debug_info)
 
     json["bzn-api"] = "pbft";
     json["pbft-data"] = boost::beast::detail::base64_encode(msg.SerializeAsString());
-    if(debug_info.length() > 0)
+    if (debug_info.length() > 0)
     {
         json["debug-info"] = debug_info;
     }
@@ -458,7 +458,7 @@ pbft::wrap_message(const audit_message& msg, const std::string& debug_info)
 
     json["bzn-api"] = "audit";
     json["audit-data"] = boost::beast::detail::base64_encode(msg.SerializeAsString());
-    if(debug_info.length() > 0)
+    if (debug_info.length() > 0)
     {
         json["debug-info"] = debug_info;
     }
@@ -481,7 +481,7 @@ pbft::set_audit_enabled(bool setting)
 void
 pbft::notify_audit_failure_detected()
 {
-    if(this->audit_enabled)
+    if (this->audit_enabled)
     {
         audit_message msg;
         msg.mutable_failure_detected()->set_sender_uuid(this->uuid);

pbft/pbft_failure_detector.cpp

@@ -53,7 +53,7 @@ pbft_failure_detector::handle_timeout(boost::system::error_code /*ec*/)
         this->ordered_requests.pop_front();
     }
 
-    if(this->ordered_requests.size() > 0)
+    if (this->ordered_requests.size() > 0)
     {
         this->start_timer();
     }
@@ -72,7 +72,7 @@ pbft_failure_detector::request_seen(const pbft_request& req)
         this->ordered_requests.emplace_back(req);
         this->outstanding_requests.emplace(req_hash);
 
-        if(this->ordered_requests.size() == 1)
+        if (this->ordered_requests.size() == 1)
         {
             this->start_timer();
         }

raft/raft.cpp

@@ -436,7 +436,7 @@ raft::validate_new_peer(std::shared_ptr<bzn::session_base> session, const bzn::m
     }
 
     const auto signature{peer["signature"].asString()};
-    if(!bzn::utils::crypto::verify_signature(bzn::utils::crypto::retrieve_bluzelle_public_key_from_contract(), signature, uuid))
+    if (!bzn::utils::crypto::verify_signature(bzn::utils::crypto::retrieve_bluzelle_public_key_from_contract(), signature, uuid))
     {
         this->send_session_error_message(session, ERROR_UNABLE_TO_VALIDATE_UUID);
         return false;