Compile root cmake project with -Wextra

CMakeLists.txt

@@ -8,21 +8,15 @@ 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")
-
-# 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.
-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")
-endif()
+#
+# Disable the unused parameter warning for now. F' has a lot of interfaces, so unused method
+# parameters are common in the F prime code base. Eventually all intentionally unused parameters
+# should be annotated to avoid this error.
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Werror -Wno-unused-parameter")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -Wno-unused-parameter")
 
 # Include the build for F prime.
 include("${CMAKE_CURRENT_LIST_DIR}/cmake/FPrime.cmake")

Os/Posix/Task.cpp

@@ -121,7 +121,9 @@ namespace Os {
         }
 
         pthread_t* tid = new pthread_t;
-        this->m_routineWrapper = {.routine = routine, .arg = arg};
+        this->m_routineWrapper.routine = routine;
+        this->m_routineWrapper.arg = arg;
+
         stat = pthread_create(tid,&att,pthread_entry_wrapper,&this->m_routineWrapper);
 
         switch (stat) {

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

cmake/Options.cmake

@@ -74,7 +74,7 @@ option(CMAKE_DEBUG_OUTPUT "Generate F prime's debug output while running CMake"
 option(FPRIME_ENABLE_FRAMEWORK_UTS "Enable framework UT generation" ON)
 if (NOT FPRIME_ENABLE_FRAMEWORK_UTS)
     message(WARNING "-DFPRIME_ENABLE_FRAMEWORK_UTS=OFF will be deprecated in a future release")
-endif() 
+endif()
 
 ####
 # `FPRIME_ENABLE_AUTOCODER_UTS:`
@@ -91,7 +91,7 @@ endif()
 option(FPRIME_ENABLE_AUTOCODER_UTS "Enable autocoder UT generation" ON)
 if (NOT FPRIME_ENABLE_AUTOCODER_UTS)
     message(WARNING "-DFPRIME_ENABLE_AUTOCODER_UTS=OFF will be deprecated in a future release")
-endif() 
+endif()
 
 ####
 # `SKIP_TOOLS_CHECK:`
@@ -127,13 +127,18 @@ endif()
 #
 # e.g. `-DCMAKE_BUILD_TYPE=TESTING`
 ####
-SET(CMAKE_CXX_FLAGS_TESTING "-g -DBUILD_UT -DPROTECTED=public -DPRIVATE=public -DSTATIC= -fprofile-arcs -ftest-coverage"
+SET(CMAKE_CXX_FLAGS_RELEASE "-std=c++03" CACHE STRING "C++ flags." FORCE)
+SET(CMAKE_C_FLAGS_RELEASE "-std=c99" CACHE STRING "C flags." FORCE)
+# Raise C++ standard to C++11 while unit testing to support googletest
+SET(CMAKE_CXX_FLAGS_TESTING "-std=c++11 -g -DBUILD_UT -DPROTECTED=public -DPRIVATE=public -DSTATIC= -fprofile-arcs -ftest-coverage"
     CACHE STRING "Testing C++ flags." FORCE)
-SET(CMAKE_C_FLAGS_TESTING "-g -DBUILD_UT -DPROTECTED=public -DPRIVATE=public -DSTATIC= -fprofile-arcs -ftest-coverage"
+SET(CMAKE_C_FLAGS_TESTING "-std=c99 -g -DBUILD_UT -DPROTECTED=public -DPRIVATE=public -DSTATIC= -fprofile-arcs -ftest-coverage"
     CACHE STRING "Testing C flags." FORCE)
 SET(CMAKE_EXE_LINKER_FLAGS_TESTING "" CACHE STRING "Testing linker flags." FORCE)
 SET(CMAKE_SHARED_LINKER_FLAGS_TESTING "" CACHE STRING "Testing linker flags." FORCE)
 MARK_AS_ADVANCED(
+    CMAKE_CXX_FLAGS_RELEASE
+    CMAKE_C_FLAGS_RELEASE
     CMAKE_CXX_FLAGS_TESTING
     CMAKE_C_FLAGS_TESTING
     CMAKE_EXE_LINKER_FLAGS_TESTING

cmake/platform/Darwin.cmake

@@ -19,16 +19,5 @@ if (NOT DEFINED FPRIME_USE_BAREMETAL_SCHEDULER)
    FIND_PACKAGE ( Threads REQUIRED )
 endif()
 
-
-# Darwin specific flags: shared, C, and C++ settings
-set(DARWIN_COMMON
-  "-Wall -Wextra -fno-builtin -fno-asm -Wno-unused-parameter -Wno-long-long"
-)
-set(CMAKE_C_FLAGS
-  "${CMAKE_C_FLAGS} ${DARWIN_COMMON} -std=c99 -pedantic -Werror-implicit-function-declaration -Wstrict-prototypes"
-)
-set(CMAKE_CXX_FLAGS
-  "${CMAKE_CXX_FLAGS} ${DARWIN_COMMON} -std=c++11"
-)
 # Add linux include path which is compatible with Darwin for StandardTypes.hpp
 include_directories(SYSTEM "${FPRIME_FRAMEWORK_PATH}/Fw/Types/Linux")