diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..7dab184d2 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,24 @@ +--- +Checks: > + -*, + bugprone-argument-comment, + bugprone-too-small-loop-variable, + google-explicit-constructor, + google-readability-casting, + misc-include-cleaner, + misc-unused-using-decls, + modernize-loop-convert, + modernize-use-bool-literals, + modernize-use-equals-default, + modernize-use-equals-delete, + modernize-use-nullptr, + readability-avoid-const-params-in-decls, + readability-else-after-return, + readability-inconsistent-declaration-parameter-name, + readability-make-member-function-const, + readability-redundant-control-flow, + readability-redundant-member-init, + readability-simplify-boolean-expr, + readability-static-accessed-through-instance +WarningsAsErrors: '*' +HeaderFilterRegex: '(include|src|tests).*(?/dev/null; then + clang_tidy=$(command -v clang-tidy-14) +elif command -v clang-tidy >/dev/null; then + clang_tidy=$(command -v clang-tidy) + case "$($clang_tidy --version)" in + *"$CLANG_TIDY_VERSION"*) ;; + + *) + die "$($clang_tidy --version); $CLANG_TIDY_VERSION required" + ;; + esac +else + die "clang-tidy 14.0 required" +fi + +# Search for clang-apply-replacements-14 +if command -v clang-apply-replacements-14 >/dev/null; then + clang_apply_replacements=$(command -v clang-apply-replacements-14) +elif command -v clang-apply-replacements >/dev/null; then + clang_apply_replacements=$(command -v clang-apply-replacements) + case "$($clang_apply_replacements --version)" in + "$CLANG_APPLY_REPLACEMENTS_VERSION"*) ;; + + *) + die "$($clang_apply_replacements --version); $CLANG_APPLY_REPLACEMENTS_VERSION required" + ;; + esac +else + die "clang-apply-replacements 14.0 required" +fi + +# Search for run-clang-tidy-14.py +if command -v run-clang-tidy-14.py >/dev/null; then + run_clang_tidy=$(command -v run-clang-tidy-14.py) +elif command -v run-clang-tidy-14 >/dev/null; then + run_clang_tidy=$(command -v run-clang-tidy-14) +elif command -v run-clang-tidy.py >/dev/null; then + run_clang_tidy=$(command -v run-clang-tidy.py) +elif command -v run-clang-tidy >/dev/null; then + run_clang_tidy=$(command -v run-clang-tidy) +else + die "run-clang-tidy.py 14.0 required" +fi + +$run_clang_tidy -clang-tidy-binary "$clang_tidy" -clang-apply-replacements-binary "$clang_apply_replacements" "$@" || die + +exit 0 diff --git a/script/make-pretty b/script/make-pretty index c9ffae457..6a51eebf0 100755 --- a/script/make-pretty +++ b/script/make-pretty @@ -37,6 +37,7 @@ # Format c/c++ only: # # script/make-pretty clang +# script/make-pretty clang-tidy # # Format markdown only: # @@ -49,12 +50,15 @@ # Check only: # # script/make-pretty check clang +# script/make-pretty check clang-tidy # script/make-pretty check markdown # script/make-pretty check python # set -euo pipefail +SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) + readonly OT_BUILD_JOBS=$(getconf _NPROCESSORS_ONLN) readonly OT_EXCLUDE_DIRS=(third_party) @@ -62,6 +66,14 @@ readonly OT_CLANG_SOURCES=('*.c' '*.cc' '*.cpp' '*.h' '*.hpp') readonly OT_MARKDOWN_SOURCES=('*.md') readonly OT_PYTHON_SOURCES=('*.py') +OT_CLANG_TIDY_FIX_DIRS=('${SCRIPT_DIR}../include' '${SCRIPT_DIR}../src' '${SCRIPT_DIR}../tests') +readonly OT_CLANG_TIDY_FIX_DIRS + +OT_CLANG_TIDY_BUILD_OPTS=( + '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON' +) +readonly OT_CLANG_TIDY_BUILD_OPTS + do_clang_format() { echo -e '=====================' @@ -82,6 +94,30 @@ do_clang_check() | xargs -n3 -P"${OT_BUILD_JOBS}" script/clang-format-check } +do_clang_tidy_fix() +{ + echo -e '========================================' + echo -e ' format c/c++ (clang-tidy)' + echo -e '========================================' + + (mkdir -p ./build/cmake-tidy \ + && cd ./build/cmake-tidy \ + && cmake "${OT_CLANG_TIDY_BUILD_OPTS[@]}" ../.. \ + && ../../script/clang-tidy -j"$OT_BUILD_JOBS" "${OT_CLANG_TIDY_FIX_DIRS[@]}" -fix) +} + +do_clang_tidy_check() +{ + echo -e '========================================' + echo -e ' check c/c++ (clang-tidy)' + echo -e '========================================' + + (mkdir -p ./build/cmake-tidy \ + && cd ./build/cmake-tidy \ + && cmake "${OT_CLANG_TIDY_BUILD_OPTS[@]}" ../.. \ + && ../../script/clang-tidy -j"$OT_BUILD_JOBS" "${OT_CLANG_TIDY_FIX_DIRS[@]}") +} + do_markdown_format() { echo -e '======================' @@ -126,10 +162,13 @@ do_check() { if [ $# == 0 ]; then do_clang_check + do_clang_tidy_check do_markdown_check do_python_check elif [ "$1" == 'clang' ]; then do_clang_check + elif [ "$1" == 'clang-tidy' ]; then + do_clang_tidy_check elif [ "$1" == 'markdown' ]; then do_markdown_check elif [ "$1" == 'python' ]; then @@ -149,6 +188,8 @@ main() do_python_format elif [ "$1" == 'clang' ]; then do_clang_format + elif [ "$1" == 'clang-tidy' ]; then + do_clang_tidy_fix elif [ "$1" == 'markdown' ]; then do_markdown_format elif [ "$1" == 'python' ]; then diff --git a/src/app/cli/interpreter.cpp b/src/app/cli/interpreter.cpp index 613b2208a..26aeffdb0 100644 --- a/src/app/cli/interpreter.cpp +++ b/src/app/cli/interpreter.cpp @@ -1202,12 +1202,12 @@ Interpreter::Value Interpreter::ProcessBr(const Expression &aExpr) } else { - for (auto iter = json.begin(); iter != json.end(); ++iter) + for (const auto &jsonObject : json) { BorderAgent ba; try { - BorderAgentFromJson(ba, *iter); + BorderAgentFromJson(ba, jsonObject); } catch (std::exception &e) { ExitNow(value = ERROR_BAD_FORMAT("incorrect border agent JSON format: {}", e.what())); @@ -1440,7 +1440,7 @@ Interpreter::Value Interpreter::ProcessBr(const Expression &aExpr) auto mdnsCancelEventQueue = [](evutil_socket_t aSocket, short aWhat, void *aArg) { (void)aSocket; (void)aWhat; - event_base *ev_base = (event_base *)aArg; + event_base *ev_base = static_cast(aArg); event_base_loopbreak(ev_base); }; // Attach cancel pipe to event loop. When bytes written to the diff --git a/src/app/cli/interpreter.hpp b/src/app/cli/interpreter.hpp index 57283fab3..9ff704bee 100644 --- a/src/app/cli/interpreter.hpp +++ b/src/app/cli/interpreter.hpp @@ -179,7 +179,7 @@ class Interpreter * * @see @ref Interpreter::MultiNetCommandContext */ - Error ReParseMultiNetworkSyntax(const Expression &aExpr, Expression &aRretExpr); + Error ReParseMultiNetworkSyntax(const Expression &aExpr, Expression &aRetExpr); /** * Updates on-screen visualization of the current network * selection. diff --git a/src/app/cli/interpreter_test.cpp b/src/app/cli/interpreter_test.cpp index b6dfe00b3..8f07f92d5 100644 --- a/src/app/cli/interpreter_test.cpp +++ b/src/app/cli/interpreter_test.cpp @@ -50,7 +50,6 @@ using namespace ot::commissioner::persistent_storage; using testing::_; using testing::DoAll; -using testing::Mock; using testing::Return; using testing::ReturnRef; using testing::StrEq; diff --git a/src/app/cli/job_manager.cpp b/src/app/cli/job_manager.cpp index 345920c85..3cb196312 100644 --- a/src/app/cli/job_manager.cpp +++ b/src/app/cli/job_manager.cpp @@ -92,7 +92,7 @@ void JobManager::CleanupJobs() { for (auto job : mJobPool) { - ASSERT(job == NULL || job->IsStopped()); + ASSERT(job == nullptr || job->IsStopped()); delete job; } mJobPool.clear(); @@ -122,12 +122,16 @@ Error JobManager::CreateJob(CommissionerAppPtr &aCommissioner, const Interpreter Error JobManager::PrepareJobs(const Interpreter::Expression &aExpr, const XpanIdArray &aNids, bool aGroupAlias) { + Error error; + if (utils::ToLower(aExpr[0]) == "start") - return PrepareStartJobs(aExpr, aNids, aGroupAlias); + { + ExitNow(error = PrepareStartJobs(aExpr, aNids, aGroupAlias)); + } else if (utils::ToLower(aExpr[0]) == "stop") - return PrepareStopJobs(aExpr, aNids, aGroupAlias); - - Error error; + { + ExitNow(error = PrepareStopJobs(aExpr, aNids, aGroupAlias)); + } for (const auto &nid : aNids) { @@ -169,6 +173,7 @@ Error JobManager::PrepareJobs(const Interpreter::Expression &aExpr, const XpanId SuccessOrExit(error = CreateJob(entry->second, jobExpr, nid)); } + exit: return error; } @@ -603,7 +608,7 @@ void JobManager::RunJobs() { for (auto job : mJobPool) { - ASSERT(job != NULL); + ASSERT(job != nullptr); job->Run(); } WaitForJobs(); @@ -616,7 +621,7 @@ void JobManager::CancelCommand() for (auto job : mJobPool) { - ASSERT(job != NULL); + ASSERT(job != nullptr); job->Cancel(); } WaitForJobs(); @@ -639,7 +644,7 @@ void JobManager::WaitForJobs() { for (auto job : mJobPool) { - ASSERT(job != NULL); + ASSERT(job != nullptr); job->Wait(); } } diff --git a/src/app/cli/job_manager_test.cpp b/src/app/cli/job_manager_test.cpp index 183760323..e50bf7b8e 100644 --- a/src/app/cli/job_manager_test.cpp +++ b/src/app/cli/job_manager_test.cpp @@ -47,7 +47,6 @@ using namespace ot::commissioner; using testing::_; using testing::DoAll; using testing::Invoke; -using testing::Mock; using testing::Return; using testing::WithArg; using testing::WithArgs; @@ -63,10 +62,7 @@ class JobManagerTestSuite : public testing::Test struct TestContext { TestContext() - : mPS{} - , mRegistry{} - , mConf{} - , mInterpreter{} + : mConf{} , mJobManager{mInterpreter} , mDefaultCommissioner{new CommissionerAppMock()} { @@ -86,10 +82,7 @@ class JobManagerTestSuite : public testing::Test CommissionerAppStaticExpecter mCommissionerAppStaticExpecter; }; - JobManagerTestSuite() - : Test() - { - } + JobManagerTestSuite() = default; virtual ~JobManagerTestSuite() = default; void SetInitialExpectations(TestContext &aContext) diff --git a/src/app/cli/main.cpp b/src/app/cli/main.cpp index aee82786c..32e1ea53c 100644 --- a/src/app/cli/main.cpp +++ b/src/app/cli/main.cpp @@ -119,7 +119,7 @@ int main(int argc, const char *argv[]) while (parseParams) { - ch = getopt_long(argc, (char *const *)argv, "hvc:r:", gCommissionerCliOptions, nullptr); + ch = getopt_long(argc, const_cast(argv), "hvc:r:", gCommissionerCliOptions, nullptr); switch (ch) { case 'h': diff --git a/src/app/cli/security_materials.cpp b/src/app/cli/security_materials.cpp index 702f5a8b3..0e8b4484c 100644 --- a/src/app/cli/security_materials.cpp +++ b/src/app/cli/security_materials.cpp @@ -49,8 +49,8 @@ namespace security_material { using SMPair = std::pair; -static Error GetNetworkSMImpl(const std::string aNwkFolder, // a folder to start from - const std::string aAlias, // network id or network name +static Error GetNetworkSMImpl(const std::string &aNwkFolder, // a folder to start from + const std::string &aAlias, // network id or network name bool aNeedCert, bool aNeedPSKc, SecurityMaterials &aSM); @@ -156,8 +156,8 @@ Error GetNetworkSM(const std::string aAlias, bool aNeedCert, bool aNeedPSKc, Sec return GetNetworkSMImpl("nwk/", aAlias, aNeedCert, aNeedPSKc, aSM); } -static Error GetNetworkSMImpl(const std::string aNwkFolder, - const std::string aAlias, +static Error GetNetworkSMImpl(const std::string &aNwkFolder, + const std::string &aAlias, bool aNeedCert, bool aNeedPSKc, SecurityMaterials &aSM) @@ -228,19 +228,19 @@ static Error GetNetworkSMImpl(const std::string aNwkFolder, return error; } -bool SecurityMaterials::IsAnyFound(bool aNeedCert, bool aNeedPSKc, bool aNeedToken /*=false*/) +bool SecurityMaterials::IsAnyFound(bool aNeedCert, bool aNeedPSKc, bool aNeedToken /*=false*/) const { return (aNeedCert && (mCertificate.size() != 0 || mPrivateKey.size() != 0 || mTrustAnchor.size() != 0)) || (aNeedToken && mCommissionerToken.size() != 0) || (aNeedPSKc && mPSKc.size() != 0); } -bool SecurityMaterials::IsIncomplete(bool aNeedCert, bool aNeedPSKc, bool aNeedToken /*=false*/) +bool SecurityMaterials::IsIncomplete(bool aNeedCert, bool aNeedPSKc, bool aNeedToken /*=false*/) const { return (aNeedCert && (mCertificate.size() == 0 || mPrivateKey.size() == 0 || mTrustAnchor.size() == 0)) || (aNeedToken && mCommissionerToken.size() == 0) || (aNeedPSKc && mPSKc.size() == 0); } -bool SecurityMaterials::IsEmpty(bool isCCM) +bool SecurityMaterials::IsEmpty(bool isCCM) const { return isCCM ? mCertificate.size() == 0 && mPrivateKey.size() == 0 && mTrustAnchor.size() == 0 && mCommissionerToken.size() == 0 diff --git a/src/app/cli/security_materials.hpp b/src/app/cli/security_materials.hpp index 91088d5a3..7ccf8bd6b 100644 --- a/src/app/cli/security_materials.hpp +++ b/src/app/cli/security_materials.hpp @@ -62,14 +62,14 @@ struct SecurityMaterials // See if any part of credentials is already found depending on the // credentials type - bool IsAnyFound(bool aNeedCert, bool aNeedPSKc, bool aNeedToken = false); + bool IsAnyFound(bool aNeedCert, bool aNeedPSKc, bool aNeedToken = false) const; // See if any part of credentials is missing depending on the // credentials type - bool IsIncomplete(bool aNeedCert, bool aNeedPSKc, bool aNeedToken = false); + bool IsIncomplete(bool aNeedCert, bool aNeedPSKc, bool aNeedToken = false) const; // See if entire set of credentials is empty - bool IsEmpty(bool isCCM); + bool IsEmpty(bool isCCM) const; }; /** diff --git a/src/app/commissioner_app.cpp b/src/app/commissioner_app.cpp index d30df0d6b..1d4c30d09 100644 --- a/src/app/commissioner_app.cpp +++ b/src/app/commissioner_app.cpp @@ -1036,7 +1036,7 @@ bool CommissionerApp::JoinerKey::operator<(const JoinerKey &aOther) const return mType < aOther.mType || (mType == aOther.mType && mId < aOther.mId); } -CommissionerDataset CommissionerApp::MakeDefaultCommissionerDataset() +CommissionerDataset CommissionerApp::MakeDefaultCommissionerDataset() const { CommissionerDataset dataset; diff --git a/src/app/commissioner_app.hpp b/src/app/commissioner_app.hpp index 8371fe01c..886312d7b 100644 --- a/src/app/commissioner_app.hpp +++ b/src/app/commissioner_app.hpp @@ -285,7 +285,7 @@ class CommissionerApp : public CommissionerHandler bool operator<(const JoinerKey &aOther) const; }; - CommissionerDataset MakeDefaultCommissionerDataset(); + CommissionerDataset MakeDefaultCommissionerDataset() const; static ByteArray &GetSteeringData(CommissionerDataset &aDataset, JoinerType aJoinerType); static uint16_t &GetJoinerUdpPort(CommissionerDataset &aDataset, JoinerType aJoinerType); diff --git a/src/app/file_util.cpp b/src/app/file_util.cpp index 4403f009d..bc4214c3b 100644 --- a/src/app/file_util.cpp +++ b/src/app/file_util.cpp @@ -73,7 +73,7 @@ Error WriteFile(const std::string &aData, const std::string &aFilename) fputs(aData.c_str(), f); exit: - if (f != NULL) + if (f != nullptr) { fclose(f); } @@ -107,7 +107,7 @@ Error ReadFile(std::string &aData, const std::string &aFilename) } exit: - if (f != NULL) + if (f != nullptr) { fclose(f); } diff --git a/src/app/file_util_test.cpp b/src/app/file_util_test.cpp index e60a73e87..d1bb05bfd 100644 --- a/src/app/file_util_test.cpp +++ b/src/app/file_util_test.cpp @@ -77,7 +77,7 @@ TEST(FileUtil, PathExists) EXPECT_TRUE(RestoreDirPath(dirName).GetCode() == ErrorCode::kNone); EXPECT_TRUE(PathExists(dirName).GetCode() == ErrorCode::kNone); FILE *f = fopen(fileName.c_str(), "w"); - if (f != NULL) + if (f != nullptr) { EXPECT_TRUE(PathExists(fileName).GetCode() == ErrorCode::kNone); fclose(f); @@ -90,7 +90,7 @@ TEST(FileUtil, PathExists) TEST(FileUtil, WriteFile) { - FILE *f = NULL; + FILE *f = nullptr; std::string path = "./test_write"; std::string test = "test"; std::string alt = "alt"; @@ -109,11 +109,11 @@ TEST(FileUtil, WriteFile) EXPECT_EQ(ReadFile(read, path), ErrorCode::kNone); EXPECT_EQ(read, alt); // write to a blocked file - EXPECT_TRUE((f = fopen(path.c_str(), "r")) != NULL); + EXPECT_TRUE((f = fopen(path.c_str(), "r")) != nullptr); EXPECT_EQ(fchmod(fileno(f), 0), 0); EXPECT_EQ(WriteFile(test, path), ErrorCode::kIOBusy); - if (f != NULL) + if (f != nullptr) { fclose(f); } diff --git a/src/app/json.cpp b/src/app/json.cpp index efbe01c02..3cc7bd976 100644 --- a/src/app/json.cpp +++ b/src/app/json.cpp @@ -410,7 +410,7 @@ static void from_json(const Json &aJson, SecurityPolicy &aSecurityPolicy) static void to_json(Json &aJson, const ot::commissioner::PanId &aPanId) { - aJson = (std::string)aPanId; + aJson = std::to_string(aPanId); } static void from_json(const Json &aJson, ot::commissioner::PanId &aPanId) diff --git a/src/app/mdns_handler.cpp b/src/app/mdns_handler.cpp index 395769359..13bc3f33b 100644 --- a/src/app/mdns_handler.cpp +++ b/src/app/mdns_handler.cpp @@ -49,15 +49,15 @@ static inline ByteArray ToByteArray(const mdns_string_t &aString) return ByteArray{aString.str, aString.str + aString.length}; } -int HandleRecord(const struct sockaddr *from, - mdns_entry_type_t entry, - uint16_t type, - uint16_t rclass, - uint32_t ttl, - const void *data, - size_t size, - size_t offset, - size_t length, +int HandleRecord(const struct sockaddr *aFrom, + mdns_entry_type_t aEntry, + uint16_t aType, + uint16_t aRclass, + uint32_t aTtl, + const void *aData, + size_t aSize, + size_t aOffset, + size_t aLength, void *aBorderAgent) { struct sockaddr_storage fromAddrStorage; @@ -70,10 +70,10 @@ int HandleRecord(const struct sockaddr *from, BorderAgent &borderAgent = borderAgentOrErrorMsg.mBorderAgent; Error &error = borderAgentOrErrorMsg.mError; - (void)rclass; - (void)ttl; + (void)aRclass; + (void)aTtl; - *reinterpret_cast(&fromAddrStorage) = *from; + *reinterpret_cast(&fromAddrStorage) = *aFrom; if (fromAddr.Set(fromAddrStorage) != ErrorCode::kNone) { ExitNow(error = ERROR_BAD_FORMAT("invalid source address of mDNS response")); @@ -81,29 +81,30 @@ int HandleRecord(const struct sockaddr *from, fromAddrStr = fromAddr.ToString(); - entryType = (entry == MDNS_ENTRYTYPE_ANSWER) ? "answer" - : ((entry == MDNS_ENTRYTYPE_AUTHORITY) ? "authority" : "additional"); - if (type == MDNS_RECORDTYPE_PTR) + entryType = (aEntry == MDNS_ENTRYTYPE_ANSWER) ? "answer" + : ((aEntry == MDNS_ENTRYTYPE_AUTHORITY) ? "authority" : "additional"); + if (aType == MDNS_RECORDTYPE_PTR) { - mdns_string_t nameStr = mdns_record_parse_ptr(data, size, offset, length, nameBuffer, sizeof(nameBuffer)); + mdns_string_t nameStr = mdns_record_parse_ptr(aData, aSize, aOffset, aLength, nameBuffer, sizeof(nameBuffer)); borderAgent.mServiceName = std::string(nameStr.str, nameStr.length); borderAgent.mPresentFlags = BorderAgent::kServiceNameBit; // TODO(wgtdkp): add debug logging. } - else if (type == MDNS_RECORDTYPE_SRV) + else if (aType == MDNS_RECORDTYPE_SRV) { - mdns_record_srv_t server = mdns_record_parse_srv(data, size, offset, length, nameBuffer, sizeof(nameBuffer)); + mdns_record_srv_t server = + mdns_record_parse_srv(aData, aSize, aOffset, aLength, nameBuffer, sizeof(nameBuffer)); borderAgent.mPort = server.port; borderAgent.mPresentFlags |= BorderAgent::kPortBit; } - else if (type == MDNS_RECORDTYPE_A) + else if (aType == MDNS_RECORDTYPE_A) { struct sockaddr_storage addrStorage; struct sockaddr_in addr; std::string addrStr; - mdns_record_parse_a(data, size, offset, length, &addr); + mdns_record_parse_a(aData, aSize, aOffset, aLength, &addr); *reinterpret_cast(&addrStorage) = addr; if (fromAddr.Set(addrStorage) != ErrorCode::kNone) @@ -120,13 +121,13 @@ int HandleRecord(const struct sockaddr *from, borderAgent.mPresentFlags |= BorderAgent::kAddrBit; } } - else if (type == MDNS_RECORDTYPE_AAAA) + else if (aType == MDNS_RECORDTYPE_AAAA) { struct sockaddr_storage addrStorage; struct sockaddr_in6 addr; std::string addrStr; - mdns_record_parse_aaaa(data, size, offset, length, &addr); + mdns_record_parse_aaaa(aData, aSize, aOffset, aLength, &addr); *reinterpret_cast(&addrStorage) = addr; if (fromAddr.Set(addrStorage) != ErrorCode::kNone) @@ -139,13 +140,13 @@ int HandleRecord(const struct sockaddr *from, borderAgent.mAddr = addrStr; borderAgent.mPresentFlags |= BorderAgent::kAddrBit; } - else if (type == MDNS_RECORDTYPE_TXT) + else if (aType == MDNS_RECORDTYPE_TXT) { mdns_record_txt_t txtBuffer[128]; size_t parsed; - parsed = - mdns_record_parse_txt(data, size, offset, length, txtBuffer, sizeof(txtBuffer) / sizeof(mdns_record_txt_t)); + parsed = mdns_record_parse_txt(aData, aSize, aOffset, aLength, txtBuffer, + sizeof(txtBuffer) / sizeof(mdns_record_txt_t)); for (size_t i = 0; i < parsed; ++i) { @@ -317,7 +318,7 @@ int HandleRecord(const struct sockaddr *from, if (borderAgent.mPresentFlags != 0) { // Actualize Timestamp - borderAgent.mUpdateTimestamp.mTime = time(0); + borderAgent.mUpdateTimestamp.mTime = time(nullptr); borderAgent.mPresentFlags |= BorderAgent::kUpdateTimestampBit; } exit: diff --git a/src/app/mdns_handler.hpp b/src/app/mdns_handler.hpp index e6b231c51..2716fe011 100644 --- a/src/app/mdns_handler.hpp +++ b/src/app/mdns_handler.hpp @@ -35,16 +35,16 @@ namespace ot { namespace commissioner { -int HandleRecord(const struct sockaddr *from, - mdns_entry_type_t entry, - uint16_t type, - uint16_t rclass, - uint32_t ttl, - const void *data, - size_t size, - size_t offset, - size_t length, - void *userData); +int HandleRecord(const struct sockaddr *aFrom, + mdns_entry_type_t aEntry, + uint16_t aType, + uint16_t aRclass, + uint32_t aTtl, + const void *aData, + size_t aSize, + size_t aOffset, + size_t aLength, + void *aBorderAgent); } // namespace commissioner diff --git a/src/app/ps/persistent_storage_json.cpp b/src/app/ps/persistent_storage_json.cpp index ba02e0a72..5a25df71e 100644 --- a/src/app/ps/persistent_storage_json.cpp +++ b/src/app/ps/persistent_storage_json.cpp @@ -60,7 +60,6 @@ using SemaphoreStatus = ot::os::semaphore::SemaphoreStatus; PersistentStorageJson::PersistentStorageJson(std::string const &aFileName) : mFileName(aFileName) - , mCache() , mStorageLock() { SemaphoreOpen("thrcomm_json_storage", mStorageLock); diff --git a/src/app/ps/persistent_storage_json.hpp b/src/app/ps/persistent_storage_json.hpp index 7c3a7f8ed..5286d3297 100644 --- a/src/app/ps/persistent_storage_json.hpp +++ b/src/app/ps/persistent_storage_json.hpp @@ -113,15 +113,15 @@ class PersistentStorageJson : public PersistentStorage * Gets value by unique id from store * * @param[in] aId value's unique id - * @param[out] aValue value from registry + * @param[out] aRetValue value from registry * @return Status * @see Status * @see registry_entries.hpp */ - Status Get(RegistrarId const &aId, Registrar &aValue) override; - Status Get(DomainId const &aId, Domain &aValue) override; - Status Get(NetworkId const &aId, Network &aValue) override; - Status Get(BorderRouterId const &aId, BorderRouter &aValue) override; + Status Get(RegistrarId const &aId, Registrar &aRetValue) override; + Status Get(DomainId const &aId, Domain &aRetValue) override; + Status Get(NetworkId const &aId, Network &aRetValue) override; + Status Get(BorderRouterId const &aId, BorderRouter &aRetValue) override; /** * Updates value in store diff --git a/src/app/ps/registry.cpp b/src/app/ps/registry.cpp index 917ae4677..b77d211db 100644 --- a/src/app/ps/registry.cpp +++ b/src/app/ps/registry.cpp @@ -136,7 +136,8 @@ Registry::Status Registry::Add(BorderAgent const &aValue) { return Registry::Status::kError; } - else if (status == Registry::Status::kNotFound) + + if (status == Registry::Status::kNotFound) { DomainId domainId{EMPTY_ID}; status = MapStatus(mStorage->Add(dom, domainId)); @@ -179,7 +180,8 @@ Registry::Status Registry::Add(BorderAgent const &aValue) { throw status; } - else if (status == Registry::Status::kNotFound) + + if (status == Registry::Status::kNotFound) { NetworkId networkId{EMPTY_ID}; // It is possible we found the network by xpan @@ -778,9 +780,9 @@ Registry::Status Registry::DeleteBorderRoutersInDomain(const std::string &aDomai return status; } -Registry::Status Registry::Update(const Network &nwk) +Registry::Status Registry::Update(const Network &aNetwork) { - return MapStatus(mStorage->Update(nwk)); + return MapStatus(mStorage->Update(aNetwork)); } } // namespace persistent_storage diff --git a/src/app/ps/registry_entries.cpp b/src/app/ps/registry_entries.cpp index bc5f5d8ce..0be67fe82 100644 --- a/src/app/ps/registry_entries.cpp +++ b/src/app/ps/registry_entries.cpp @@ -147,8 +147,8 @@ void to_json(json &aJson, const Network &aValue) aJson = json{{JSON_ID, aValue.mId}, {JSON_DOM_REF, aValue.mDomainId}, {JSON_NAME, aValue.mName}, - {JSON_PAN, (std::string)aValue.mPan}, - {JSON_XPAN, (std::string)aValue.mXpan}, + {JSON_PAN, std::string(aValue.mPan)}, + {JSON_XPAN, std::string(aValue.mXpan)}, {JSON_CHANNEL, aValue.mChannel}, {JSON_MLP, aValue.mMlp}, {JSON_CCM, aValue.mCcm}}; @@ -233,7 +233,7 @@ void to_json(json &aJson, const BorderRouter &aValue) { // Note that UnixTime::operator std::string() does formatting // here using UnixTime::kFmtString - aJson[JSON_UPDATE_TIMESTAMP] = (std::string)aValue.mAgent.mUpdateTimestamp; + aJson[JSON_UPDATE_TIMESTAMP] = std::string(aValue.mAgent.mUpdateTimestamp); } } @@ -281,7 +281,7 @@ void from_json(const json &aJson, BorderRouter &aValue) { uint64_t value; aJson.at(JSON_ACTIVE_TIMESTAMP).get_to(value); - aValue.mAgent.mActiveTimestamp.Decode(value); + aValue.mAgent.mActiveTimestamp = Timestamp::Decode(value); aValue.mAgent.mPresentFlags |= BorderAgent::kActiveTimestampBit; } if (aJson.contains(JSON_PARTITION_ID)) @@ -434,10 +434,6 @@ BorderRouter::BorderRouter(BorderRouterId const &aId, NetworkId const &aNetworkI { } -BorderRouter::~BorderRouter() -{ -} - } // namespace persistent_storage } // namespace commissioner diff --git a/src/app/ps/registry_entries.hpp b/src/app/ps/registry_entries.hpp index 9c90617c8..8a73bca65 100644 --- a/src/app/ps/registry_entries.hpp +++ b/src/app/ps/registry_entries.hpp @@ -171,7 +171,7 @@ struct BorderRouter /** * Minimum polymorphic requirements. */ - virtual ~BorderRouter(); + virtual ~BorderRouter() = default; }; typedef std::vector BorderRouterArray; diff --git a/src/app/ps/semaphore.cpp b/src/app/ps/semaphore.cpp index 3fc97261a..34fef0b37 100644 --- a/src/app/ps/semaphore.cpp +++ b/src/app/ps/semaphore.cpp @@ -46,10 +46,10 @@ namespace semaphore { SemaphoreStatus SemaphoreOpen(std::string const &aName, Semaphore &aSem) { std::string semName = "Global\\" + aName; - aSem.platform = NULL; + aSem.platform = nullptr; - aSem.platform = CreateSemaphoreA(NULL, 1, 1, semName.c_str()); - if (aSem.platform == NULL) + aSem.platform = CreateSemaphoreA(nullptr, 1, 1, semName.c_str()); + if (aSem.platform == nullptr) { return kError; } @@ -60,13 +60,13 @@ SemaphoreStatus SemaphoreOpen(std::string const &aName, Semaphore &aSem) SemaphoreStatus SemaphoreClose(Semaphore &aSem) { CloseHandle(aSem.platform); - aSem.platform = NULL; + aSem.platform = nullptr; return kSuccess; } SemaphoreStatus SemaphorePost(Semaphore &aSem) { - if (ReleaseSemaphore(aSem.platform, 1, NULL) == 0) + if (ReleaseSemaphore(aSem.platform, 1, nullptr) == 0) { return kError; } @@ -87,7 +87,7 @@ SemaphoreStatus SemaphoreWait(Semaphore &aSem) SemaphoreStatus SemaphoreOpen(std::string const &aName, Semaphore &aSem) { std::string semName = "/" + aName; - aSem.mPlatform = NULL; + aSem.mPlatform = nullptr; aSem.mPlatform = sem_open(semName.c_str(), O_CREAT, S_IWUSR | S_IRUSR, 1); if (aSem.mPlatform == SEM_FAILED) @@ -101,7 +101,7 @@ SemaphoreStatus SemaphoreOpen(std::string const &aName, Semaphore &aSem) SemaphoreStatus SemaphoreClose(Semaphore &aSem) { sem_close(aSem.mPlatform); - aSem.mPlatform = NULL; + aSem.mPlatform = nullptr; return kSuccess; } diff --git a/src/library/coap.cpp b/src/library/coap.cpp index 8f574329f..8fbcb54ac 100644 --- a/src/library/coap.cpp +++ b/src/library/coap.cpp @@ -191,23 +191,22 @@ bool Message::IsTokenEqual(const Message &aMessage) const return GetToken() == aMessage.GetToken(); } -Error Message::AppendOption(OptionType aNumber, const OptionValue &aValue) +Error Message::AppendOption(OptionType aType, const OptionValue &aValue) { Error error; - VerifyOrExit(IsValidOption(aNumber, aValue), - error = ERROR_INVALID_ARGS("invalid CoAP option (number={})", aNumber)); + VerifyOrExit(IsValidOption(aType, aValue), error = ERROR_INVALID_ARGS("invalid CoAP option (number={})", aType)); - if (aNumber == OptionType::kUriPath) + if (aType == OptionType::kUriPath) { std::string uriPath = aValue.GetStringValue(); SuccessOrExit(error = NormalizeUriPath(uriPath)); - mOptions[aNumber] = mOptions[aNumber].GetStringValue() + uriPath; + mOptions[aType] = mOptions[aType].GetStringValue() + uriPath; } else { // We don't allow multiple options of the same type. - mOptions.emplace(aNumber, aValue); + mOptions.emplace(aType, aValue); } exit: @@ -673,7 +672,6 @@ void Coap::HandleRequest(const Request &aRequest) { LOG_INFO(LOG_REGION_COAP, "server(={}) handle request failed: {}", static_cast(this), error.ToString()); } - return; } void Coap::HandleResponse(Response &aResponse) diff --git a/src/library/coap_test.cpp b/src/library/coap_test.cpp index 2f8c3ecd8..4b70472db 100644 --- a/src/library/coap_test.cpp +++ b/src/library/coap_test.cpp @@ -291,7 +291,7 @@ class MockEndpoint : public Endpoint event_add(&mSendEvent, nullptr); } - ~MockEndpoint() override {} + ~MockEndpoint() override = default; void SetPeer(MockEndpoint *aPeer) { mPeer = aPeer; } diff --git a/src/library/commissioner_impl.cpp b/src/library/commissioner_impl.cpp index 50b84f118..79f043471 100644 --- a/src/library/commissioner_impl.cpp +++ b/src/library/commissioner_impl.cpp @@ -493,7 +493,7 @@ void CommissionerImpl::GetRawActiveDataset(Handler aHandler, uint16_t } } -void CommissionerImpl::SetActiveDataset(ErrorHandler aHandler, const ActiveOperationalDataset &aDataset) +void CommissionerImpl::SetActiveDataset(ErrorHandler aHandler, const ActiveOperationalDataset &aActiveDataset) { Error error; coap::Request request{coap::Type::kConfirmable, coap::Code::kPost}; @@ -502,27 +502,27 @@ void CommissionerImpl::SetActiveDataset(ErrorHandler aHandler, const ActiveOpera aHandler(HandleStateResponse(aResponse, aError)); }; - VerifyOrExit(aDataset.mPresentFlags & ActiveOperationalDataset::kActiveTimestampBit, + VerifyOrExit(aActiveDataset.mPresentFlags & ActiveOperationalDataset::kActiveTimestampBit, error = ERROR_INVALID_ARGS("Active Timestamp is mandatory for an Active Operational Dataset")); // TLVs affect connectivity are not allowed. - VerifyOrExit((aDataset.mPresentFlags & ActiveOperationalDataset::kChannelBit) == 0, + VerifyOrExit((aActiveDataset.mPresentFlags & ActiveOperationalDataset::kChannelBit) == 0, error = ERROR_INVALID_ARGS("Channel cannot be set with Active Operational Dataset, " "try setting with Pending Operational Dataset instead")); - VerifyOrExit((aDataset.mPresentFlags & ActiveOperationalDataset::kPanIdBit) == 0, + VerifyOrExit((aActiveDataset.mPresentFlags & ActiveOperationalDataset::kPanIdBit) == 0, error = ERROR_INVALID_ARGS("PAN ID cannot be set with Active Operational Dataset, " "try setting with Pending Operational Dataset instead")); - VerifyOrExit((aDataset.mPresentFlags & ActiveOperationalDataset::kMeshLocalPrefixBit) == 0, + VerifyOrExit((aActiveDataset.mPresentFlags & ActiveOperationalDataset::kMeshLocalPrefixBit) == 0, error = ERROR_INVALID_ARGS("Mesh-Local Prefix cannot be set with Active Operational Dataset, " "try setting with Pending Operational Dataset instead")); - VerifyOrExit((aDataset.mPresentFlags & ActiveOperationalDataset::kNetworkMasterKeyBit) == 0, + VerifyOrExit((aActiveDataset.mPresentFlags & ActiveOperationalDataset::kNetworkMasterKeyBit) == 0, error = ERROR_INVALID_ARGS("Network Master Key cannot be set with Active Operational Dataset, " "try setting with Pending Operational Dataset instead")); VerifyOrExit(IsActive(), error = ERROR_INVALID_STATE("the commissioner is not active")); SuccessOrExit(error = request.SetUriPath(uri::kMgmtActiveSet)); SuccessOrExit(error = AppendTlv(request, {tlv::Type::kCommissionerSessionId, GetSessionId()})); - SuccessOrExit(error = EncodeActiveOperationalDataset(request, aDataset)); + SuccessOrExit(error = EncodeActiveOperationalDataset(request, aActiveDataset)); #if OT_COMM_CONFIG_CCM_ENABLE if (IsCcmMode()) @@ -596,7 +596,7 @@ void CommissionerImpl::GetPendingDataset(Handler aHan } } -void CommissionerImpl::SetPendingDataset(ErrorHandler aHandler, const PendingOperationalDataset &aDataset) +void CommissionerImpl::SetPendingDataset(ErrorHandler aHandler, const PendingOperationalDataset &aPendingDataset) { Error error; coap::Request request{coap::Type::kConfirmable, coap::Code::kPost}; @@ -605,18 +605,18 @@ void CommissionerImpl::SetPendingDataset(ErrorHandler aHandler, const PendingOpe aHandler(HandleStateResponse(aResponse, aError)); }; - VerifyOrExit(aDataset.mPresentFlags & PendingOperationalDataset::kActiveTimestampBit, + VerifyOrExit(aPendingDataset.mPresentFlags & PendingOperationalDataset::kActiveTimestampBit, error = ERROR_INVALID_ARGS("Active Timestamp is mandatory for a Pending Operational Dataset")); - VerifyOrExit(aDataset.mPresentFlags & PendingOperationalDataset::kPendingTimestampBit, + VerifyOrExit(aPendingDataset.mPresentFlags & PendingOperationalDataset::kPendingTimestampBit, error = ERROR_INVALID_ARGS("Pending Timestamp is mandatory for a Pending Operational Dataset")); - VerifyOrExit(aDataset.mPresentFlags & PendingOperationalDataset::kDelayTimerBit, + VerifyOrExit(aPendingDataset.mPresentFlags & PendingOperationalDataset::kDelayTimerBit, error = ERROR_INVALID_ARGS("Delay Timer is mandatory for a Pending Operational Dataset")); VerifyOrExit(IsActive(), error = ERROR_INVALID_STATE("the commissioner is not active")); SuccessOrExit(error = request.SetUriPath(uri::kMgmtPendingSet)); SuccessOrExit(error = AppendTlv(request, {tlv::Type::kCommissionerSessionId, GetSessionId()})); - SuccessOrExit(error = EncodePendingOperationalDataset(request, aDataset)); + SuccessOrExit(error = EncodePendingOperationalDataset(request, aPendingDataset)); #if OT_COMM_CONFIG_CCM_ENABLE if (IsCcmMode()) diff --git a/src/library/commissioner_impl.hpp b/src/library/commissioner_impl.hpp index 0491b18b9..be18e5620 100644 --- a/src/library/commissioner_impl.hpp +++ b/src/library/commissioner_impl.hpp @@ -240,7 +240,7 @@ class CommissionerImpl : public Commissioner static Error MakeChannelMask(ByteArray &aBuf, uint32_t aChannelMask); - void HandleRlyRx(const coap::Request &aRequest); + void HandleRlyRx(const coap::Request &aRlyRx); void HandleJoinerSessionTimer(Timer &aTimer); diff --git a/src/library/cose.cpp b/src/library/cose.cpp index fd091aeae..9c79d0dc1 100644 --- a/src/library/cose.cpp +++ b/src/library/cose.cpp @@ -71,9 +71,9 @@ Error Sign1Message::Init(int aCoseInitFlags) Error Sign1Message::Serialize(ByteArray &aBuf) { size_t length; - length = COSE_Encode((HCOSE)mSign, nullptr, 0, 0) + 1; + length = COSE_Encode(reinterpret_cast(mSign), nullptr, 0, 0) + 1; aBuf.resize(length); - length = COSE_Encode((HCOSE)mSign, aBuf.data(), 0, aBuf.size()); + length = COSE_Encode(reinterpret_cast(mSign), aBuf.data(), 0, aBuf.size()); aBuf.resize(length); return ERROR_NONE; @@ -111,13 +111,13 @@ Error Sign1Message::Validate(const CborMap &aCborPublicKey) return error; } -Error Sign1Message::Validate(const mbedtls_pk_context &aPubKey) +Error Sign1Message::Validate(const mbedtls_pk_context &aPublicKey) { Error error; const struct mbedtls_ecp_keypair *eckey; // Accepts only EC keys - VerifyOrExit(mbedtls_pk_can_do(&aPubKey, MBEDTLS_PK_ECDSA) && (eckey = mbedtls_pk_ec(aPubKey)) != nullptr, + VerifyOrExit(mbedtls_pk_can_do(&aPublicKey, MBEDTLS_PK_ECDSA) && (eckey = mbedtls_pk_ec(aPublicKey)) != nullptr, error = ERROR_INVALID_ARGS("validate COSE SIGN1 message without valid EC public key")); VerifyOrExit(COSE_Sign0_validate_eckey(mSign, eckey, nullptr), diff --git a/src/library/socket.cpp b/src/library/socket.cpp index 4e31a6f8e..9e485b1f0 100644 --- a/src/library/socket.cpp +++ b/src/library/socket.cpp @@ -40,20 +40,25 @@ namespace commissioner { static uint16_t GetSockPort(const sockaddr_storage &aSockAddr) { + uint16_t port; + if (aSockAddr.ss_family == AF_INET) { auto &addr4 = *reinterpret_cast(&aSockAddr); - return ntohs(addr4.sin_port); + ExitNow(port = ntohs(addr4.sin_port)); } else if (aSockAddr.ss_family == AF_INET6) { auto &addr6 = *reinterpret_cast(&aSockAddr); - return ntohs(addr6.sin6_port); + ExitNow(port = ntohs(addr6.sin6_port)); } else { VerifyOrDie(false); } + +exit: + return port; } Socket::Socket(struct event_base *aEventBase) @@ -111,9 +116,9 @@ UdpSocket::UdpSocket(UdpSocket &&aOther) mbedtls_net_init(&aOther.mNetCtx); } -int UdpSocket::Connect(const std::string &aHost, uint16_t aPort) +int UdpSocket::Connect(const std::string &aPeerAddr, uint16_t aPeerPort) { - auto portStr = std::to_string(aPort); + auto portStr = std::to_string(aPeerPort); // Free the fd if already opened. mbedtls_net_free(&mNetCtx); @@ -123,7 +128,7 @@ int UdpSocket::Connect(const std::string &aHost, uint16_t aPort) } // Connect - int rval = mbedtls_net_connect(&mNetCtx, aHost.c_str(), portStr.c_str(), MBEDTLS_NET_PROTO_UDP); + int rval = mbedtls_net_connect(&mNetCtx, aPeerAddr.c_str(), portStr.c_str(), MBEDTLS_NET_PROTO_UDP); VerifyOrExit(rval == 0); VerifyOrExit((rval = mbedtls_net_set_nonblock(&mNetCtx)) == 0); @@ -144,9 +149,9 @@ int UdpSocket::Connect(const std::string &aHost, uint16_t aPort) return rval; } -int UdpSocket::Bind(const std::string &aBindIp, uint16_t aPort) +int UdpSocket::Bind(const std::string &aLocalAddr, uint16_t aLocalPort) { - auto portStr = std::to_string(aPort); + auto portStr = std::to_string(aLocalPort); // Free the fd if already opened. mbedtls_net_free(&mNetCtx); @@ -156,7 +161,7 @@ int UdpSocket::Bind(const std::string &aBindIp, uint16_t aPort) } // Bind - int rval = mbedtls_net_bind(&mNetCtx, aBindIp.c_str(), portStr.c_str(), MBEDTLS_NET_PROTO_UDP); + int rval = mbedtls_net_bind(&mNetCtx, aLocalAddr.c_str(), portStr.c_str(), MBEDTLS_NET_PROTO_UDP); VerifyOrExit(rval == 0); VerifyOrExit((rval = mbedtls_net_set_nonblock(&mNetCtx)) == 0); diff --git a/src/library/token_manager.cpp b/src/library/token_manager.cpp index a8344787a..e29faf1e6 100644 --- a/src/library/token_manager.cpp +++ b/src/library/token_manager.cpp @@ -299,7 +299,7 @@ Error TokenManager::MakeTokenRequest(ByteArray &aBuf, uint8_t tokenBuf[kMaxTokenRequestSize]; // Use the commissioner Id as kid(truncated to kMaxCoseKeyIdLength) - const ByteArray kid = {aId.begin(), aId.begin() + std::min(aId.size(), (size_t)kMaxCoseKeyIdLength)}; + const ByteArray kid = {aId.begin(), aId.begin() + std::min(aId.size(), static_cast(kMaxCoseKeyIdLength))}; VerifyOrExit(mbedtls_pk_can_do(&aPublicKey, MBEDTLS_PK_ECDSA), error = ERROR_INVALID_ARGS("the public key is not a ECDSA key")); diff --git a/src/library/token_manager.hpp b/src/library/token_manager.hpp index a9a09ef84..59f233d61 100644 --- a/src/library/token_manager.hpp +++ b/src/library/token_manager.hpp @@ -173,7 +173,7 @@ class TokenManager /* * Thread Constants. */ - static const size_t kMaxCoseKeyIdLength = 16; + static constexpr int kMaxCoseKeyIdLength = 16; // Move the resource from src to des, leaving the src invalid. static void MoveMbedtlsKey(mbedtls_pk_context &aDes, mbedtls_pk_context &aSrc); diff --git a/src/library/udp_proxy.cpp b/src/library/udp_proxy.cpp index f8aa20b49..f19ee53b9 100644 --- a/src/library/udp_proxy.cpp +++ b/src/library/udp_proxy.cpp @@ -159,7 +159,6 @@ void ProxyClient::HandleUdpRx(const coap::Request &aUdpRx) LOG_WARN(LOG_REGION_COAP, "client(={}) handle UDP_RX.ntf request failed: {}", static_cast(this), error.ToString()); } - return; } void ProxyClient::FetchMeshLocalPrefix(Commissioner::ErrorHandler aHandler) diff --git a/src/library/udp_proxy.hpp b/src/library/udp_proxy.hpp index 8011def7a..47ddd91f8 100644 --- a/src/library/udp_proxy.hpp +++ b/src/library/udp_proxy.hpp @@ -58,7 +58,7 @@ class ProxyEndpoint : public Endpoint } ~ProxyEndpoint() override = default; - Error Send(const ByteArray &aBuf, MessageSubType aSubType) override; + Error Send(const ByteArray &aRequest, MessageSubType aSubType) override; Address GetPeerAddr() const override { return mPeerAddr; } uint16_t GetPeerPort() const override { return mPeerPort; } diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index 28518779c..b55779996 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -42,6 +42,7 @@ endif() add_subdirectory(fmtlib/repo) if (OT_COMM_APP) + set(JSON_BuildTests OFF CACHE BOOL "Disable compiling json tests" FORCE) add_subdirectory(json/repo) endif()