[ci] Add cppcheck github action

Signed-off-by: czm <[email protected]> [ci] Add cpplint github action Signed-off-by: czm <[email protected]> [ci] Add sanitizers for unit tests Signed-off-by: czm <[email protected]>
wu-hanqing · Oct 10, 2023 · 8a07e66 · 8a07e66
1 parent a63a8f6
commit 8a07e66
Show file tree

Hide file tree

Showing 30 changed files with 185 additions and 72 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -7,4 +7,20 @@ build:gcc7-later --cxxopt -faligned-new
 build --incompatible_blacklisted_protos_requires_proto_info=false
 build --copt=-fdiagnostics-color=always
 
+build:sanitize-common --strip=never
+build:sanitize-common --copt -O1
+build:sanitize-common --copt -g
+build:sanitize-common --copt -fno-omit-frame-pointer
+
+build:asan --config=sanitize-common
+build:asan --copt -fsanitize=address
+build:asan --copt -DADDRESS_SANITIZER
+build:asan --linkopt -fsanitize=address
+
+build:asan --config=sanitize-common
+build:msan --copt -fsanitize=memory
+build:msan --copt -fsanitize=undefined
+build:msan --linkopt -fsanitize=address
+build:msan --linkopt -fsanitize=undefined
+
 run --copt=-fdiagnostics-color=always
diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml
@@ -0,0 +1,24 @@
+name: Static Checker
+
+on: pull_request
+
+jobs:
+  cppcheck:
+    name: CppCheck
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: chmorgan/[email protected]
+        with:
+          enable: all
+          std: c++11
+          inconclusive: disable
+          output_file: ./cppcheck_report.txt
+          other_options: "-j4 --suppressions-list=util/cppcheck/cppcheck.suppressions --error-exitcode=1 -itest -icurvefs/test -inebd/test -inbd/test -icurvefs_python -icurvesnapshot_python -ithirdparties"
+
+      - name: Show cppcheck report
+        if: failure()
+        run: |
+          cat ./cppcheck_report.txt
+          exit 1
diff --git a/Makefile b/Makefile
@@ -66,7 +66,7 @@ dep:
 	@bash util/build.sh --stor=$(stor) --only="" --dep=1
 
 ci-build:
-	@bash util/build_in_image.sh --stor=$(stor) --only=$(only) --dep=$(dep) --release=$(release) --ci=$(ci) --os=$(os)
+	@bash util/build_in_image.sh --stor=$(stor) --only=$(only) --dep=$(dep) --release=$(release) --ci=$(ci) --os=$(os) --sanitizer=$(sanitizer)
 
 ci-dep:
 	@bash util/build_in_image.sh --stor=$(stor) --only="" --dep=1

diff --git a/curvefs/src/client/filesystem/meta.cpp b/curvefs/src/client/filesystem/meta.cpp
@@ -106,7 +106,6 @@ std::string Attr2Str(const InodeAttr& attr) {
         return "";
     }
 
-    std::string smode;
     return absl::StrFormat(" (%d,[%s:0%06o,%d,%d,%d,%d,%d,%d,%d])",
         attr.inodeid(), StrMode(attr.mode()).c_str(), attr.mode(), attr.nlink(),
         attr.uid(), attr.gid(),

diff --git a/curvefs/src/client/s3/client_s3_cache_manager.cpp b/curvefs/src/client/s3/client_s3_cache_manager.cpp
@@ -1039,7 +1039,8 @@ void FileCacheManager::ReleaseCache() {
     }
 
     chunkCacheMap_.clear();
-    g_s3MultiManagerMetric->chunkManagerNum << -1 * chunNum;
+    g_s3MultiManagerMetric->chunkManagerNum
+        << -1 * static_cast<int64_t>(chunNum);
     return;
 }
 
@@ -1509,7 +1510,7 @@ DataCachePtr ChunkCacheManager::FindWriteableDataCache(
             }
 
             std::vector<uint64_t>::iterator iterDel = waitDelVec.begin();
