Werror: Enable warnings as errors

Added C/CXX warning messages similar to MRAA (w/ -Werror).

    * Added syslog warning for missing switch cases
    * Cleaned up uint vs int usage
    * Fixed redifinition errors for C structs
    * Added virtual destructors for base classes
    * Removed redundant CMAKE_CXX_FLAGS from COMPILE_FLAGS for all three
      wrapper languages.  The CMAKE_CXX_FLAGS were showing up twice in
      the compile commands for the wrappers.
    * Added CMake WERROR option to enable/disable warnings as errors for
      all targets.
    * Disable a handful of compiler warnings for the wrapper cxx files,
      this minimizes the number of warnings from auto-generated code).

Signed-off-by: Noel Eck <noel.eck@intel.com>

.travis.yml

@@ -36,11 +36,7 @@ script:
   - sudo make install
   - sudo ldconfig
   # Build/install UPM
-  - cd $UPM_ROOT && mkdir $UPM_BUILD && cd $_ && cmake -DNODE_ROOT_DIR:PATH="${NODE_ROOT_DIR}" -DBUILDSWIGJAVA=$BUILDJAVA -DBUILDEXAMPLES=ON -DBUILDJAVAEXAMPLES=$BUILDJAVA -DBUILDTESTS=ON ..
+  - cd $UPM_ROOT && mkdir $UPM_BUILD && cd $_ && cmake -DNODE_ROOT_DIR:PATH="${NODE_ROOT_DIR}" -DBUILDSWIGJAVA=$BUILDJAVA -DBUILDEXAMPLES=ON -DBUILDJAVAEXAMPLES=$BUILDJAVA -DBUILDTESTS=ON .. && sudo make install && sudo ldconfig && ctest --output-on-failure -E examplenames_js
-  - sudo make install
-  - sudo ldconfig
-  # Run UPM ctests
-  - ctest --output-on-failure -E examplenames_js
 addons:
   apt:
     sources:

CMakeLists.txt

@@ -13,6 +13,7 @@ option (BUILDJAVAEXAMPLES "Build java example jars" OFF)
 option (IPK "Generate IPK using CPack" OFF)
 option (RPM "Generate RPM using CPack" OFF)
 option (BUILDTESTS "Generate check-ups for upm" ON)
+option (WERROR "Make all warnings into errors." OFF)
 
 # Warn if building in source root
 if ("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_BINARY_DIR}")
@@ -22,6 +23,93 @@ endif ()
 # Appends the cmake/modules path to MAKE_MODULE_PATH variable.
 set (CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules ${CMAKE_MODULE_PATH})
 
