Compile root cmake project with -Wextra

CMakeLists.txt

@@ -8,20 +8,18 @@ project(FPrime C CXX)
 set(FPRIME_FRAMEWORK_PATH "${CMAKE_CURRENT_LIST_DIR}" CACHE PATH "Location of F prime framework" FORCE)
 set(FPRIME_PROJECT_ROOT "${CMAKE_CURRENT_LIST_DIR}" CACHE PATH "Root path of F prime project" FORCE)
 
-# This cmake project is only intended to be used by CI and developers to test F-prime
-# We enable -Wall on this particular project as a canary to warn about compilation errors without
+# This cmake project is only intended to be used by CI and developers to test F-prime. We enable
+# -Wall and -Wextra on this particular project as a canary to warn about compilation errors without
 # impacting real projects, where a warning from an untested compiler could break the build.
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Werror")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror")
 
-# We disable the buggy -Wstringop-truncation warning, which detects overflows when using strncpy and
-# strncat, for the gcc compiler. It triggers false positive warnings on some of our usages of strncpy,
-# despite the fact that we're explicitly NULL terminating the end of the buffer. Our usage of strncpy
-# should be refactored and eventually removed, but there's no drop-in replacement in the C stdlib, so
-# we will simply ignore the false positive warnings for now.
+# Disable the unused parameter warning in GCC for now. F' has a lot of interfaces, so unused method
+# parameters are common in the F prime codebase. Eventually all intentionally unused parameters
+# should be annotated to avoid this error.
 if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-stringop-truncation")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-stringop-truncation")
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-parameter")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter")
 endif()
 
 # Include the build for F prime.

Svc/ComLogger/ComLogger.cpp

@@ -7,20 +7,21 @@
 #include <Svc/ComLogger/ComLogger.hpp>
 #include <Fw/Types/BasicTypes.hpp>
 #include <Fw/Types/SerialBuffer.hpp>
+#include <Fw/Types/StringUtils.hpp>
 #include <Os/ValidateFile.hpp>
 #include <stdio.h>
 
 namespace Svc {
 
   // ----------------------------------------------------------------------
-  // Construction, initialization, and destruction 
+  // Construction, initialization, and destruction
   // ----------------------------------------------------------------------
 
   ComLogger ::
     ComLogger(const char* compName, const char* incomingFilePrefix, U32 maxFileSize, bool storeBufferLength) :
-      ComLoggerComponentBase(compName), 
+      ComLoggerComponentBase(compName),
       maxFileSize(maxFileSize),
-      fileMode(CLOSED), 
+      fileMode(CLOSED),
       byteCount(0),
       writeErrorOccurred(false),
       openErrorOccurred(false),
@@ -32,16 +33,14 @@ namespace Svc {
     else {
       FW_ASSERT(maxFileSize > sizeof(0), maxFileSize); // must be a positive integer
     }
-    FW_ASSERT((NATIVE_UINT_TYPE)strnlen(incomingFilePrefix, sizeof(this->filePrefix)) < sizeof(this->filePrefix), 
+    FW_ASSERT((NATIVE_UINT_TYPE)strnlen(incomingFilePrefix, sizeof(this->filePrefix)) < sizeof(this->filePrefix),
       (NATIVE_UINT_TYPE) strnlen(incomingFilePrefix, sizeof(this->filePrefix)), (NATIVE_UINT_TYPE) sizeof(this->filePrefix)); // ensure that file prefix is not too big
 
     // Set the file prefix:
-    memset(this->filePrefix, 0, sizeof(this->filePrefix)); // probably unnecessary, but I am paranoid.
-    U8* dest = (U8*) strncpy((char*) this->filePrefix, incomingFilePrefix, sizeof(this->filePrefix));
-    FW_ASSERT(dest == this->filePrefix, reinterpret_cast<U64>(dest), reinterpret_cast<U64>(this->filePrefix));
+    Fw::StringUtils::string_copy((char*) this->filePrefix, incomingFilePrefix, sizeof(this->filePrefix));
   }
 
-  void ComLogger :: 
+  void ComLogger ::
     init(
       NATIVE_INT_TYPE queueDepth, //!< The queue depth
       NATIVE_INT_TYPE instance //!< The instance number
@@ -56,7 +55,7 @@ namespace Svc {
     // Close file:
     // this->closeFile();
     // NOTE: the above did not work because we don't want to issue an event
-    // in the destructor. This can cause "virtual method called" segmentation 
+    // in the destructor. This can cause "virtual method called" segmentation
     // faults.
     // So I am copying part of that function here.
     if( OPEN == this->fileMode ) {
@@ -90,7 +89,7 @@ namespace Svc {
 
     // Get length of buffer:
     U32 size32 = data.getBuffLength();
-    // ComLogger only writes 16-bit sizes to save space 
+    // ComLogger only writes 16-bit sizes to save space
     // on disk:
     FW_ASSERT(size32 < 65536, size32);
     U16 size = size32 & 0xFFFF;
@@ -117,7 +116,7 @@ namespace Svc {
     }
   }
 
-  void ComLogger :: 
+  void ComLogger ::
     CloseFile_cmdHandler(
       FwOpcodeType opCode,
       U32 cmdSeq
@@ -148,15 +147,15 @@ namespace Svc {
     // Create filename:
     Fw::Time timestamp = getTime();
     memset(this->fileName, 0, sizeof(this->fileName));
-    bytesCopied = snprintf((char*) this->fileName, sizeof(this->fileName), "%s_%d_%d_%06d.com", 
+    bytesCopied = snprintf((char*) this->fileName, sizeof(this->fileName), "%s_%d_%d_%06d.com",
       this->filePrefix, (U32) timestamp.getTimeBase(), timestamp.getSeconds(), timestamp.getUSeconds());
 
     // "A return value of size or more means that the output was truncated"
     // See here: http://linux.die.net/man/3/snprintf
     FW_ASSERT( bytesCopied < sizeof(this->fileName) );
 
     // Create sha filename:
-    bytesCopied = snprintf((char*) this->hashFileName, sizeof(this->hashFileName), "%s_%d_%d_%06d.com%s", 
+    bytesCopied = snprintf((char*) this->hashFileName, sizeof(this->hashFileName), "%s_%d_%d_%06d.com%s",
       this->filePrefix, (U32) timestamp.getTimeBase(), timestamp.getSeconds(), timestamp.getUSeconds(), Utils::Hash::getFileExtensionString());
     FW_ASSERT( bytesCopied < sizeof(this->hashFileName) );
 
@@ -176,8 +175,8 @@ namespace Svc {
       this->byteCount = 0;
 
       // Set mode:
-      this->fileMode = OPEN; 
-    }    
+      this->fileMode = OPEN;
+    }
   }
 
   void ComLogger ::
@@ -208,7 +207,7 @@ namespace Svc {
   {
     if( this->storeBufferLength ) {
       U8 buffer[sizeof(size)];
-      Fw::SerialBuffer serialLength(&buffer[0], sizeof(size)); 
+      Fw::SerialBuffer serialLength(&buffer[0], sizeof(size));
       serialLength.serialize(size);
       if(this->writeToFile(serialLength.getBuffAddr(),
               static_cast<U16>(serialLength.getBuffLength()))) {
@@ -227,7 +226,7 @@ namespace Svc {
 
   bool ComLogger ::
     writeToFile(
-      void* data, 
+      void* data,
       U16 length
     )
   {
@@ -247,7 +246,7 @@ namespace Svc {
     return true;
   }
 
-  void ComLogger :: 
+  void ComLogger ::
     writeHashFile(
     )
   {