From e3d64c1c96b57a9a488112fef84622cb149be3db Mon Sep 17 00:00:00 2001 From: wenlong12 Date: Thu, 10 Feb 2022 17:18:30 +0800 Subject: [PATCH] Fix codechek issue Signed-off-by:wenlong12 Signed-off-by: wenlong12 --- OAT.xml | 3 +- demo/cpp/hiperf_example_cmd.cpp | 6 +- include/hashlist.hpp | 185 ++++++------------ include/perf_event_record.h | 9 +- include/perf_events.h | 2 +- include/perf_record_format.h | 2 +- include/report.h | 6 +- include/report_protobuf_file.h | 2 +- include/subcommand_list.h | 4 +- include/subcommand_report.h | 2 +- .../innerkits/native/src/hiperf_client.cpp | 8 +- src/elf_symbol.cpp | 2 +- src/perf_event_record.cpp | 4 +- src/perf_events.cpp | 2 +- src/perf_file_writer.cpp | 6 +- src/report.cpp | 36 +++- src/report_json_file.cpp | 16 +- src/report_protobuf_file.cpp | 16 +- src/subcommand_record.cpp | 2 +- src/subcommand_report.cpp | 4 +- src/subcommand_stat.cpp | 6 +- src/symbols_file.cpp | 7 +- src/utilities.cpp | 8 +- .../PerfFile_fuzzer/PerfFile_fuzzer.cpp | 6 +- .../common/native/debug_logger_test.cpp | 1 - .../common/native/include/callstack_test.h | 6 +- .../common/native/option_debug_test.cpp | 4 +- .../common/native/symbols_file_test.cpp | 8 +- .../unittest/common/native/utilities_test.cpp | 3 +- 29 files changed, 173 insertions(+), 193 deletions(-) diff --git a/OAT.xml b/OAT.xml index 9941f55..edb24d2 100755 --- a/OAT.xml +++ b/OAT.xml @@ -71,6 +71,8 @@ Note:If the text contains special characters, please escape them according to th + + @@ -110,7 +112,6 @@ Note:If the text contains special characters, please escape them according to th - diff --git a/demo/cpp/hiperf_example_cmd.cpp b/demo/cpp/hiperf_example_cmd.cpp index 1ab9c16..3129324 100755 --- a/demo/cpp/hiperf_example_cmd.cpp +++ b/demo/cpp/hiperf_example_cmd.cpp @@ -108,7 +108,11 @@ USED_FUNCTION void LoopBranch() USED_FUNCTION void LoopIowait() { std::default_random_engine rnd; - std::unique_ptr fd {fopen("temp", "rw"), &fclose}; + FILE *fp = fopen("temp", "rw"); + if (fp == nullptr) { + return; + } + std::unique_ptr fd {fp, &fclose}; if (fd != nullptr) { const std::string tempBuf = std::to_string(rnd()); fwrite(tempBuf.c_str(), tempBuf.size(), 1, fd.get()); diff --git a/include/hashlist.hpp b/include/hashlist.hpp index 20e4d06..65fa703 100755 --- a/include/hashlist.hpp +++ b/include/hashlist.hpp @@ -32,22 +32,21 @@ template LinkNode::LinkNode(const Key &key, Val &&val) : key_ {key}, val_ {std::move(val)} {} template -LinkNode::LinkNode(const LinkNode& node) +LinkNode::LinkNode(const LinkNode& node) :link_ {node.link_}, key_ {node.key_}, val_ {node.val_} {} template -LinkNode::LinkNode(LinkNode&& node) +LinkNode::LinkNode(LinkNode&& node) :link_ {std::move(node.link_)}, key_ {std::move(node.key_)}, val_ {std::move(node.val_)} {} template -auto -LinkNode::operator=(const LinkNode& node) +auto LinkNode::operator=(const LinkNode& node) -> LinkNode& { link_ = node.link_; @@ -56,8 +55,7 @@ LinkNode::operator=(const LinkNode& node) } template -auto -LinkNode::operator=(LinkNode&& node) +auto LinkNode::operator=(LinkNode&& node) -> LinkNode& { link_ = std::move(node.link_); @@ -66,8 +64,7 @@ LinkNode::operator=(LinkNode&& node) } template -auto -LinkNode::GetLinkNode(Val *pval) +auto LinkNode::GetLinkNode(Val *pval) -> LinkNode* { if (pval) { @@ -80,8 +77,7 @@ LinkNode::GetLinkNode(Val *pval) } template -auto -LinkNode::GetLinkNode(Link *plink) +auto LinkNode::GetLinkNode(Link *plink) -> LinkNode* { if (plink) { @@ -128,8 +124,7 @@ HashList::Iterator::Iterator(Iterator&& itr) } template -auto -HashList::Iterator::operator=(const Iterator& itr) +auto HashList::Iterator::operator=(const Iterator& itr) -> HashList::Iterator& { Iterator temp {itr}; @@ -138,8 +133,7 @@ HashList::Iterator::operator=(const Iterator& itr) } template -auto -HashList::Iterator::operator=(Iterator&& itr) +auto HashList::Iterator::operator=(Iterator&& itr) -> HashList::Iterator& { Iterator temp {std::move(itr)}; @@ -148,8 +142,7 @@ HashList::Iterator::operator=(Iterator&& itr) } template -auto -HashList::Iterator::operator++() noexcept +auto HashList::Iterator::operator++() noexcept -> HashList::Iterator & { if (pnode_ == nullptr or phashList_ == nullptr) { @@ -167,8 +160,7 @@ HashList::Iterator::operator++() noexcept } template -auto -HashList::Iterator::operator++(int) noexcept +auto HashList::Iterator::operator++(int) noexcept -> HashList::Iterator { Iterator res {*this}; @@ -187,8 +179,7 @@ HashList::Iterator::operator++(int) noexcept } template -auto -HashList::Iterator::operator--() noexcept +auto HashList::Iterator::operator--() noexcept -> HashList::Iterator & { if (phashList_ == nullptr) { @@ -204,14 +195,13 @@ HashList::Iterator::operator--() noexcept pnode_ = nullptr; phashList_ = nullptr; return *this; - } + } pnode_ = LinkNode::GetLinkNode(plink); return *this; } template -auto -HashList::Iterator::operator--(int) noexcept +auto HashList::Iterator::operator--(int) noexcept -> HashList::Iterator { Iterator res {*this}; @@ -234,8 +224,7 @@ HashList::Iterator::operator--(int) noexcept } template -bool -HashList::Iterator::operator<(const HashList::Iterator &itr) const noexcept +bool HashList::Iterator::operator<(const HashList::Iterator &itr) const noexcept { if (IsDangled() or itr.IsDangled()) { return false; @@ -257,8 +246,7 @@ HashList::Iterator::operator<(const HashList::Iterator &itr) } template -bool -HashList::Iterator::operator==(const HashList::Iterator &itr) const noexcept +bool HashList::Iterator::operator==(const HashList::Iterator &itr) const noexcept { if (IsDangled() or itr.IsDangled()) { return false; @@ -270,36 +258,31 @@ HashList::Iterator::operator==(const HashList::Iterator &itr } template -Val & -HashList::Iterator::operator*() +Val& HashList::Iterator::operator*() { return pnode_->val_; } template -const Val & -HashList::Iterator::operator*() const +const Val& HashList::Iterator::operator*() const { return pnode_->val_; } template -Val * -HashList::Iterator::operator->() +Val* HashList::Iterator::operator->() { return &pnode_->val_; } template -const Val * -HashList::Iterator::operator->() const +const Val* HashList::Iterator::operator->() const { return &pnode_->val_; } template -void -HashList::Iterator::swap(HashList::Iterator& other) +void HashList::Iterator::swap(HashList::Iterator& other) { using std::swap; swap(pnode_, other.pnode_); @@ -319,8 +302,8 @@ HashList::ReverseIterator::ReverseIterator(LinkNode *pnode, template HashList::ReverseIterator::ReverseIterator(const LinkNode *pnode, const HashList *phashList) - : pnode_ {const_cast *>(pnode)}, - phashList_ {const_cast(phashList)} + : pnode_ {const_cast *>(pnode)}, + phashList_ {const_cast(phashList)} { if (phashList_ == nullptr) { pnode_ = nullptr; @@ -341,8 +324,7 @@ HashList::ReverseIterator::ReverseIterator(ReverseIterator &&itr) } template -auto -HashList::ReverseIterator::operator=(const ReverseIterator& itr) +auto HashList::ReverseIterator::operator=(const ReverseIterator& itr) -> HashList::ReverseIterator& { ReverseIterator temp {itr}; @@ -351,8 +333,7 @@ HashList::ReverseIterator::operator=(const ReverseIterator& itr) } template -auto -HashList::ReverseIterator::operator=(ReverseIterator&& itr) +auto HashList::ReverseIterator::operator=(ReverseIterator&& itr) -> HashList::ReverseIterator& { ReverseIterator temp {std::move(itr)}; @@ -361,7 +342,7 @@ HashList::ReverseIterator::operator=(ReverseIterator&& itr) } template -auto HashList::ReverseIterator::operator++() noexcept +auto HashList::ReverseIterator::operator++() noexcept -> HashList::ReverseIterator & { if (pnode_ == nullptr or phashList_ == nullptr) { @@ -379,7 +360,7 @@ auto HashList::ReverseIterator::operator++() noexcept } template -auto HashList::ReverseIterator::operator++(int) noexcept +auto HashList::ReverseIterator::operator++(int) noexcept -> HashList::ReverseIterator { ReverseIterator res {*this}; @@ -398,7 +379,7 @@ auto HashList::ReverseIterator::operator++(int) noexcept } template -auto HashList::ReverseIterator::operator--() noexcept +auto HashList::ReverseIterator::operator--() noexcept -> HashList::ReverseIterator & { if (phashList_ == nullptr) { @@ -420,7 +401,7 @@ auto HashList::ReverseIterator::operator--() noexcept } template -auto HashList::ReverseIterator::operator--(int) noexcept +auto HashList::ReverseIterator::operator--(int) noexcept -> HashList::ReverseIterator { ReverseIterator res {*this}; @@ -479,36 +460,31 @@ bool HashList::ReverseIterator::operator==( } template -Val & -HashList::ReverseIterator::operator*() +Val& HashList::ReverseIterator::operator*() { return pnode_->val_; } template -const Val & -HashList::ReverseIterator::operator*() const +const Val& HashList::ReverseIterator::operator*() const { return pnode_->val_; } template -Val * -HashList::ReverseIterator::operator->() +Val* HashList::ReverseIterator::operator->() { return &pnode_->val_; } template -const Val * -HashList::ReverseIterator::operator->() const +const Val* HashList::ReverseIterator::operator->() const { return &pnode_->val_; } template -void -HashList::ReverseIterator::swap(HashList::ReverseIterator& other) +void HashList::ReverseIterator::swap(HashList::ReverseIterator& other) { using std::swap; swap(pnode_, other.pnode_); @@ -586,7 +562,7 @@ HashList::~HashList() } template -HashList::HashList(HashList &&source) +HashList::HashList(HashList &&source) : numItem_ {source.numItem_}, pData_ {source.pData_}, dataHead_ {std::move(source.dataHead_)}, @@ -597,8 +573,7 @@ HashList::HashList(HashList &&source) } template -auto -HashList::operator=(HashList &&source) +auto HashList::operator=(HashList &&source) -> HashList& { if (this == &source) { @@ -618,8 +593,7 @@ HashList::operator=(HashList &&source) } template -auto -HashList::begin() +auto HashList::begin() -> HashList::Iterator { if (empty()) { @@ -629,8 +603,7 @@ HashList::begin() } template -auto -HashList::cbegin() const +auto HashList::cbegin() const -> const HashList::Iterator { if (empty()) { @@ -640,24 +613,21 @@ HashList::cbegin() const } template -auto -HashList::end() +auto HashList::end() -> HashList::Iterator { return Iterator(nullptr, this); } template -auto -HashList::cend() const +auto HashList::cend() const -> const HashList::Iterator { return Iterator(nullptr, this); } template -auto -HashList::rbegin() +auto HashList::rbegin() -> HashList::ReverseIterator { if (empty()) { @@ -667,8 +637,7 @@ HashList::rbegin() } template -auto -HashList::crbegin() const +auto HashList::crbegin() const -> const HashList::ReverseIterator { if (empty()) { @@ -678,39 +647,34 @@ HashList::crbegin() const } template -auto -HashList::rend() +auto HashList::rend() -> HashList::ReverseIterator { return ReverseIterator(nullptr, this); } template -auto -HashList::crend() const +auto HashList::crend() const -> const HashList::ReverseIterator { return ReverseIterator(nullptr, this); } template -Val& -HashList::front() +Val& HashList::front() { LinkNode *pnode = LinkNode::GetLinkNode(dataHead_.next_); return pnode->val_; } template -const Val& -HashList::front() const +const Val& HashList::front() const { return front(); } template -Val& -HashList::back(bool prepend) +Val& HashList::back(bool prepend) { auto pnode = LinkNode::GetLinkNode(dataHead_.prev_); if (prepend) { @@ -720,8 +684,7 @@ HashList::back(bool prepend) } template -Val& -HashList::operator[](const Key &key) +Val& HashList::operator[](const Key &key) { LinkNode *pnode {nullptr}; if (valueTab_.find(key) == valueTab_.end()) { @@ -737,8 +700,7 @@ HashList::operator[](const Key &key) } template -auto -HashList::find(const Key &key) +auto HashList::find(const Key &key) -> HashList::Iterator { const auto &itr = valueTab_.find(key); @@ -749,8 +711,7 @@ HashList::find(const Key &key) } template -void -HashList::push_front(const Key& key, const Val& val) +void HashList::push_front(const Key& key, const Val& val) { if (valueTab_.find(key) == valueTab_.end()) { LinkNode* pnode = AllocateNode(key, val); @@ -763,8 +724,7 @@ HashList::push_front(const Key& key, const Val& val) } template -void -HashList::push_front(const Key& key, Val&& val) +void HashList::push_front(const Key& key, Val&& val) { if (valueTab_.find(key) == valueTab_.end()) { LinkNode* pnode = AllocateNode(key, std::move(val)); @@ -777,8 +737,7 @@ HashList::push_front(const Key& key, Val&& val) } template -void -HashList::push_back(const Key& key, const Val& val) +void HashList::push_back(const Key& key, const Val& val) { if (valueTab_.find(key) == valueTab_.end()) { LinkNode* pnode = AllocateNode(key, val); @@ -791,8 +750,7 @@ HashList::push_back(const Key& key, const Val& val) } template -void -HashList::push_back(const Key& key, Val&& val) +void HashList::push_back(const Key& key, Val&& val) { if (valueTab_.find(key) == valueTab_.end()) { LinkNode* pnode = AllocateNode(key, std::move(val)); @@ -805,8 +763,7 @@ HashList::push_back(const Key& key, Val&& val) } template -void -HashList::pop_front() +void HashList::pop_front() { if (empty()) { return; @@ -817,8 +774,7 @@ HashList::pop_front() } template -void -HashList::pop_back() +void HashList::pop_back() { if (empty()) { return; @@ -829,8 +785,7 @@ HashList::pop_back() } template -auto -HashList::erase(const Key& key) +auto HashList::erase(const Key& key) -> HashList::Iterator { if (valueTab_.find(key) == valueTab_.end()) { @@ -845,8 +800,7 @@ HashList::erase(const Key& key) } template -auto -HashList::erase(const Iterator pos) +auto HashList::erase(const Iterator pos) -> HashList::Iterator { // assume pos is valid, otherwise the result is undefined @@ -859,8 +813,7 @@ HashList::erase(const Iterator pos) } template -auto -HashList::erase(const Iterator first, const Iterator last) +auto HashList::erase(const Iterator first, const Iterator last) -> HashList::Iterator { // assume pos is valid, otherwise the result is undefined @@ -875,8 +828,7 @@ HashList::erase(const Iterator first, const Iterator last) } template -bool -HashList::MoveNode(const Iterator& pos, LinkNode *&pnode) +bool HashList::MoveNode(const Iterator& pos, LinkNode *&pnode) { LinkNode *curNode = pos.GetNode(); if (curNode == pnode) { @@ -889,7 +841,7 @@ HashList::MoveNode(const Iterator& pos, LinkNode *&pnode) Link* nextLink = pnode->link_.next_; if (prevLink and nextLink) { prevLink->next_ = nextLink; - nextLink->prev_ = prevLink; + nextLink->prev_ = prevLink; } Link *currLink = &curNode->link_; prevLink = currLink->prev_; @@ -902,15 +854,13 @@ HashList::MoveNode(const Iterator& pos, LinkNode *&pnode) } template -void -HashList::MoveToHead(LinkNode *&pnode) +void HashList::MoveToHead(LinkNode *&pnode) { if (pnode->link_.prev_ and pnode->link_.next_) { Link* prev = pnode->link_.prev_; Link* next = pnode->link_.next_; prev->next_ = next; next->prev_ = prev; - } pnode->link_.next_ = dataHead_.next_; dataHead_.next_->prev_ = &pnode->link_; @@ -919,8 +869,7 @@ HashList::MoveToHead(LinkNode *&pnode) } template -void -HashList::MoveToTail(LinkNode *&pnode) +void HashList::MoveToTail(LinkNode *&pnode) { if (pnode->link_.prev_ and pnode->link_.next_) { Link* prev = pnode->link_.prev_; @@ -935,8 +884,7 @@ HashList::MoveToTail(LinkNode *&pnode) } template -auto -HashList::AllocateNode(const Key &key) +auto HashList::AllocateNode(const Key &key) ->LinkNode * { if (IsFull()) { @@ -952,8 +900,7 @@ HashList::AllocateNode(const Key &key) } template -auto -HashList::AllocateNode(const Key &key, const Val &val) +auto HashList::AllocateNode(const Key &key, const Val &val) ->LinkNode * { if (IsFull()) { @@ -969,8 +916,7 @@ HashList::AllocateNode(const Key &key, const Val &val) } template -auto -HashList::AllocateNode(const Key &key, Val &&val) +auto HashList::AllocateNode(const Key &key, Val &&val) ->LinkNode * { if (IsFull()) { @@ -986,8 +932,7 @@ HashList::AllocateNode(const Key &key, Val &&val) } template -void -HashList::ReclaimNode(LinkNode *&pnode) +void HashList::ReclaimNode(LinkNode *&pnode) { Link *prevLink = pnode->link_.prev_; Link *nextLink = pnode->link_.next_; diff --git a/include/perf_event_record.h b/include/perf_event_record.h index 09635d7..1b9d56c 100755 --- a/include/perf_event_record.h +++ b/include/perf_event_record.h @@ -21,13 +21,12 @@ #include #include #include -#include -#include - -#include -#include #include #include +#include +#include +#include +#include #include "debug_logger.h" #include "mem_map_item.h" diff --git a/include/perf_events.h b/include/perf_events.h index 5d25a42..68ac0e6 100755 --- a/include/perf_events.h +++ b/include/perf_events.h @@ -31,9 +31,9 @@ #include #endif -#include #include #include +#include #include "debug_logger.h" #include "perf_event_record.h" diff --git a/include/perf_record_format.h b/include/perf_record_format.h index f3ae86b..6513acb 100755 --- a/include/perf_record_format.h +++ b/include/perf_record_format.h @@ -15,8 +15,8 @@ #ifndef HIPERF_PERF_RECORD_FORMAT_H #define HIPERF_PERF_RECORD_FORMAT_H -#include #include +#include #include "utilities.h" diff --git a/include/report.h b/include/report.h index a90201f..9156a05 100755 --- a/include/report.h +++ b/include/report.h @@ -20,15 +20,13 @@ #include #include #include -#include #include #include "debug_logger.h" -#include "utilities.h" -#include "virtual_runtime.h" - // remove me latter #include "report_json_file.h" +#include "utilities.h" +#include "virtual_runtime.h" namespace OHOS { namespace Developtools { diff --git a/include/report_protobuf_file.h b/include/report_protobuf_file.h index b1a6439..8e1594f 100755 --- a/include/report_protobuf_file.h +++ b/include/report_protobuf_file.h @@ -17,8 +17,8 @@ #define HIPERF_REPORT_PROTOBUF_FILE #include -#include #include +#include #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream_impl_lite.h" diff --git a/include/subcommand_list.h b/include/subcommand_list.h index 1da5271..8ff60ae 100644 --- a/include/subcommand_list.h +++ b/include/subcommand_list.h @@ -15,8 +15,6 @@ #ifndef SUBCOMMAND_LIST_H_ #define SUBCOMMAND_LIST_H_ -#include "subcommand.h" - #include #include #include @@ -25,7 +23,7 @@ #include "debug_logger.h" #include "option.h" #include "perf_events.h" -#include "subcommand_list.h" +#include "subcommand.h" #include "utilities.h" namespace OHOS { diff --git a/include/subcommand_report.h b/include/subcommand_report.h index f06e5a8..0558b72 100644 --- a/include/subcommand_report.h +++ b/include/subcommand_report.h @@ -19,12 +19,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include "perf_file_reader.h" #if HAVE_PROTOBUF diff --git a/interfaces/innerkits/native/src/hiperf_client.cpp b/interfaces/innerkits/native/src/hiperf_client.cpp index 64b4ee1..32773cf 100644 --- a/interfaces/innerkits/native/src/hiperf_client.cpp +++ b/interfaces/innerkits/native/src/hiperf_client.cpp @@ -265,8 +265,12 @@ Client::Client(const std::string &outputDir) Setup(outputDir); // review: maybe change to some nice check , not just ign th pipe broken? - signal(SIGPIPE, SIG_IGN); - signal(SIGCHLD, SIG_IGN); + if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) { + HIPERF_HILOGD(MODULE_CPP_API, "signal failed."); + } + if (signal(SIGCHLD, SIG_IGN) == SIG_ERR) { + HIPERF_HILOGD(MODULE_CPP_API, "signal failed."); + } } bool Client::Setup(std::string outputDir) diff --git a/src/elf_symbol.cpp b/src/elf_symbol.cpp index 8c23a27..839e9d1 100755 --- a/src/elf_symbol.cpp +++ b/src/elf_symbol.cpp @@ -110,7 +110,7 @@ std::unique_ptr SymbolTable::MakeUnique(const std::string &symNames Symbol table '.symtab' contains 2 entries: Num: Value Size Type Bind Vis Ndx Name 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND - 1: 000000009c868f20 356 FUNC GLOBAL DEFAULT 1 + 1: 000000009c868f20 356 FUNC GLOBAL DEFAULT 1 */ if ((curPos + entrySize) > secSize) { break; diff --git a/src/perf_event_record.cpp b/src/perf_event_record.cpp index f0fae6c..19f1ffc 100755 --- a/src/perf_event_record.cpp +++ b/src/perf_event_record.cpp @@ -14,11 +14,11 @@ */ #define HILOG_TAG "PerfRecord" -#include - #include "perf_event_record.h" +#include #include "utilities.h" + using namespace std; namespace OHOS { namespace Developtools { diff --git a/src/perf_events.cpp b/src/perf_events.cpp index ffafa55..d389807 100755 --- a/src/perf_events.cpp +++ b/src/perf_events.cpp @@ -685,7 +685,7 @@ bool PerfEvents::EnableTracking() trackedCommand_->Stop(); } std::string commandName = trackedCommand_->GetCommandName(); - printf("failed to execute command: %zu: %s\n", commandName.size(),commandName.c_str()); + printf("failed to execute command: %zu: %s\n", commandName.size(), commandName.c_str()); return false; } } else if (trackedCommand_->GetState() != TrackedCommand::State::COMMAND_STARTED) { diff --git a/src/perf_file_writer.cpp b/src/perf_file_writer.cpp index de7d5f5..95f44ad 100755 --- a/src/perf_file_writer.cpp +++ b/src/perf_file_writer.cpp @@ -363,7 +363,11 @@ bool PerfFileWriter::WriteFeatureData() return false; } } - HLOGV("features data at file '0x%lx'", ftell(fp_)); + long offset = ftell(fp_); + if (offset < 0) { + return false; + } + HLOGV("features data at file '0x%lx'", offset); i = 0; for (auto &featureSection : featureSections_) { diff --git a/src/report.cpp b/src/report.cpp index 632990b..32b7dba 100755 --- a/src/report.cpp +++ b/src/report.cpp @@ -321,10 +321,16 @@ bool Report::MultiLevelSorting(const ReportItem &a, const ReportItem &b) void Report::OutputStdStatistics(ReportEventConfigItem &config) { - fprintf(output_, "\n"); // make a blank line for new event - fprintf(output_, "Event: %s (type %" PRIu32 " id %" PRIu64 ")\n", config.eventName_.c_str(), - config.type_, config.config_); - fprintf(output_, "Samples Count: %" PRIu64 "\n", config.sampleCount_); + if (fprintf(output_, "\n") < 0) { + return; + } // make a blank line for new event + if (fprintf(output_, "Event: %s (type %" PRIu32 " id %" PRIu64 ")\n", config.eventName_.c_str(), + config.type_, config.config_) < 0) { + return; + } + if (fprintf(output_, "Samples Count: %" PRIu64 "\n", config.sampleCount_) < 0) { + return; + } if (!config.coutMode_) { fprintf(output_, "Time in ns: "); } else { @@ -356,7 +362,9 @@ void Report::OutputStdHead(ReportEventConfigItem &config, bool diffMode) { // head print const std::string head = "Heating"; - fprintf(output_, "%-*s ", FULL_PERCENTAGE_LEN, head.c_str()); + if (fprintf(output_, "%-*s ", FULL_PERCENTAGE_LEN, head.c_str()) < 0) { + return; + } if (diffMode) { const std::string diff = "Diff"; @@ -378,12 +386,16 @@ void Report::OutputStdHead(ReportEventConfigItem &config, bool diffMode) if (remainingWidth <= 0) { key.maxLen_ = 0; } - fprintf(output_, "%-*s ", (remainingWidth > 0) ? static_cast(key.maxLen_) : 0, - key.keyName_.c_str()); + if (fprintf(output_, "%-*s ", (remainingWidth > 0) ? static_cast(key.maxLen_) : 0, + key.keyName_.c_str()) < 0) { + return; + } HLOGD("'%s' max len %zu(from '%s') console width %d", key.keyName_.c_str(), key.maxLen_, key.maxValue_.c_str(), remainingWidth); } - fprintf(output_, "\n"); + if (fprintf(output_, "\n") < 0) { + return; + } } bool Report::OutputStdCallFrame(int indent, const std::string_view &funcName, uint64_t eventCount, @@ -507,9 +519,13 @@ void Report::OutputStdContentItem(const ReportItem &reportItem) // output by sort keys for (auto sortKey : displayKeyNames_) { ReportKey &reportKey = Report::reportKeyMap_.at(sortKey); - fprintf(output_, "%s ", reportKey.GetValue(reportItem).c_str()); + if (fprintf(output_, "%s ", reportKey.GetValue(reportItem).c_str()) < 0) { + return; + } + } + if (fprintf(output_, "\n") < 0) { + return; } - fprintf(output_, "\n"); } void Report::OutputStdItemHeating(float heat, float heat2) diff --git a/src/report_json_file.cpp b/src/report_json_file.cpp index a89810c..938c135 100755 --- a/src/report_json_file.cpp +++ b/src/report_json_file.cpp @@ -286,10 +286,14 @@ void ReportJsonFile::OutputJsonRuntimeInfo() } OutputJsonVectorList(output_, "symbolsFileList", jsonFilePaths); - fprintf(output_, ","); + if (fprintf(output_, ",") < 0) { + return; + } OutputJsonMap(output_, "SymbolMap", functionMap_, true); - fprintf(output_, ","); + if (fprintf(output_, ",") < 0) { + return; + } OutputJsonMapList(output_, "recordSampleInfo", reportConfigItems_, true); } @@ -300,11 +304,15 @@ bool ReportJsonFile::OutputJson(FILE *output) return false; } output_ = output; - fprintf(output_, "{"); + if (fprintf(output, "{") < 0) { + return false; + } OutputJsonFeatureString(); OutputJsonRuntimeInfo(); - fprintf(output_, "}"); + if (fprintf(output, "}") < 0) { + return false; + } return true; } } // namespace HiPerf diff --git a/src/report_protobuf_file.cpp b/src/report_protobuf_file.cpp index 5086caf..2a002ab 100755 --- a/src/report_protobuf_file.cpp +++ b/src/report_protobuf_file.cpp @@ -251,27 +251,13 @@ bool ReportProtobufFileReader::Dump(std::string fileName, ProtobufReadBack readB try { protobufFileStream_->exceptions(std::ifstream::failbit | std::ifstream::badbit); protobufFileStream_->open(fileName_.c_str(), std::fstream::in | std::fstream::binary); - printf("open proto buf file succeed.\n"); - if (!CheckFileMagic()) { return false; } - - protpbufInputStream_ = - std::make_unique(this); + protpbufInputStream_ =std::make_unique(this); protpbufCodedInputStream_ = std::make_unique(protpbufInputStream_.get()); - - // LittleEndian32(sample_size) - // message Record(sample) - // LittleEndian32(sample_size) - // message Record(sample) - // ... - // LittleEndian32(sample_size) - // message Record(sample) - // LittleEndian32(0) - // let's read one by one uint32_t recordLength = 0; do { protpbufCodedInputStream_->ReadLittleEndian32(&recordLength); diff --git a/src/subcommand_record.cpp b/src/subcommand_record.cpp index cfa270e..d9cbbb6 100644 --- a/src/subcommand_record.cpp +++ b/src/subcommand_record.cpp @@ -626,7 +626,7 @@ bool SubCommandRecord::TraceOffCpu() if (!ReadIntFromProcFile(node.c_str(), enable) and !ReadIntFromProcFile(nodeDebug.c_str(), enable)) { printf("Cannot trace off CPU, event sched:sched_switch is not avilable (%s or %s)\n", - node.c_str(), nodeDebug.c_str()); + node.c_str(), nodeDebug.c_str()); return false; } diff --git a/src/subcommand_report.cpp b/src/subcommand_report.cpp index e382ea1..d1ed978 100644 --- a/src/subcommand_report.cpp +++ b/src/subcommand_report.cpp @@ -475,7 +475,9 @@ bool SubCommandReport::LoadPerfData() bool SubCommandReport::OutputStd() { - fprintf(output_, "<>\n", diffMode_ ? " Diff" : ""); + if (fprintf(output_, "<>\n", diffMode_ ? " Diff" : "") < 0) { + return false; + } // feature string: const auto &featureSections = recordFileReader_->GetFeatureSections(); diff --git a/src/subcommand_stat.cpp b/src/subcommand_stat.cpp index ce1c0dc..9a39f58 100644 --- a/src/subcommand_stat.cpp +++ b/src/subcommand_stat.cpp @@ -15,12 +15,13 @@ #define HILOG_TAG "Stat" +#include "subcommand_stat.h" + #include #include #include #include "debug_logger.h" -#include "subcommand_stat.h" #include "utilities.h" const uint16_t ONE_HUNDRED = 100; @@ -179,8 +180,7 @@ bool SubCommandStat::IsMonitoredAtAllTime(const double &scale) return (fabs(scale - 1.0) < SCALE_ERROR_LIMIT); } -void SubCommandStat::GetComments( - const std::map> &countEvents, +void SubCommandStat::GetComments(const std::map> &countEvents, std::map &comments) { double running_time_in_sec = 0; diff --git a/src/symbols_file.cpp b/src/symbols_file.cpp index ddcc531..64b63e0 100755 --- a/src/symbols_file.cpp +++ b/src/symbols_file.cpp @@ -502,8 +502,12 @@ private: { #ifndef HIPERF_ELF_READ_USE_MMAP if (readFd_ == nullptr) { + FILE *fp = fopen(loadElfPath.c_str(), "rb"); + if (fp == nullptr) { + return; + } readFd_ = - std::unique_ptr(fopen(loadElfPath.c_str(), "rb"), &fclose); + std::unique_ptr(fp, &fclose); return; } #else @@ -1215,7 +1219,6 @@ const Symbol SymbolsFile::GetSymbolWithVaddr(uint64_t vaddrInFile) 4 < 5 at index 3 5 < 6 at index 5 6 < not found - if key symbol vaddr is { 1, 2, 4, 5, 5, 6 }; check ip vaddr for each val : ip sym diff --git a/src/utilities.cpp b/src/utilities.cpp index edc348b..6227bdf 100755 --- a/src/utilities.cpp +++ b/src/utilities.cpp @@ -330,7 +330,9 @@ bool CompressFile(const std::string &dataFile, const std::string &destFile) return false; } } - fclose(fp); + if (fclose(fp) < 0) { + return false; + } return true; } @@ -368,7 +370,9 @@ bool UncompressFile(const std::string &gzipFile, const std::string &dataFile) return false; } } - fclose(fp); + if (fclose(fp) < 0) { + return false; + } return true; } diff --git a/test/fuzztest/PerfFile_fuzzer/PerfFile_fuzzer.cpp b/test/fuzztest/PerfFile_fuzzer/PerfFile_fuzzer.cpp index fb8713c..00bd20a 100755 --- a/test/fuzztest/PerfFile_fuzzer/PerfFile_fuzzer.cpp +++ b/test/fuzztest/PerfFile_fuzzer/PerfFile_fuzzer.cpp @@ -63,7 +63,11 @@ public: static std::unique_ptr Instance(const std::string &fileName, const uint8_t *data, size_t size) { - std::unique_ptr fp(fopen(fileName.c_str(), "rb"), fclose); + FILE *fp_ = fopen(fileName.c_str(), "rb"); + if (fp_ == nullptr) { + return nullptr; + } + std::unique_ptr fp(fp_, fclose); if (fp == nullptr) { HLOGE("fail to open file %s", fileName.c_str()); return nullptr; diff --git a/test/unittest/common/native/debug_logger_test.cpp b/test/unittest/common/native/debug_logger_test.cpp index 319da87..7f9af66 100755 --- a/test/unittest/common/native/debug_logger_test.cpp +++ b/test/unittest/common/native/debug_logger_test.cpp @@ -25,7 +25,6 @@ using namespace OHOS::HiviewDFX; namespace OHOS { namespace Developtools { namespace HiPerf { - DebugLevel DebugLogger::debugLevel_ = DEFAULT_LOG_LEVEL; bool DebugLogger::logDisabled_ = false; diff --git a/test/unittest/common/native/include/callstack_test.h b/test/unittest/common/native/include/callstack_test.h index 70e4617..60abba3 100755 --- a/test/unittest/common/native/include/callstack_test.h +++ b/test/unittest/common/native/include/callstack_test.h @@ -109,7 +109,11 @@ static const std::vector TEST_DWARF_FRAMES = { template void LoadFromFile(const std::string &fileName, std::vector &data) { - std::unique_ptr fp(fopen(fileName.c_str(), "r"), fclose); + FILE* fp_ = fopen(fileName.c_str(), "r"); + if (fp_ == nullptr) { + return; + } + std::unique_ptr fp(fp_, fclose); if (fp) { struct stat sb = {}; ASSERT_NE(fstat(fileno(fp.get()), &sb), -1); diff --git a/test/unittest/common/native/option_debug_test.cpp b/test/unittest/common/native/option_debug_test.cpp index 4c4cbd0..fb970a9 100755 --- a/test/unittest/common/native/option_debug_test.cpp +++ b/test/unittest/common/native/option_debug_test.cpp @@ -78,7 +78,9 @@ void OptionDebugTest::LogLevelTest(std::vector args, const DebugLev HLOGV("%s", logMessage.c_str()); HLOGM("%s", logMessage.c_str()); - fflush(DebugLogger::GetInstance()->file_); + if (fflush(DebugLogger::GetInstance()->file_) != 0) { + HLOGD("fflush failed."); + } std::string log = ReadFileToString(DebugLogger::GetInstance()->logPath_); ASSERT_EQ(log.empty(), false); // we have 6 level log diff --git a/test/unittest/common/native/symbols_file_test.cpp b/test/unittest/common/native/symbols_file_test.cpp index 2d6d066..7f1c418 100755 --- a/test/unittest/common/native/symbols_file_test.cpp +++ b/test/unittest/common/native/symbols_file_test.cpp @@ -391,7 +391,7 @@ HWTEST_F(SymbolsFileTest, GetSymbolWithVaddr2, TestSize.Level1) part of elf32_test's symbols vaddr(hex) size(dec) name 00001000 0 _init - 00001030 0 + 00001030 0 00001320 58 _start 00001512 27 main 0000145d 124 TestGlobalChildFunction @@ -477,7 +477,7 @@ HWTEST_F(SymbolsFileTest, GetSymbolWithVaddrFullMatch, TestSize.Level1) // last one 0000124c T _fini */ - #ifdef __arm__ +#ifdef __arm__ enum SymbolAddr : uint64_t { INIT = 0X1000U, START = 0X1070U, @@ -490,7 +490,7 @@ HWTEST_F(SymbolsFileTest, GetSymbolWithVaddrFullMatch, TestSize.Level1) CSU_FINI = 0X1240U, FINI = 0X124CU, }; - #else +#else enum SymbolAddr : uint64_t { INIT = 0X1000U, START = 0X1040U, @@ -503,7 +503,7 @@ HWTEST_F(SymbolsFileTest, GetSymbolWithVaddrFullMatch, TestSize.Level1) CSU_FINI = 0X11B0U, FINI = 0X11B8U, }; - #endif +#endif for (uint64_t addr = SymbolAddr::INIT; addr < SymbolAddr::START; ++addr) { if (elfSymbols->GetSymbolWithVaddr(addr).isValid()) { EXPECT_EQ(elfSymbols->GetSymbolWithVaddr(addr).demangle_, "_init"); diff --git a/test/unittest/common/native/utilities_test.cpp b/test/unittest/common/native/utilities_test.cpp index 09fae35..f61324e 100755 --- a/test/unittest/common/native/utilities_test.cpp +++ b/test/unittest/common/native/utilities_test.cpp @@ -14,10 +14,9 @@ */ #include "utilities_test.h" - -#include "utilities.h" #include #include +#include "utilities.h" using namespace testing::ext; using namespace std; -- Gitee