+# Check if provided compiler supports target flag
+# usage:
+#       compiler_flag_supported(C/CXX is_supported flag)
+#
+# The result of output_variable is set to the sanitized flag name if supported
+# or cleared if the flag is not supported.
+function (compiler_flag_supported compiler output_variable flag)
+  # Currently only C and CXX compile flags
+  if (NOT ${compiler} MATCHES "C|CXX")
+    message (WARNING "Unknown compiler: ${compiler}")
+  else ()
+    string (REPLACE "-" "_" SANITIZED_FLAG_NAME "${flag}")
+    string (REPLACE "/" "_" SANITIZED_FLAG_NAME "${SANITIZED_FLAG_NAME}")
+    string (REPLACE "=" "_" SANITIZED_FLAG_NAME "${SANITIZED_FLAG_NAME}")
+    string (REPLACE " " "_" SANITIZED_FLAG_NAME "${SANITIZED_FLAG_NAME}")
+
+    # Disable messages from CHECK_C/XX_COMPILER_FLAGS macros
+    set (CMAKE_REQUIRED_QUIET_SAVED ${CMAKE_REQUIRED_QUIET})
+    set (CMAKE_REQUIRED_QUIET ON)
+
+    # C or CXX?
+    if (${compiler} STREQUAL C)
+      CHECK_C_COMPILER_FLAG("${flag}" HAS_${SANITIZED_FLAG_NAME})
+    elseif (${compiler} STREQUAL CXX)
+      CHECK_CXX_COMPILER_FLAG("${flag}" HAS_${SANITIZED_FLAG_NAME})
+    endif ()
+
+    # Restore previous CMAKE_REQUIRED_QUIET state
+    set (CMAKE_REQUIRED_QUIET ${CMAKE_REQUIRED_QUIET_SAVED})
+
+    # Does the compiler support this flag?
+    if (HAS_${SANITIZED_FLAG_NAME})
+      set ("${output_variable}" "${SANITIZED_FLAG_NAME}" PARENT_SCOPE)
+    else ()
+      set ("${output_variable}" "" PARENT_SCOPE)
+    endif()
+  endif()
+endfunction ()
+
+include (CheckCCompilerFlag)
+include (CheckCXXCompilerFlag)
+# Add scoped compile flag/s using add_compile_options.
+# This function checks to see if each flag is supported
+# by the compiler before setting the compile option.
+function (upm_add_compile_flags compiler)
+  # Iterate the flags, check if supported
+  foreach (flag ${ARGN})
+    # Check if this compile flag is supported
+    compiler_flag_supported(${compiler} is_supported ${flag})
+
+    # Add if supported, warn and skip if not supported
+    if (is_supported)
+      set (_TMP_COMPILER_FLAGS "${_TMP_COMPILER_FLAGS} ${flag}")
+    else ()
+      message (WARNING "${compiler} compiler does not support flag \"${flag}\"")
+    endif ()
+  endforeach (flag ${ARGN})
+
+  # Set the variable in the parent scope
+  set (CMAKE_${compiler}_FLAGS ${_TMP_COMPILER_FLAGS} PARENT_SCOPE)
+endfunction ()
+
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+
+# Compiler flags common to both C and CXX
+set (C_CXX_WARNING_FLAGS -Wall)
+
+# Errors as warnings?
+if (WERROR)
+  list (APPEND C_CXX_WARNING_FLAGS -Werror)
+  message (STATUS "Warnings as errors enabled (-Werror), disable with -DWERROR=off")
+endif (WERROR)
+
+# Set C compiler warning flags at top-level scope and emit a warning about
+# unsupported flags
+upm_add_compile_flags(C ${C_CXX_WARNING_FLAGS}
+  -Winit-self
+  -Wimplicit
+  -Wmissing-parameter-type)
+
+# Set CXX compiler warning flags at top-level scope and emit a warning about
+# unsupported flags
+upm_add_compile_flags(CXX ${C_CXX_WARNING_FLAGS}
+  -Wnon-virtual-dtor
+  -Woverloaded-virtual
+  -Wreorder)
+
 find_package (Threads REQUIRED)
 find_package (PkgConfig REQUIRED)
 

examples/c++/CMakeLists.txt

@@ -54,6 +54,12 @@ macro(add_example example_name)
   set(example_bin "${example_name}-example-cxx")
   get_module_name(${example_name} module_name)
   set(module_dir "${PROJECT_SOURCE_DIR}/src/${module_name}")
+
+  # nrf examples can flag a warning, make sure this isn't an error, currently
+  # this is done for all examples
+  set_source_files_properties(${example_src}
+      PROPERTIES COMPILE_FLAGS -Wno-tautological-compare)
+
   if (EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${example_src}"
      AND EXISTS ${module_dir}
      AND IS_DIRECTORY ${module_dir})

examples/c++/ds1307.cxx

@@ -35,7 +35,7 @@ void printTime(upm::DS1307 *rtc)
        << rtc->hours << ":" << rtc->minutes << ":" << rtc->seconds;
 
   if (rtc->amPmMode)
-    cout << (rtc->pm) ? " PM " : " AM ";
+    cout << ((rtc->pm) ? " PM " : " AM ");
 
   cout << endl;
 

examples/c++/nrf8001-broadcast.cxx

@@ -122,6 +122,8 @@ main(int argc, char **argv)
                             lib_aci_broadcast(10/* in seconds */, 0x0100 /* advertising interval 100ms */);
                             printf ("Broadcasting started\n");
                         break;
