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

@@ -6,6 +6,6 @@ Checks: >
     modernize-redundant-void-arg,
     modernize-use-bool-literals,
     modernize-use-nullptr,
-    readability-braces-around-statements
+    readability-braces-around-statements,
-    -clang-analyzer-security.insecureAPI.rand,
+    -clang-analyzer-security.insecureAPI.rand
 WarningsAsErrors: '*'

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

Os/Posix/test/ut/PosixConsoleTests.cpp

@@ -19,9 +19,10 @@ TEST(Nominal, SwitchStream) {
 }
 TEST(OffNominal, SwitchStream) {
     Os::Posix::Console::PosixConsole posix_console;
-    ASSERT_DEATH(
+    // intentional death test
-        posix_console.setOutputStream(static_cast<Os::Posix::Console::PosixConsole::Stream>(3)),
+    ASSERT_DEATH(posix_console.setOutputStream(static_cast<  // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange)
-        "Posix/|\\Console.cpp:33");  // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional death test
+                                               Os::Posix::Console::PosixConsole::Stream>(3)),
+                 "Posix/|\\Console.cpp:33");
 }
 
 int main(int argc, char** argv) {

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;

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;
 }
 

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) {

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<FPrime::Records::Descriptor>(
+        FPrime::Records::Descriptor descriptor =
-            10);  // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test
+            static_cast<FPrime::Records::Descriptor>(  // 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<AMPCSSequence::Record::TimeFlag::t>(
+        const AMPCSSequence::Record::TimeFlag::t timeFlag =
-            10);  // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) intentional test
+            static_cast<AMPCSSequence::Record::TimeFlag::t>(  // 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;

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();
+    }
 }
 
 // ----------------------------------------------------------------------

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<U32>(i + 1));
         ASSERT_from_drvAsyncSendOut(static_cast<U32>(i), buffers[i]);

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;

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;
 }

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);