Add shadow variable and pedantic warnings (#2544)

* Add shadow variable and pedantic warnings

* Fix warnings from CI

* Fix failing unit tests

CMakeLists.txt

@@ -20,18 +20,17 @@ add_compile_options(
     -Wall
     -Wextra
     -Werror
-#    -pedantic
     $<$<COMPILE_LANGUAGE:CXX>:-Wold-style-cast>
     -Wno-unused-parameter
 )
 
-# Add -Wshadow only for framework code, not the unit tests
-# TODO: Next lines to be uncommented once FPP is not generating files with shadow variables
-# if (NOT BUILD_TESTING)
-#     add_compile_options(
-#         -Wshadow
-#     )
-# endif()
+# Add -Wshadow and -pedantic only for framework code, not the unit tests
+if (NOT BUILD_TESTING)
+    add_compile_options(
+        -Wshadow
+        -pedantic
+    )
+endif()
 
 # Turn on pedantic checks for clang, but disable specific checks that F' doesn't comply with.
 # GCC doesn't support disabling specific pedantic checks, so skip pedantic on GCC for now.

Os/MacOs/IPCQueueStub.cpp

@@ -112,7 +112,7 @@ namespace Os {
     if(pushSucceeded) {
       // Push worked - wake up a thread that might be waiting on
       // the other end of the queue:
-      NATIVE_INT_TYPE ret = pthread_cond_signal(queueNotEmpty);
+      ret = pthread_cond_signal(queueNotEmpty);
       FW_ASSERT(ret == 0, errno); // If this fails, something horrible happened.
     }
     else {
@@ -146,7 +146,7 @@ namespace Os {
 
     // If the queue is full, wait until a message is taken off the queue:
     while( queue->isFull() ) {
-      NATIVE_INT_TYPE ret = pthread_cond_wait(queueNotFull, queueLock);
+      ret = pthread_cond_wait(queueNotFull, queueLock);
       FW_ASSERT(ret == 0, errno);
     }
 
@@ -226,7 +226,7 @@ namespace Os {
 
         // Pop worked - wake up a thread that might be waiting on
         // the send end of the queue:
-        NATIVE_INT_TYPE ret = pthread_cond_signal(queueNotFull);
+        ret = pthread_cond_signal(queueNotFull);
         FW_ASSERT(ret == 0, errno); // If this fails, something horrible happened.
       }
       else {
@@ -275,7 +275,7 @@ namespace Os {
 
       // If the queue is empty, wait until a message is put on the queue:
       while( queue->isEmpty() ) {
-        NATIVE_INT_TYPE ret = pthread_cond_wait(queueNotEmpty, queueLock);
+        ret = pthread_cond_wait(queueNotEmpty, queueLock);
         FW_ASSERT(ret == 0, errno);
       }
 
@@ -289,7 +289,7 @@ namespace Os {
 
         // Pop worked - wake up a thread that might be waiting on
         // the send end of the queue:
-        NATIVE_INT_TYPE ret = pthread_cond_signal(queueNotFull);
+        ret = pthread_cond_signal(queueNotFull);
         FW_ASSERT(ret == 0, errno); // If this fails, something horrible happened.
       }
       else {

Svc/FileUplink/FileUplink.cpp

@@ -23,11 +23,11 @@ namespace Svc {
   FileUplink ::
     FileUplink(const char *const name) :
       FileUplinkComponentBase(name),
-      receiveMode(START),
-      lastSequenceIndex(0),
-      filesReceived(this),
-      packetsReceived(this),
-      warnings(this)
+      m_receiveMode(START),
+      m_lastSequenceIndex(0),
+      m_filesReceived(this),
+      m_packetsReceived(this),
+      m_warnings(this)
   {
 
   }
@@ -106,91 +106,91 @@ namespace Svc {
     this->log_WARNING_HI_InvalidReceiveMode_ThrottleClear();
     this->log_WARNING_HI_PacketOutOfBounds_ThrottleClear();
     this->log_WARNING_HI_PacketOutOfOrder_ThrottleClear();
-    this->packetsReceived.packetReceived();
-    if (this->receiveMode != START) {
-      this->file.osFile.close();
-      this->warnings.invalidReceiveMode(Fw::FilePacket::T_START);
+    this->m_packetsReceived.packetReceived();
+    if (this->m_receiveMode != START) {
+      this->m_file.osFile.close();
+      this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_START);
     }
-    const Os::File::Status status = this->file.open(startPacket);
+    const Os::File::Status status = this->m_file.open(startPacket);
     if (status == Os::File::OP_OK) {
       this->goToDataMode();
     }
     else {
-      this->warnings.fileOpen(this->file.name);
+      this->m_warnings.fileOpen(this->m_file.name);
       this->goToStartMode();
     }
   }
 
   void FileUplink ::
     handleDataPacket(const Fw::FilePacket::DataPacket& dataPacket)
   {
-    this->packetsReceived.packetReceived();
-    if (this->receiveMode != DATA) {
-      this->warnings.invalidReceiveMode(Fw::FilePacket::T_DATA);
+    this->m_packetsReceived.packetReceived();
+    if (this->m_receiveMode != DATA) {
+      this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_DATA);
       return;
     }
     const U32 sequenceIndex = dataPacket.asHeader().getSequenceIndex();
     this->checkSequenceIndex(sequenceIndex);
     const U32 byteOffset = dataPacket.getByteOffset();
     const U32 dataSize = dataPacket.getDataSize();
-    if (byteOffset + dataSize > this->file.size) {
-      this->warnings.packetOutOfBounds(sequenceIndex, this->file.name);
+    if (byteOffset + dataSize > this->m_file.size) {
+      this->m_warnings.packetOutOfBounds(sequenceIndex, this->m_file.name);
       return;
     }
-    const Os::File::Status status = this->file.write(
+    const Os::File::Status status = this->m_file.write(
         dataPacket.getData(),
         byteOffset,
         dataSize
     );
     if (status != Os::File::OP_OK) {
-      this->warnings.fileWrite(this->file.name);
+      this->m_warnings.fileWrite(this->m_file.name);
     }
   }
 
   void FileUplink ::
     handleEndPacket(const Fw::FilePacket::EndPacket& endPacket)
   {
-    this->packetsReceived.packetReceived();
-    if (this->receiveMode == DATA) {
-      this->filesReceived.fileReceived();
+    this->m_packetsReceived.packetReceived();
+    if (this->m_receiveMode == DATA) {
+      this->m_filesReceived.fileReceived();
       this->checkSequenceIndex(endPacket.asHeader().getSequenceIndex());
       this->compareChecksums(endPacket);
-      this->log_ACTIVITY_HI_FileReceived(this->file.name);
+      this->log_ACTIVITY_HI_FileReceived(this->m_file.name);
     }
     else {
-      this->warnings.invalidReceiveMode(Fw::FilePacket::T_END);
+      this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_END);
     }
     this->goToStartMode();
   }
 
   void FileUplink ::
     handleCancelPacket()
   {
-    this->packetsReceived.packetReceived();
+    this->m_packetsReceived.packetReceived();
     this->log_ACTIVITY_HI_UplinkCanceled();
     this->goToStartMode();
   }
 
   void FileUplink ::
     checkSequenceIndex(const U32 sequenceIndex)
   {
-    if (sequenceIndex != this->lastSequenceIndex + 1) {
-      this->warnings.packetOutOfOrder(
+    if (sequenceIndex != this->m_lastSequenceIndex + 1) {
+      this->m_warnings.packetOutOfOrder(
           sequenceIndex,
-          this->lastSequenceIndex
+          this->m_lastSequenceIndex
       );
     }
-    this->lastSequenceIndex = sequenceIndex;
+    this->m_lastSequenceIndex = sequenceIndex;
   }
 
   void FileUplink ::
     compareChecksums(const Fw::FilePacket::EndPacket& endPacket)
   {
     CFDP::Checksum computed, stored;
-    this->file.getChecksum(computed);
+    this->m_file.getChecksum(computed);
     endPacket.getChecksum(stored);
     if (computed != stored) {
-      this->warnings.badChecksum(
+      this->m_warnings.badChecksum(
           computed.getValue(),
           stored.getValue()
       );
@@ -200,16 +200,16 @@ namespace Svc {
   void FileUplink ::
     goToStartMode()
   {
-    this->file.osFile.close();
-    this->receiveMode = START;
-    this->lastSequenceIndex = 0;
+    this->m_file.osFile.close();
+    this->m_receiveMode = START;
+    this->m_lastSequenceIndex = 0;
   }
 
   void FileUplink ::
     goToDataMode()
   {
-    this->receiveMode = DATA;
-    this->lastSequenceIndex = 0;
+    this->m_receiveMode = DATA;
+    this->m_lastSequenceIndex = 0;
   }
 
 }

Svc/FileUplink/FileUplink.hpp

@@ -269,22 +269,22 @@ namespace Svc {
       // ----------------------------------------------------------------------
 
       //! The receive mode
-      ReceiveMode receiveMode;
+      ReceiveMode m_receiveMode;
 
       //! The sequence index of the last packet received
-      U32 lastSequenceIndex;
+      U32 m_lastSequenceIndex;
 
       //! The file being assembled
-      File file;
+      File m_file;
 
       //! The total number of files received
-      FilesReceived filesReceived;
+      FilesReceived m_filesReceived;
 
       //! The total number of cancel packets
-      PacketsReceived packetsReceived;
+      PacketsReceived m_packetsReceived;
 
       //! The total number of warnings
-      Warnings warnings;
+      Warnings m_warnings;
 
   };
 

Svc/FileUplink/Warnings.cpp

@@ -19,7 +19,7 @@ namespace Svc {
   {
     this->m_fileUplink->log_WARNING_HI_InvalidReceiveMode(
         static_cast<U32>(packetType),
-        static_cast<U32>(m_fileUplink->receiveMode)
+        static_cast<U32>(m_fileUplink->m_receiveMode)
     );
     this->warning();
   }
@@ -71,7 +71,7 @@ namespace Svc {
     )
   {
     this->m_fileUplink->log_WARNING_HI_BadChecksum(
-        this->m_fileUplink->file.name,
+        this->m_fileUplink->m_file.name,
         computed,
         read
     );

Svc/FileUplink/test/ut/FileUplinkTester.cpp

@@ -39,7 +39,7 @@ namespace Svc {
   FileUplinkTester ::
     ~FileUplinkTester()
   {
-    this->component.file.osFile.close();
+    this->component.m_file.osFile.close();
   }
 
   // ----------------------------------------------------------------------
@@ -96,7 +96,7 @@ namespace Svc {
     ASSERT_EVENTS_FileReceived(0, destPath);
 
     // Assert we are back in START mode
-    ASSERT_EQ(FileUplink::START, this->component.receiveMode);
+    ASSERT_EQ(FileUplink::START, this->component.m_receiveMode);
 
     // Verify the file data
     this->verifyFileData(destPath, linearPacketData, fileSize);
@@ -182,7 +182,7 @@ namespace Svc {
     ASSERT_EVENTS_SIZE(1);
     ASSERT_EVENTS_FileOpenError(0, destPath);
 
-    ASSERT_EQ(FileUplink::START, this->component.receiveMode);
+    ASSERT_EQ(FileUplink::START, this->component.m_receiveMode);
 
   }
 
@@ -205,7 +205,7 @@ namespace Svc {
     ASSERT_EVENTS_SIZE(0);
 
     // Close the file so writing will fail
-    this->component.file.osFile.close();
+    this->component.m_file.osFile.close();
 
     // Send the data packet (packet 1)
     const size_t byteOffset = PACKET_SIZE;
@@ -251,7 +251,7 @@ namespace Svc {
         FileUplink::DATA
     );
 
-    ASSERT_EQ(FileUplink::DATA, this->component.receiveMode);
+    ASSERT_EQ(FileUplink::DATA, this->component.m_receiveMode);
 
     this->removeFile(destPath);
   }
@@ -299,7 +299,7 @@ namespace Svc {
         FileUplink::START
     );
 
-    ASSERT_EQ(FileUplink::START, this->component.receiveMode);
+    ASSERT_EQ(FileUplink::START, this->component.m_receiveMode);
 
   }
 
@@ -405,8 +405,8 @@ namespace Svc {
     ASSERT_EVENTS_UplinkCanceled_SIZE(1);
 
     // Check component state
-    ASSERT_EQ(0U, this->component.lastSequenceIndex);
-    ASSERT_EQ(FileUplink::START, this->component.receiveMode);
+    ASSERT_EQ(0U, this->component.m_lastSequenceIndex);
+    ASSERT_EQ(FileUplink::START, this->component.m_receiveMode);
 
     // Remove the file
     this->removeFile("test.bin");
@@ -452,8 +452,8 @@ namespace Svc {
     ASSERT_EVENTS_UplinkCanceled_SIZE(1);
 
     // Check component state
-    ASSERT_EQ(0U, this->component.lastSequenceIndex);
-    ASSERT_EQ(FileUplink::START, this->component.receiveMode);
+    ASSERT_EQ(0U, this->component.m_lastSequenceIndex);
+    ASSERT_EQ(FileUplink::START, this->component.m_receiveMode);
 
     // Remove the file
     this->removeFile("test.bin");