+                        default:
+                        break;
                     }
                 }
                 break; //ACI Device Started Event
@@ -156,6 +158,8 @@ main(int argc, char **argv)
                 case ACI_EVT_HW_ERROR:
                     printf ("ACI_EVT_HW_ERROR\n");
                     break;
+                default:
+                    break;
             }
         }
 

examples/c++/nrf8001-helloworld.cxx

@@ -239,6 +239,8 @@ main(int argc, char **argv)
                                 printf ("Advertising started \n");
                             }
                             break;
+                        default:
+                            break;
                     }
                 }
                 break; // ACI Device Started Event
@@ -335,7 +337,8 @@ main(int argc, char **argv)
                     lib_aci_connect(0/* in seconds, 0 means forever */, 0x0050 /* advertising interval 50ms*/);
                     printf ("Advertising started \n");
                 break;
-
+                default:
+                break;
             }
         }
 

examples/c++/pulsensor.cxx

@@ -43,7 +43,7 @@ sig_handler(int signo)
 
 void
 handler (clbk_data data) {
-    printf ("callback data (%d)\n", data);
+    printf ("callback data (%d)\n", data.is_heart_beat);
 }
 
 int

examples/c++/sx1276-fsk.cxx

@@ -95,7 +95,7 @@ int main(int argc, char **argv)
           // receiving
           cout << "Attempting to receive..." << endl;
           int rv;
-          if (rv = sensor->setRx(3000))
+          if ((rv = sensor->setRx(3000)))
             {
               cout << "setRx returned " << rv << endl;
             }

examples/c++/sx1276-lora.cxx

@@ -92,7 +92,7 @@ int main(int argc, char **argv)
           // receiving
           cout << "Attempting to receive..." << endl;
           int rv;
-          if (rv = sensor->setRx(3000))
+          if ((rv = sensor->setRx(3000)))
             {
               cout << "setRx returned " << rv << endl;
             }

src/CMakeLists.txt

@@ -55,6 +55,9 @@ macro (upm_target_link_libraries target_name)
   endforeach(_library ${ARGN})
 endmacro (upm_target_link_libraries target_name)
 
+# Selectively do not emit warnings from the SWIG-generated wrappers
+set (disabled_flags -Wno-delete-non-virtual-dtor -Wno-unused-function)
+
 # Create a single swig target for python
 macro(_upm_swig_python)
   include_directories (${CMAKE_CURRENT_SOURCE_DIR}/..)
@@ -85,6 +88,15 @@ macro(_upm_swig_python)
       "${PYTHON_INCLUDE_PATH}"
       "${PYTHON_INCLUDE_DIRS}")
 
+  # Turn off flags for wrapper
+  foreach(flag ${disabled_flags})
+    compiler_flag_supported(CXX is_supported ${flag})
+    if (is_supported)
+      target_compile_options(${python_wrapper_target}
+        PRIVATE "${flag}")
+    endif(is_supported)
+  endforeach(flag ${disabled_flags})
+
   # Add C++ comments to ALL python modules (requires doc build)
   if (BUILDDOC)
     # Python module depends on doxy2swig .i file generated from the monolithic doxygen xml file
@@ -97,7 +109,6 @@ macro(_upm_swig_python)
   # Python collateral names will be the same for python2/3 w/different library dirs
   set_target_properties (${python_wrapper_target} PROPERTIES
       OUTPUT_NAME _pyupm_${libname}
-      COMPILE_FLAGS "${CMAKE_CXX_FLAGS}"
       LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_PYTHON_BINARY_DIR})
 
   # Install .py's to python packages directory/upm
@@ -168,8 +179,17 @@ macro(upm_swig_node)
       "${NODE_INCLUDE_DIRS}"
     )
 
