CPPCHECK improvements (#3004)

- enable uninitMemberVar and corresponding code fixes;
- set cmake options when CPPCHECK option is enabled;

.cppcheck/suppressions.txt

@@ -4,7 +4,6 @@
 # TODO [TK] - do we want enable cppckeck for tests?
 
 incorrectStringBooleanError
-uninitMemberVar
 noCopyConstructor
 noOperatorEq
 

.github/workflows/cppcheck.yml

@@ -22,5 +22,5 @@ jobs:
             mkdir build
             cd build
             echo $(date +%y-%m-%d_%H-%M-%S) > timestamp
-            cmake -DBUILD_TESTING=OFF -DBUILD_UTT=OFF -DUSE_LOG4CPP=OFF -DCPPCHECK=ON ..
+            cmake -DCPPCHECK=ON ..
             make -j$(nproc)

CMakeLists.txt

@@ -4,14 +4,15 @@ project(concord-bft VERSION 0.1.0.0 LANGUAGES CXX)
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
-set(SLEEP_FOR_DBG FALSE)
+
 # targets with generic names like "format" may already exist in an imported libs
 cmake_policy(SET CMP0002 OLD)
 set(ALLOW_DUPLICATE_CUSTOM_TARGETS TRUE)
+
 set(MIN_BOOST_VERSION 1.80)
 set(YAML_CPP_VERSION 0.7.0)
 
-# Default to debug builds
+# Defaults to debug builds
 # Release builds can be enabled by running cmake with -DCMAKE_BUILD_TYPE=Release
 if(NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "Enable debug or release builds" FORCE)
@@ -21,10 +22,7 @@ option(USE_LOG4CPP "Enable LOG4CPP" ON)
 option(RUN_APOLLO_TESTS "Enable Apollo tests run" ON)
 option(KEEP_APOLLO_LOGS "Retains logs from replicas in separate folder for each test in build/tests/apollo/logs" ON)
 option(TXN_SIGNING_ENABLED "Enable External concord client transcattion signing" ON)
-option(LEAKCHECK "Enable Address and Leak Sanitizers" OFF)
-option(HEAPTRACK "Enable Heaptrack - a heap memory profiler for Linux" OFF)
-option(THREADCHECK "Enable Thread Sanitizer" OFF)
-option(UNDEFINED_BEHAVIOR_CHECK "Enable Undefined Behavior Sanitizer" OFF)
+
 # Rocksdb is required for storage now. Consider removing this flag.
 option(BUILD_ROCKSDB_STORAGE "Enable building of RocksDB storage library" ON)
 option(USE_S3_OBJECT_STORE "Enable S3 Object Store" ON)
@@ -45,20 +43,14 @@ if((USE_OPENSSL) AND NOT BUILD_THIRDPARTY)
     set(OPENSSL_ROOT_DIR /usr/local/ssl) # not to confuse with system ssl libs
 endif()
 
-if(SLEEP_FOR_DBG)
-    add_definitions(-DSLEEP_DBG)
-endif()
-
 # include compiler specific options
 include(cmake/${CMAKE_CXX_COMPILER_ID}.cmake)
 include(cmake/cppcheck.cmake)
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake")
 include(cmake/grpc_utils.cmake)
 
-if(BUILD_TESTING)
-	include(CTest)
-endif()
+include(CTest)
 
 if(USE_S3_OBJECT_STORE)
     add_compile_definitions(USE_S3_OBJECT_STORE=1)

bftengine/include/bftengine/EpochManager.hpp

@@ -20,7 +20,7 @@
 namespace bftEngine {
 class EpochManager : public bftEngine::ResPagesClient<EpochManager, 1> {
   struct EpochData : public concord::serialize::SerializableFactory<EpochData> {
-    uint64_t epochNumber_;
+    uint64_t epochNumber_ = 0;
     EpochData() = default;
     EpochData(uint64_t epochNumber) : epochNumber_{epochNumber} {}
 
@@ -79,4 +79,4 @@ class EpochManager : public bftEngine::ResPagesClient<EpochManager, 1> {
   concordMetrics::Component metrics_;
   concordMetrics::GaugeHandle epoch_number_gauge_;
 };
-}  // namespace bftEngine
+}  // namespace bftEngine

bftengine/src/bftengine/DebugStatistics.hpp

@@ -41,21 +41,21 @@ class DebugStatistics {
 
  private:
   struct DebugStatDesc {
-    bool initialized;
+    bool initialized = false;
     Time lastCycleTime;
 
-    size_t receivedMessages;
-    std::atomic<size_t> sendMessages;
-    size_t completedReadOnlyRequests;
-    size_t completedReadWriteRequests;
-    size_t numberOfReceivedSTMessages;
-    size_t numberOfReceivedStatusMessages;
-    size_t numberOfReceivedCommitMessages;
-    int64_t lastExecutedSequenceNumber;
-
-    size_t prePrepareMessages;
-    size_t batchRequests;
-    size_t pendingRequests;
+    size_t receivedMessages = 0;
+    std::atomic<size_t> sendMessages = 0;
+    size_t completedReadOnlyRequests = 0;
+    size_t completedReadWriteRequests = 0;
+    size_t numberOfReceivedSTMessages = 0;
+    size_t numberOfReceivedStatusMessages = 0;
+    size_t numberOfReceivedCommitMessages = 0;
+    int64_t lastExecutedSequenceNumber = 0;
+
+    size_t prePrepareMessages = 0;
+    size_t batchRequests = 0;
+    size_t pendingRequests = 0;
 
     DebugStatDesc() : initialized(false) {}
   };

bftengine/src/bftengine/ReplicaSpecificInfoManager.hpp

@@ -34,8 +34,8 @@ class RsiItem {
 
  private:
   std::string data_;
-  uint64_t index_;
-  uint64_t req_seq_num_;
+  uint64_t index_ = 0;
+  uint64_t req_seq_num_ = 0;
 };
 
 class RsiDataManager {
@@ -58,4 +58,4 @@ class RsiDataManager {
   std::unordered_map<uint32_t, uint64_t> clientsIndex_;
 };
 }  // namespace impl
-}  // namespace bftEngine
+}  // namespace bftEngine

bftengine/src/bftengine/RetransmissionsManager.cpp

@@ -169,8 +169,8 @@ class RetransmissionsLogic {
     SeqNum seqNumber = 0;
 
     bool ackOrAbort = false;
-    uint16_t numOfTransmissions;  // valid only if ackOrAbort==false
-    Time timeOfTransmission;      // valid only if ackOrAbort==false
+    uint16_t numOfTransmissions = 0;  // valid only if ackOrAbort==false
+    Time timeOfTransmission;          // valid only if ackOrAbort==false
   };
 
   class ReplicaInfo {

bftengine/src/preprocessor/messages/PreProcessResultMsg.hpp

@@ -63,7 +63,7 @@ class PreProcessResultMsg : public ClientRequestMsg {
 struct PreProcessResultSignature {
   std::vector<concord::Byte> signature;
   NodeIdType sender_replica;
-  bftEngine::OperationResult pre_process_result;
+  bftEngine::OperationResult pre_process_result = bftEngine::OperationResult::UNKNOWN;
 
   PreProcessResultSignature() = default;
 

client/bftclient/include/bftclient/fake_comm.h

@@ -78,7 +78,7 @@ class BehaviorThreadRunner {
   void stop() { stop_ = true; }
 
  private:
-  IReceiver* receiver_;
+  IReceiver* receiver_ = nullptr;
   std::atomic<bool> stop_ = false;
 
   Behavior behavior_;
@@ -131,7 +131,7 @@ class FakeCommunication : public bft::communication::ICommunication {
   void restartCommunication(NodeNum i) override {}
 
  private:
-  IReceiver* receiver_;
+  IReceiver* receiver_ = nullptr;
   BehaviorThreadRunner runner_;
   std::thread fakeCommThread_;
 };

cmake/cppcheck.cmake

@@ -1,7 +1,10 @@
 option(CPPCHECK "Perform cppcheck" OFF)
 
 if(CPPCHECK)
-	message(STATUS "CPPCHECK is ON")
+    set(BUILD_TESTING OFF) 
+    set(BUILD_UTT OFF)
+    set(USE_LOG4CPP OFF)	
+    message(STATUS "CPPCHECK is ON")
     find_program(cppcheck cppcheck HINTS "/usr/local/bin/cppcheck" REQUIRED)
     message(STATUS "cppcheck ${cppcheck}")
     # Create <cppcheck> work folder for whole program analysis, for faster analysis and to store some useful debug information

kvbc/include/KVBCInterfaces.h

@@ -33,17 +33,17 @@ class ICommandsHandler;
 
 struct ClientConfig {
   // F value - max number of faulty/malicious replicas. fVal >= 1
-  uint16_t fVal;
+  uint16_t fVal = 0;
 
   // C value. cVal >=0
-  uint16_t cVal;
+  uint16_t cVal = 0;
 
   // unique identifier of the client.
   // clientId should also represent this client in ICommunication.
   // In the current version, replicaId should be a number between N and
   // N+numOfClientProxies-1 (N is the number replicas in the system.
   // numOfClientProxies is part of the replicas' configuration)
-  uint16_t clientId;
+  uint16_t clientId = 0;
 };
 
 /////////////////////////////////////////////////////////////////////////////

libs/util/throughput.hpp

@@ -191,7 +191,7 @@ class Throughput {
   Stats overall_stats_;
   Stats current_window_stats_;
   Stats previous_window_stats_;
-  uint64_t previous_window_index_;
+  uint64_t previous_window_index_ = 0;
   uint64_t reports_counter_ = 0;
   std::string name_;
 };  // class Throughput