-            for (; iterDel != waitDelVec.end(); iterDel++) {
+            for (; iterDel != waitDelVec.end(); ++iterDel) {
                 auto iter = dataWCacheMap_.find(*iterDel);
                 VLOG(9) << "delete data cache chunkPos:"
                         << iter->second->GetChunkPos()
@@ -1576,7 +1577,8 @@ void ChunkCacheManager::AddReadDataCache(DataCachePtr dataCache) {
         uint64_t actualLen = (*dcpIter)->GetActualLen();
         if (s3ClientAdaptor_->GetFsCacheManager()->Delete(dcpIter)) {
             g_s3MultiManagerMetric->readDataCacheNum << -1;
-            g_s3MultiManagerMetric->readDataCacheByte << -1 * actualLen;
+            g_s3MultiManagerMetric->readDataCacheByte
+                << -1 * static_cast<int64_t>(actualLen);
             dataRCacheMap_.erase(iter);
         }
     }
@@ -1669,7 +1671,8 @@ void ChunkCacheManager::TruncateReadCache(uint64_t chunkPos) {
         if ((dcChunkPos + dcLen) > chunkPos) {
             if (s3ClientAdaptor_->GetFsCacheManager()->Delete(rIter->second)) {
                 g_s3MultiManagerMetric->readDataCacheNum << -1;
-                g_s3MultiManagerMetric->readDataCacheByte << -1 * dcActualLen;
+                g_s3MultiManagerMetric->readDataCacheByte
+                    << -1 * static_cast<int64_t>(dcActualLen);
                 dataRCacheMap_.erase(next(rIter).base());
             }
         } else {
@@ -1691,7 +1694,6 @@ void ChunkCacheManager::ReleaseWriteDataCache(const DataCachePtr &dataCache) {
 
 CURVEFS_ERROR ChunkCacheManager::Flush(uint64_t inodeId, bool force,
                                        bool toS3) {
-    std::map<uint64_t, DataCachePtr> tmp;
     curve::common::LockGuard lg(flushMtx_);
     CURVEFS_ERROR ret = CURVEFS_ERROR::OK;
     // DataCachePtr dataCache;
@@ -2076,7 +2078,7 @@ void DataCache::Write(uint64_t chunkPos, uint64_t len, const char *data,
         } else {
             std::vector<DataCachePtr>::const_iterator iter =
                 mergeDataCacheVer.begin();
-            for (; iter != mergeDataCacheVer.end(); iter++) {
+            for (; iter != mergeDataCacheVer.end(); ++iter) {
                 /*
                      ------         ------    DataCache
                   ---------------------       WriteData
@@ -2126,7 +2128,7 @@ void DataCache::Write(uint64_t chunkPos, uint64_t len, const char *data,
         } else {
             std::vector<DataCachePtr>::const_iterator iter =
                 mergeDataCacheVer.begin();
-            for (; iter != mergeDataCacheVer.end(); iter++) {
+            for (; iter != mergeDataCacheVer.end(); ++iter) {
                 /*
                      ------         ------    DataCache
                         ----------------       WriteData

diff --git a/curvefs/src/client/s3/disk_cache_read.cpp b/curvefs/src/client/s3/disk_cache_read.cpp
@@ -85,7 +85,7 @@ int DiskCacheRead::LinkWriteToRead(const std::string fileName,
                                    const std::string fullWriteDir,
                                    const std::string fullReadDir) {
     VLOG(6) << "LinkWriteToRead start. name = " << fileName;
-    std::string fullReadPath, fullWritePath, dirPath;
+    std::string fullReadPath, fullWritePath;
     fullWritePath = fullWriteDir + "/" + fileName;
     fullReadPath = fullReadDir + "/" + fileName;
     int ret;

diff --git a/curvefs/src/client/s3/disk_cache_write.cpp b/curvefs/src/client/s3/disk_cache_write.cpp
@@ -176,7 +176,7 @@ int DiskCacheWrite::UploadFile(const std::string &name,
 void DiskCacheWrite::UploadFile(const std::list<std::string> &toUpload,
                                 std::shared_ptr<SynchronizationTask> syncTask) {
     std::list<std::string>::const_iterator iter;
-    for (iter = toUpload.begin(); iter != toUpload.end(); iter++) {
+    for (iter = toUpload.begin(); iter != toUpload.end(); ++iter) {
         UploadFile(*iter, syncTask);
     }
 }

diff --git a/curvefs/src/client/warmup/warmup_manager.cpp b/curvefs/src/client/warmup/warmup_manager.cpp
@@ -562,7 +562,6 @@ void WarmupManagerS3Impl::ScanCleanFetchDentryPool() {
     WriteLockGuard lock(inode2FetchDentryPoolMutex_);
     for (auto iter = inode2FetchDentryPool_.begin();
          iter != inode2FetchDentryPool_.end();) {
-        std::deque<WarmupInodes>::iterator iterInode;
         if (iter->second->QueueSize() == 0) {
             VLOG(9) << "remove FetchDentry task: " << iter->first;
             iter->second->Stop();

diff --git a/curvefs/src/client/xattr_manager.cpp b/curvefs/src/client/xattr_manager.cpp
@@ -46,8 +46,7 @@ bool IsOneLayer(const char *name) {
     return false;
 }
 
-CURVEFS_ERROR XattrManager::CalOneLayerSumInfo(InodeAttr *attr) {
-    std::stack<uint64_t> iStack;
+CURVEFS_ERROR XattrManager::CalOneLayerSumInfo(InodeAttr* attr) {
     // use set can deal with hard link
     std::set<uint64_t> inodeIds;
     std::list<InodeAttr> attrs;

diff --git a/curvefs/src/mds/schedule/scheduler.cpp b/curvefs/src/mds/schedule/scheduler.cpp
@@ -79,7 +79,7 @@ MetaServerIdType Scheduler::SelectBestPlacementMetaServer(
     }
 
     uint16_t standardZoneNum = topo_->GetStandardZoneNumInPool(poolId);
-    if (standardZoneNum <= 0) {
+    if (standardZoneNum == 0) {
         LOG(ERROR) << "topoAdapter find pool " << poolId
                    << " standard zone num: " << standardZoneNum << " invalid";
         return UNINITIALIZE_ID;

diff --git a/curvefs/src/metaserver/s3compact_inode.cpp b/curvefs/src/metaserver/s3compact_inode.cpp
@@ -123,7 +123,7 @@ CompactInodeJob::BuildValidList(const S3ChunkInfoList& s3chunkinfolist,
             if (begin <= b) {
                 if (end < b) {
                     return;
-                } else if (end >= b && end < e) {
+                } else if (end < e) {
                     // free [it->begin, it->end] -> [end+1, it->end]
                     // used [it->begin, end]
                     *it = std::make_pair(end + 1, e);
@@ -134,7 +134,7 @@ CompactInodeJob::BuildValidList(const S3ChunkInfoList& s3chunkinfolist,
                     freeList.erase(it);
                     used[b] = std::make_pair(e, i);
                 }
-            } else if (begin > b && begin <= e) {
+            } else if (begin <= e) {
                 if (end < e) {
                     // free [it-begin, it->end]
                     // -> [it->begin, begin-1], [end+1, it->end]
@@ -579,7 +579,6 @@ void CompactInodeJob::CompactChunks(const S3CompactTask& task) {
     std::unordered_map<uint64_t, std::vector<std::string>> objsAddedMap;
     ::google::protobuf::Map<uint64_t, S3ChunkInfoList> s3ChunkInfoAdd;
     ::google::protobuf::Map<uint64_t, S3ChunkInfoList> s3ChunkInfoRemove;
-    std::vector<uint64_t> indexToDelete;
     VLOG(6) << "s3compact: begin to compact fsId:" << fsId
             << ", inodeId:" << inodeId;
     for (const auto& index : needCompact) {

diff --git a/curvefs/src/tools/create/curvefs_create_topology_tool.cpp b/curvefs/src/tools/create/curvefs_create_topology_tool.cpp
@@ -282,7 +282,7 @@ int CurvefsBuildTopologyTool::ScanCluster() {
     for (auto it = poolInfos.begin(); it != poolInfos.end(); it++) {
         auto ix = std::find_if(
             poolDatas.begin(), poolDatas.end(),
-            [it](Pool& data) { return data.name == it->poolname(); });
+            [it](const Pool& data) { return data.name == it->poolname(); });
         if (ix != poolDatas.end()) {
             poolDatas.erase(ix);
         } else {
@@ -301,8 +301,8 @@ int CurvefsBuildTopologyTool::ScanCluster() {
     }
 
     for (auto it = zoneInfos.begin(); it != zoneInfos.end(); it++) {
-        auto ix =
-            std::find_if(zoneDatas.begin(), zoneDatas.end(), [it](Zone& data) {
+        auto ix = std::find_if(
+            zoneDatas.begin(), zoneDatas.end(), [it](const Zone& data) {
                 return (data.poolName == it->poolname()) &&
                        (data.name == it->zonename());
             });
@@ -325,7 +325,7 @@ int CurvefsBuildTopologyTool::ScanCluster() {
 
     for (auto it = serverInfos.begin(); it != serverInfos.end(); it++) {
         auto ix = std::find_if(serverDatas.begin(), serverDatas.end(),
-                               [it](Server& data) {
+                               [it](const Server& data) {
                                    return (data.name == it->hostname()) &&
                                           (data.zoneName == it->zonename()) &&
                                           (data.poolName == it->poolname());

diff --git a/curvefs/src/volume/free_extents.cpp b/curvefs/src/volume/free_extents.cpp
@@ -290,7 +290,7 @@ void FreeExtents::MarkUsedInternal(const uint64_t off, const uint64_t len) {
 
     auto iter = extents_.lower_bound(off);
     if (iter != extents_.end() && iter->first == off) {
-        if (iter->first == off && iter->second == len) {
+        if (iter->second == len) {
             extents_.erase(iter);
         } else {
             auto newOff = iter->first + len;

diff --git a/nebd/src/common/configuration.cpp b/nebd/src/common/configuration.cpp
@@ -39,8 +39,7 @@ bool Configuration::LoadConfig() {
             // FIXME: may not remove middle spaces
             line.erase(std::remove_if(line.begin(), line.end(), isspace),
                        line.end());
-            if (line[0] == '#' || line.empty())
-                continue;
+            if (line.empty() || line[0] == '#') continue;
 
             int delimiterPos = line.find("=");
             std::string key = line.substr(0, delimiterPos);

diff --git a/nebd/src/part2/metafile_manager.cpp b/nebd/src/part2/metafile_manager.cpp
@@ -205,7 +205,6 @@ int NebdMetaFileParser::Parse(Json::Value root,
     }
 
     for (const auto& volume : volumes) {
-        std::string fileName;
         NebdFileMeta meta;
 
         if (volume[kFileName].isNull()) {

diff --git a/src/chunkserver/copyset_node_manager.cpp b/src/chunkserver/copyset_node_manager.cpp
@@ -481,12 +481,13 @@ bool CopysetNodeManager::PurgeCopysetNodeData(const LogicPoolID &logicPoolId,
                            << ToGroupIdString(logicPoolId, copysetId)
                            << " persistently.";
                 ret = false;
+            } else {
+                LOG(INFO) << "Move copyset"
+                          << ToGroupIdString(logicPoolId, copysetId)
+                          << "to trash success.";
+                copysetNodeMap_.erase(it);
+                ret = true;
             }
-            LOG(INFO) << "Move copyset"
-                      << ToGroupIdString(logicPoolId, copysetId)
-                      << "to trash success.";
-            copysetNodeMap_.erase(it);
-            ret = true;
         }
     }
 

diff --git a/src/client/client_config.cpp b/src/client/client_config.cpp
@@ -274,24 +274,24 @@ int ClientConfig::Init(const std::string& configpath) {
     // only client side need these follow 5 options
     ret = conf_.GetUInt32Value("csClientOpt.rpcTimeoutMs",
         &fileServiceOption_.csClientOpt.rpcTimeoutMs);
-    LOG_IF(WARNING, ret = false) << "config no csClientOpt.rpcTimeoutMs info";
+    LOG_IF(WARNING, ret == false) << "config no csClientOpt.rpcTimeoutMs info";
 
     ret = conf_.GetUInt32Value("csClientOpt.rpcMaxTry",
         &fileServiceOption_.csClientOpt.rpcMaxTry);
-    LOG_IF(WARNING, ret = false) << "config no csClientOpt.rpcMaxTry info";
+    LOG_IF(WARNING, ret == false) << "config no csClientOpt.rpcMaxTry info";
 
     ret = conf_.GetUInt32Value("csClientOpt.rpcIntervalUs",
         &fileServiceOption_.csClientOpt.rpcIntervalUs);
-    LOG_IF(WARNING, ret = false) << "config no csClientOpt.rpcIntervalUs info";
+    LOG_IF(WARNING, ret == false) << "config no csClientOpt.rpcIntervalUs info";
 
     ret = conf_.GetUInt32Value("csClientOpt.rpcMaxTimeoutMs",
         &fileServiceOption_.csClientOpt.rpcIntervalUs);
-    LOG_IF(WARNING, ret = false)
+    LOG_IF(WARNING, ret == false)
         << "config no csClientOpt.rpcMaxTimeoutMs info";
 
     ret = conf_.GetUInt32Value("csBroadCasterOpt.broadCastMaxNum",
         &fileServiceOption_.csBroadCasterOpt.broadCastMaxNum);
-    LOG_IF(WARNING, ret = false)
+    LOG_IF(WARNING, ret == false)
         << "config no csBroadCasterOpt.broadCastMaxNum info";
 
     return 0;

diff --git a/src/common/configuration.cpp b/src/common/configuration.cpp
@@ -41,8 +41,7 @@ bool Configuration::LoadConfig() {
             // FIXME: may not remove middle spaces
             line.erase(std::remove_if(line.begin(), line.end(), isspace),
                        line.end());
-            if (line[0] == '#' || line.empty())
-                continue;
+            if (line.empty() || line[0] == '#') continue;
 
             int delimiterPos = line.find("=");
             std::string key = line.substr(0, delimiterPos);

diff --git a/src/mds/schedule/leaderScheduler.cpp b/src/mds/schedule/leaderScheduler.cpp
@@ -157,7 +157,7 @@ bool LeaderScheduler::transferLeaderOut(ChunkServerIdType source, int count,
         candidateInfos.emplace_back(cInfo);
     }
 
-    if (candidateInfos.size() <= 0) {
+    if (candidateInfos.size() == 0) {
         return false;
     }
 
@@ -241,7 +241,7 @@ bool LeaderScheduler::transferLeaderIn(ChunkServerIdType target, int count,
         }
     }
 
-    if (candidateInfos.size() <= 0) {
+    if (candidateInfos.size() == 0) {
         return false;
     }
 

diff --git a/src/mds/schedule/recoverScheduler.cpp b/src/mds/schedule/recoverScheduler.cpp
@@ -100,7 +100,7 @@ int RecoverScheduler::Schedule() {
             }
         }
 
-        if (offlinelists.size() <= 0) {
+        if (offlinelists.size() == 0) {
             continue;
         }
 
@@ -206,7 +206,7 @@ void RecoverScheduler::CalculateExcludesChunkServer(
             continue;
         }
 
-        if (unhealthyStateCS.count(cs.info.serverId) <= 0) {
+        if (unhealthyStateCS.count(cs.info.serverId) == 0) {
             unhealthyStateCS[cs.info.serverId] =
                 std::vector<ChunkServerIdType>{cs.info.id};
         } else {

diff --git a/src/mds/schedule/scheduler.cpp b/src/mds/schedule/scheduler.cpp
@@ -146,7 +146,6 @@ ChunkServerIdType Scheduler::SelectBestPlacementChunkServer(
         }
 
         // calculate the influence on scatter-width of other replicas
-        std::map<ChunkServerIdType, std::pair<int, int>> out;
         int source = UNINTIALIZE_ID;
         int target = cs.info.id;
         int affected = 0;

diff --git a/src/mds/schedule/scheduler_helper.cpp b/src/mds/schedule/scheduler_helper.cpp
@@ -133,7 +133,7 @@ bool SchedulerHelper::SatisfyZoneAndScatterWidthLimit(
         }
     }
 
-    if (zoneList.count(targetZone) <= 0) {
+    if (zoneList.count(targetZone) == 0) {
         zoneList[targetZone] = 1;
     } else {
         zoneList[targetZone] += 1;
@@ -246,15 +246,13 @@ void SchedulerHelper::CalculateAffectOfMigration(
     std::map<ChunkServerIdType, std::pair<int, int>> *scatterWidth) {
     // get scatter-width map and scatter-width of target
     std::map<ChunkServerIdType, int> targetMap;
-    std::pair<int, int> targetScatterWidth;
     if (target != UNINTIALIZE_ID) {
         topo->GetChunkServerScatterMap(target, &targetMap);
         (*scatterWidth)[target].first = targetMap.size();
     }
 
     // get scatter-width map and scatter-width of the source
     std::map<ChunkServerIdType, int> sourceMap;
-    std::pair<int, int> sourceScatterWidth;
     if (source != UNINTIALIZE_ID) {
         topo->GetChunkServerScatterMap(source, &sourceMap);
         (*scatterWidth)[source].first = sourceMap.size();
@@ -272,14 +270,14 @@ void SchedulerHelper::CalculateAffectOfMigration(
         // if target was initialized
         if (target != UNINTIALIZE_ID) {
             // influence on target
-            if (targetMap.count(peer.id) <= 0) {
+            if (targetMap.count(peer.id) == 0) {
                 targetMap[peer.id] = 1;
             } else {
                 targetMap[peer.id]++;
             }
 
             // target's influence on other chunkservers
-            if (tmpMap.count(target) <= 0) {
+            if (tmpMap.count(target) == 0) {
                 tmpMap[target] = 1;
             } else {
                 tmpMap[target]++;