+    # Turn off flags for wrapper
+    foreach(flag ${disabled_flags})
+      compiler_flag_supported(CXX is_supported ${flag})
+      if (is_supported)
+        target_compile_options(${SWIG_MODULE_jsupm_${libname}_REAL_NAME}
+          PRIVATE "${flag}")
+      endif(is_supported)
+    endforeach(flag ${disabled_flags})
+
     set_target_properties (jsupm_${libname} PROPERTIES
-      COMPILE_FLAGS "${CMAKE_CXX_FLAGS} -DBUILDING_NODE_EXTENSION -DSWIG_V8_VERSION=${V8_VERSION_HEX}"
+      COMPILE_FLAGS "-DBUILDING_NODE_EXTENSION -DSWIG_V8_VERSION=${V8_VERSION_HEX}"
       PREFIX ""
       SUFFIX ".node"
     )
@@ -210,11 +230,20 @@ macro(upm_swig_java)
       "${JAVA_INCLUDE_PATH}"
      )
     set_target_properties (javaupm_${libname} PROPERTIES
-      COMPILE_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive -DJAVACALLBACK"
+      COMPILE_FLAGS "-fpermissive -DJAVACALLBACK"
       PREFIX "lib"
       SUFFIX ".so"
     )
 
+    # Turn off flags for wrapper
+    foreach(flag ${disabled_flags})
+      compiler_flag_supported(CXX is_supported ${flag})
+      if (is_supported)
+        target_compile_options(${SWIG_MODULE_javaupm_${libname}_REAL_NAME}
+          PRIVATE "${flag}")
+      endif(is_supported)
+    endforeach(flag ${disabled_flags})
+
     install (TARGETS javaupm_${libname} LIBRARY DESTINATION ${LIB_INSTALL_DIR})
     # Java jar files always need to go under lib/java, regardless of
     # architecture.

src/bacnetmstp/bacnetmstp.cxx

@@ -27,6 +27,7 @@
 #include <stdexcept>
 #include <string>
 #include <sstream>
+#include <syslog.h>
 
 #include "bacnetmstp.hpp"
 #include "handlers.h"
@@ -503,8 +504,10 @@ bool BACNETMSTP::dispatchRequest()
                          << (int)m_invokeID << endl;
 
                   break;
+                default:
+                  syslog(LOG_WARNING, "%s: switch case not defined",
+                          std::string(__FUNCTION__).c_str());
                 }
