From 903677e97d8548de303a9063f44e80deed319a4b Mon Sep 17 00:00:00 2001 From: Saransh Chopra Date: Tue, 4 Nov 2025 02:45:54 +0100 Subject: [PATCH] Fix clang-tidy checks (#4379) * fix(.clang-tidy): `Checks` is supposed to be a comma separated list of checks * fix clang-tidy warning * fix a few lint warnings * run clang-format * more format --- .clang-tidy | 4 ++-- Fw/Types/GTest/Bytes.cpp | 3 ++- Os/Posix/test/ut/PosixConsoleTests.cpp | 7 ++++--- Os/test/ut/directory/DirectoryRules.cpp | 3 ++- Svc/ActivePhaser/test/ut/ActivePhaserTester.cpp | 6 ++++-- Svc/BufferLogger/test/ut/BufferLoggerTester.cpp | 3 ++- .../test/ut/SequenceFiles/BadDescriptorFile.cpp | 12 ++++++++---- Svc/ComLogger/test/ut/ComLoggerTester.cpp | 3 ++- Svc/ComStub/test/ut/ComStubTester.cpp | 3 ++- Svc/FileDownlink/test/ut/FileDownlinkTester.cpp | 2 +- Svc/PrmDb/PrmDbImpl.cpp | 3 ++- Utils/test/ut/TokenBucketTester.cpp | 4 ++-- 12 files changed, 33 insertions(+), 20 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2ce6d5166c..b90d94ddd0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -6,6 +6,6 @@ Checks: > modernize-redundant-void-arg, modernize-use-bool-literals, modernize-use-nullptr, - readability-braces-around-statements - -clang-analyzer-security.insecureAPI.rand, + readability-braces-around-statements, + -clang-analyzer-security.insecureAPI.rand WarningsAsErrors: '*' diff --git a/Fw/Types/GTest/Bytes.cpp b/Fw/Types/GTest/Bytes.cpp index 9752bc8f35..a7e531cb29 100644 --- a/Fw/Types/GTest/Bytes.cpp +++ b/Fw/Types/GTest/Bytes.cpp @@ -18,8 +18,9 @@ namespace GTest { void Bytes ::compare(const Bytes& expected, const Bytes& actual) { ASSERT_EQ(expected.size, actual.size); - for (size_t i = 0; i < expected.size; ++i) + for (size_t i = 0; i < expected.size; ++i) { ASSERT_EQ(expected.bytes[i], actual.bytes[i]) << "At i=" << i << "\n"; + } } } // namespace GTest diff --git a/Os/Posix/test/ut/PosixConsoleTests.cpp b/Os/Posix/test/ut/PosixConsoleTests.cpp index c54231c2de..b689480090 100644 --- a/Os/Posix/test/ut/PosixConsoleTests.cpp +++ b/Os/Posix/test/ut/PosixConsoleTests.cpp @@ -19,9 +19,10 @@ TEST(Nominal, SwitchStream) { } TEST(OffNominal, SwitchStream) { Os::Posix::Console::PosixConsole posix_console; - ASSERT_DEATH( - posix_console.setOutputStream(static_cast(3)), - "Posix/|\\Console.cpp:33"); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional death test + // intentional death test + ASSERT_DEATH(posix_console.setOutputStream(static_cast< // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) + Os::Posix::Console::PosixConsole::Stream>(3)), + "Posix/|\\Console.cpp:33"); } int main(int argc, char** argv) { diff --git a/Os/test/ut/directory/DirectoryRules.cpp b/Os/test/ut/directory/DirectoryRules.cpp index 96bffd068a..c03b7d2c96 100644 --- a/Os/test/ut/directory/DirectoryRules.cpp +++ b/Os/test/ut/directory/DirectoryRules.cpp @@ -193,7 +193,8 @@ void Os::Test::Directory::Tester::ReadAllFiles::action(Os::Test::Directory::Test // Number of files read should be the number of files in the directory minus the original seek position ASSERT_EQ(outFileCount, state.m_filenames.size()); for (FwSizeType i = 0; i < outFileCount; i++) { - ASSERT_TRUE(state.is_valid_filename(std::string(outArray[i].toChar()))); + ASSERT_TRUE( + state.is_valid_filename(std::string(outArray[i].toChar()))); // NOLINT(clang-analyzer-security.ArrayBound) } // readDirectory resets the seek position to the end state.m_seek_position = 0; diff --git a/Svc/ActivePhaser/test/ut/ActivePhaserTester.cpp b/Svc/ActivePhaser/test/ut/ActivePhaserTester.cpp index 5320a88138..8ede245c32 100644 --- a/Svc/ActivePhaser/test/ut/ActivePhaserTester.cpp +++ b/Svc/ActivePhaser/test/ut/ActivePhaserTester.cpp @@ -37,8 +37,9 @@ FauxPhaser::State FauxPhaser::run(U32 tick_of_cycle, U32 cycle_length) { (state == FauxPhaser::RUNNING || state == FauxPhaser::STARTING)) { bool finished_on_time = (current->length == current->actual_length); current++; - if (current < children.end()) + if (current < children.end()) { current->runtime = 0; + } // A child has run long, the machine is forced to idle before starting the next child to avoid a short cycle state = finished_on_time ? FauxPhaser::STARTING : FauxPhaser::IDLE; } @@ -53,8 +54,9 @@ FauxPhaser::State FauxPhaser::run(U32 tick_of_cycle, U32 cycle_length) { } else { EXPECT_FALSE(1); } - if (current < children.end()) + if (current < children.end()) { current->runtime++; + } return state; } diff --git a/Svc/BufferLogger/test/ut/BufferLoggerTester.cpp b/Svc/BufferLogger/test/ut/BufferLoggerTester.cpp index 2dc70eece9..054bf17322 100644 --- a/Svc/BufferLogger/test/ut/BufferLoggerTester.cpp +++ b/Svc/BufferLogger/test/ut/BufferLoggerTester.cpp @@ -125,8 +125,9 @@ void BufferLoggerTester ::dispatchOne() { } void BufferLoggerTester ::dispatchAll() { - while (this->component.m_queue.getMessagesAvailable() > 0) + while (this->component.m_queue.getMessagesAvailable() > 0) { this->dispatchOne(); + } } Fw::Time BufferLoggerTester ::generateTestTime(const U32 seconds) { diff --git a/Svc/CmdSequencer/test/ut/SequenceFiles/BadDescriptorFile.cpp b/Svc/CmdSequencer/test/ut/SequenceFiles/BadDescriptorFile.cpp index fec8e8040f..dca62b1892 100644 --- a/Svc/CmdSequencer/test/ut/SequenceFiles/BadDescriptorFile.cpp +++ b/Svc/CmdSequencer/test/ut/SequenceFiles/BadDescriptorFile.cpp @@ -37,8 +37,10 @@ void BadDescriptorFile ::serializeFPrime(Fw::SerializeBufferBase& buffer) { for (U32 record = 0; record < this->n; record++) { Fw::Time t(TimeBase::TB_WORKSTATION_TIME, 0, 0); // Force an invalid record descriptor - FPrime::Records::Descriptor descriptor = static_cast( - 10); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test + FPrime::Records::Descriptor descriptor = + static_cast( // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) + // intentional test + 10); FPrime::Records::serialize(descriptor, t, record, record + 1, buffer); } // CRC @@ -51,8 +53,10 @@ void BadDescriptorFile ::serializeAMPCS(Fw::SerializeBufferBase& buffer) { // Records for (U32 i = 0; i < this->n; ++i) { // Force an invalid time flag - const AMPCSSequence::Record::TimeFlag::t timeFlag = static_cast( - 10); // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test + const AMPCSSequence::Record::TimeFlag::t timeFlag = + static_cast( // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) + // intentional test + 10); const AMPCSSequence::Record::Time::t time = 0; const AMPCSSequence::Record::Opcode::t opcode = i; const U32 argument = i + 1; diff --git a/Svc/ComLogger/test/ut/ComLoggerTester.cpp b/Svc/ComLogger/test/ut/ComLoggerTester.cpp index fc6a8e76f2..28a1b2ed4f 100644 --- a/Svc/ComLogger/test/ut/ComLoggerTester.cpp +++ b/Svc/ComLogger/test/ut/ComLoggerTester.cpp @@ -49,8 +49,9 @@ void ComLoggerTester ::dispatchOne() { } void ComLoggerTester ::dispatchAll() { - while (this->comLogger.m_queue.getMessagesAvailable() > 0) + while (this->comLogger.m_queue.getMessagesAvailable() > 0) { this->dispatchOne(); + } } // ---------------------------------------------------------------------- diff --git a/Svc/ComStub/test/ut/ComStubTester.cpp b/Svc/ComStub/test/ut/ComStubTester.cpp index 79e802c523..4f41650d1e 100644 --- a/Svc/ComStub/test/ut/ComStubTester.cpp +++ b/Svc/ComStub/test/ut/ComStubTester.cpp @@ -113,7 +113,8 @@ void ComStubTester ::test_retry_async() { } // Retrying for as many times as the RETRY_LIMIT should be ok for (FwIndexType i = 0; i < this->component.RETRY_LIMIT; i++) { - invoke_to_drvAsyncSendReturnIn(0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); + invoke_to_drvAsyncSendReturnIn( + 0, buffers[i], Drv::ByteStreamStatus::SEND_RETRY); // NOLINT(clang-analyzer-security.ArrayBound) // Test we have indeed retried (data sent on drvAsyncSendOut) ASSERT_from_drvAsyncSendOut_SIZE(static_cast(i + 1)); ASSERT_from_drvAsyncSendOut(static_cast(i), buffers[i]); diff --git a/Svc/FileDownlink/test/ut/FileDownlinkTester.cpp b/Svc/FileDownlink/test/ut/FileDownlinkTester.cpp index f7830210c1..19a8b31d17 100644 --- a/Svc/FileDownlink/test/ut/FileDownlinkTester.cpp +++ b/Svc/FileDownlink/test/ut/FileDownlinkTester.cpp @@ -342,7 +342,7 @@ void FileDownlinkTester ::from_bufferSendOut_handler(const FwIndexType portNum, ASSERT_LT(buffers_index, FW_NUM_ARRAY_ELEMENTS(this->buffers)); // Copy buffer before recycling U8* data = new U8[buffer.getSize()]; - this->buffers[buffers_index] = data; + this->buffers[buffers_index] = data; // NOLINT(clang-analyzer-security.ArrayBound) buffers_index++; ::memcpy(data, buffer.getData(), buffer.getSize()); Fw::Buffer buffer_new = buffer; diff --git a/Svc/PrmDb/PrmDbImpl.cpp b/Svc/PrmDb/PrmDbImpl.cpp index a854898745..fb8b7c4d7b 100644 --- a/Svc/PrmDb/PrmDbImpl.cpp +++ b/Svc/PrmDb/PrmDbImpl.cpp @@ -507,8 +507,9 @@ void PrmDbImpl::clearDb(PrmDbType prmDbType) { bool PrmDbImpl::dbEqual() { for (FwSizeType i = 0; i < PRMDB_NUM_DB_ENTRIES; i++) { - if (!(this->m_dbStore1[i] == this->m_dbStore2[i])) + if (!(this->m_dbStore1[i] == this->m_dbStore2[i])) { return false; + } } return true; } diff --git a/Utils/test/ut/TokenBucketTester.cpp b/Utils/test/ut/TokenBucketTester.cpp index db1addf846..1e1844a022 100644 --- a/Utils/test/ut/TokenBucketTester.cpp +++ b/Utils/test/ut/TokenBucketTester.cpp @@ -90,8 +90,8 @@ void TokenBucketTester ::testReconfiguring() { ASSERT_EQ(bucket.getTokens(), newMaxTokens - 1); // set new rate, replenish quickly - while (bucket.trigger(Fw::Time(0, 0))) - ; + while (bucket.trigger(Fw::Time(0, 0))) { + } bucket.setReplenishInterval(1000000); U32 newRate = 2; bucket.setReplenishRate(newRate);