-
             }
           else if (tsm_invoke_id_free(m_invokeID))
             {

src/dfrph/dfrph.cxx

@@ -66,7 +66,7 @@ float DFRPH::pH(unsigned int samples)
     if (samples == 0) samples = 1;
 
     float ph = 0.0;
-    for (int i =0; i < samples; i++)
+    for (unsigned int i = 0; i < samples; i++)
     {
         dfrph_get_ph(_dev, &ph);
         ph_avg += ph;

src/ds18b20/ds18b20.c

@@ -37,11 +37,11 @@
 
 // an internal struct we use to store information on the devices
 // found during initialization
-typedef struct _ds18b20_info_t {
+struct _ds18b20_info_t {
   uint8_t id[ROMCODE_SIZE];          // 8-byte romcode id
   float temperature;
   DS18B20_RESOLUTIONS_T resolution;
-} ds18b20_info_t;
+};
 
 // internal utility function forward to read temperature from a single
 // device
@@ -121,7 +121,7 @@ ds18b20_context ds18b20_init(unsigned int uart)
 
           if (!dsPtr)
             {
-              printf("%s: realloc(%d) failed\n",
+              printf("%s: realloc(%zu) failed\n",
                      __FUNCTION__,
                      sizeof(ds18b20_info_t) * (dev->numDevices + 1));
               ds18b20_close(dev);

src/ds18b20/ds18b20.cxx

@@ -25,6 +25,7 @@
 #include <iostream>
 #include <time.h>
 #include <stdexcept>
+#include <syslog.h>
 
 #include "ds18b20.hpp"
 
@@ -214,6 +215,9 @@ float DS18B20::readSingleTemp(int index)
     case RESOLUTION_9BITS: frac &= 0x08; break;
     case RESOLUTION_10BITS: frac &= 0x0c; break;
     case RESOLUTION_11BITS: frac &= 0x0e; break;
+    default:
+        syslog(LOG_WARNING, "%s: switch case not defined",
+            std::string(__FUNCTION__).c_str());
     }
 
   // remove the fractional with extreme prejudice

src/gas/gas.hpp

@@ -54,7 +54,7 @@ class Gas {
         /**
          * Gas object destructor
          */
-        ~Gas();
+        virtual ~Gas();
 
         /**
          * Gets samples from the gas sensor according to the provided window and

src/ili9341/ili9341_gfx.hpp

@@ -46,6 +46,11 @@ namespace upm
              */
             GFX(int16_t w, int16_t h);
 
+            /**
+             * Empyt virtual destructor
+             */
+            virtual ~GFX() {};
+             
             /**
              * Sends a pixel color (RGB) to the driver chip. This must be
              * defined by the subclass (pure virtual function).

src/mpr121/mpr121.cxx

@@ -190,7 +190,6 @@ bool MPR121::configAN3944()
 
 void MPR121::readButtons()
 {
-  uint8_t rv;
   uint8_t buffer[2];
 
   // read in the 2 bytes at register 0x00-0x01, and setup the member

src/nrf8001/acilib.cpp

@@ -27,6 +27,8 @@
  * @brief Implementation of the acilib module.
  */
 
+#include <stdexcept>
+#include <syslog.h>
 
 #include "hal_platform.h"
 #include "aci.h"
@@ -438,6 +440,9 @@ void acil_decode_evt_command_response(uint8_t *buffer_in, aci_evt_params_cmd_rsp
       p_dtm_evt->evt_msb = (uint8_t)*(buffer_in + OFFSET_ACI_EVT_T_CMD_RSP + OFFSET_ACI_EVT_PARAMS_CMD_RSP_T_DTM_CMD + OFFSET_ACI_EVT_CMD_RSP_PARAMS_DTM_CMD_T_EVT_MSB);
       p_dtm_evt->evt_lsb = (uint8_t)*(buffer_in + OFFSET_ACI_EVT_T_CMD_RSP + OFFSET_ACI_EVT_PARAMS_CMD_RSP_T_DTM_CMD + OFFSET_ACI_EVT_CMD_RSP_PARAMS_DTM_CMD_T_EVT_LSB);
       break;
+    default:
+      syslog(LOG_WARNING, "%s: switch case not defined",
+        std::string(__FUNCTION__).c_str());
   }
 }
 

src/ozw/zwNode.cxx

@@ -187,7 +187,7 @@ void zwNode::dumpNode(bool all)
                   valueUnits.c_str(),
                   perms.c_str());
 
-          fprintf(stderr, "\t\t VID: %016" PRIx64 "\n",
+          fprintf(stderr, "\t\t VID: %016llx\n",
                   vid.GetId());
         }
     }

src/relay/relay.h

@@ -48,8 +48,6 @@ typedef struct _relay_context{
     mraa_gpio_context gpio;
 } *relay_context;
 
-typedef struct _relay_context *relay_context;
-
 relay_context relay_init(int pin);
 
 void relay_close(relay_context dev);

src/rotary/rotary.h

@@ -51,8 +51,6 @@ typedef struct _rotary_context {
 
 #define ROTARY_MAX_ANGLE 300
 
-typedef struct _rotary_context* rotary_context;
-
 rotary_context rotary_init(int pin, float aRef);
 
 void rotary_close(rotary_context dev);

src/ssd1351/ssd1351_gfx.hpp

@@ -310,7 +310,7 @@ class GFX {
         /**
          * GFX object destructor
          */
-        ~GFX ();
+        virtual ~GFX ();
 
         /**
          * Sends a pixel color (RGB) to the chip. Must be implemented by the

src/st7735/st7735_gfx.hpp

@@ -54,7 +54,7 @@ class GFX {
         /**
          * GFX object destructor
          */
-        ~GFX ();
+        virtual ~GFX ();
 
         /**
          * Sets the window address

src/t6713/t6713.cxx

@@ -22,11 +22,13 @@
  * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#include <iostream>
-#include <unistd.h>
-#include <stdlib.h>
-#include <math.h>
 #include <errno.h>
+#include <iostream>
+#include <math.h>
+#include <stdlib.h>
+#include <syslog.h>
+#include <unistd.h>
+
 #include <t6713.hpp>
 
 #define T6713_ADDR                                0x15
@@ -103,6 +105,7 @@ uint16_t T6713::getSensorData (MODBUS_COMMANDS cmd)
 		     //printf("\nRS485 mode set\n ");
 		break;
 		case I2C:
+                {
 		     //printf("\nI2C mode set\n");
 		     data = 0;
 		     runCommand(cmd);
@@ -113,7 +116,7 @@ uint16_t T6713::getSensorData (MODBUS_COMMANDS cmd)
 			 // TODO: need to handle this
 	  	     }
 		     RESPONSE * response  = new RESPONSE ;
-	             if(readBytes = i2c->read((uint8_t*)(response), sizeof(RESPONSE) ) !=  sizeof(RESPONSE))
+	             if((readBytes = i2c->read((uint8_t*)(response), sizeof(RESPONSE) ) !=  sizeof(RESPONSE)))
 		     {
 		     	 UPM_THROW("I2C read failed");
 			 // TODO
@@ -128,6 +131,10 @@ uint16_t T6713::getSensorData (MODBUS_COMMANDS cmd)
 		     delete(response); response=NULL;
 	             return(data);
 		break;
+                }
+                default:
+                    syslog(LOG_WARNING, "%s: switch case not defined",
+                        std::string(__FUNCTION__).c_str());
 	}
 	return 0;
 
@@ -218,7 +225,7 @@ STATUS T6713::getStatus()
                 UPM_THROW ("I2C error setting slave address");
 		//need to handle tnis
 	}
-	if(readBytes = i2c->read((uint8_t*) (response), sizeof(RESPONSE)) != sizeof(RESPONSE))
+	if((readBytes = i2c->read((uint8_t*) (response), sizeof(RESPONSE)) != sizeof(RESPONSE)))
 	{
                 UPM_THROW("I2C read failed");
 

src/tsl2561/tsl2561.c

@@ -208,7 +208,7 @@ upm_result_t tsl2561_compute_lux(const tsl2561_context dev, int *int_data) {
         ratio_1 = (channel1 << (LUX_RATIOSCALE+1)) / channel0;
 
     // round the ratio value
-    unsigned long ratio = (ratio_1 + 1) >> 1;
+    int64_t ratio = (ratio_1 + 1) >> 1;
     unsigned int b, m;
 
     // CS package
@@ -237,7 +237,7 @@ upm_result_t tsl2561_compute_lux(const tsl2561_context dev, int *int_data) {
     else if (ratio > LUX_K8C){
         b=LUX_B8C; m=LUX_M8C;
     }
-    uint64_t temp_lux = 0;
+    int64_t temp_lux = 0;
     temp_lux = ((channel0 * b) - (channel1 * m));
     // do not allow negative lux value
     if (temp_lux < 0) temp_lux = 0;

src/tsl2561/tsl2561.cxx

@@ -152,7 +152,7 @@ TSL2561::getLux()
     if (channel0 != 0) ratio1 = (channel1 << (LUX_RATIOSCALE+1)) / channel0;
 
     // round the ratio value
-    unsigned long ratio = (ratio1 + 1) >> 1;
+    int64_t ratio = (ratio1 + 1) >> 1;
 
     unsigned int b, m;
 
@@ -175,7 +175,7 @@ TSL2561::getLux()
     else if (ratio > LUX_K8C)
        {b=LUX_B8C; m=LUX_M8C;}
 
-    uint64_t tempLux = 0;
+    int64_t tempLux = 0;
     tempLux = ((channel0 * b) - (channel1 * m));
     // do not allow negative lux value
     if (tempLux < 0) tempLux = 0;