Bugfix/fix redundant compiler flags (#658)

* Improved GCC compiler flags in ARM, OS, and test builds. Removed redundant flags, and made them consistent across various builds.

* Fixed redundant redeclaration of various functions detected by change in compiler flags.

* Fixed string truncation warning in bip-init detected by change in compiler flags.

* Fixed some set-but-not-used variables by creating dummy functions instead of using macros.

CMakeLists.txt

@@ -103,14 +103,15 @@ endif()
 
 if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "AppleClang" OR CMAKE_C_COMPILER_ID MATCHES "GNU")
   add_compile_options(-Wall -Wextra -pedantic)
-  add_compile_options(-Wfloat-equal -Wconversion -Wparentheses)
+  add_compile_options(-Wfloat-equal -Wconversion)
-  add_compile_options(-Wunused-value -Wreturn-type -Wswitch-default)
+  add_compile_options(-Wredundant-decls -Wswitch-default)
-  add_compile_options(-Wuninitialized -Winit-self)
+  # don't warn about conversion, sign, compares, long long and attributes
+  # since they are common in embedded
   add_compile_options(-Wno-sign-conversion -Wno-conversion)
   add_compile_options(-Wno-sign-compare -Wno-long-long)
-  add_compile_options(-Wno-implicit-fallthrough -Wno-attributes)
+  add_compile_options(-Wno-attributes)
-  add_compile_options(-Wunused-variable -Wunused-function)
+  # don't warn about implicit fallthrough since it's common in network protocols
-  add_compile_options(-Wunused-parameter)
+  add_compile_options(-Wno-implicit-fallthrough)
 endif()
 
 if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "AppleClang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU")

apps/Makefile

@@ -145,13 +145,14 @@ CSTANDARD ?= -std=gnu89
 #build for release (default) or debug
 OPTIMIZATION ?= -Os
 DEBUGGING ?=
-WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion -Wparentheses
+# enable all relevant warnings that find bugs
-WARNING_ALL += -pedantic -Wunused-parameter -Wunused-variable -Wreturn-type
+WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion
-WARNING_ALL += -Wunused-function -Wreturn-type -Wunused-value
+WARNING_ALL += -Wredundant-decls -Wswitch-default -pedantic
-WARNING_ALL += -Wswitch-default -Wuninitialized -Winit-self
+# don't warn about conversion, sign, compares, long long and attributes
+# since they are common in embedded
 WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
-WARNING_ALL += -Wno-long-long -Wno-implicit-fallthrough -Wno-attributes
+WARNING_ALL += -Wno-long-long -Wno-attributes
-#WARNING_ALL += -Wredundant-decls
+WARNING_ALL += -Wno-implicit-fallthrough
 #WARNING_ALL += -Werror
 WARNINGS ?= $(WARNING_ALL)
 # dead code removal

apps/lib/Makefile

@@ -11,7 +11,17 @@ BACNET_DEFINES ?=
 #build for release (default) or debug
 OPTIMIZATION ?= -Os
 DEBUGGING ?=
-WARNINGS ?= -Wall -Wmissing-prototypes
+# enable all relevant warnings that find bugs
+WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion
+WARNING_ALL += -Wredundant-decls -Wswitch-default -pedantic
+# don't warn about conversion, sign, compares, long long and attributes
+# since they are common in embedded
+WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
+WARNING_ALL += -Wno-long-long -Wno-attributes
+# don't warn about implicit fallthrough since it's common in network protocols
+WARNING_ALL += -Wno-implicit-fallthrough
+#WARNING_ALL += -Werror
+WARNINGS ?= $(WARNING_ALL)
 # dead code removal
 ifeq (${BUILD},debug)
 OPTIMIZATION = -O0

ports/at91sam7s/CMakeLists.txt

@@ -191,9 +191,20 @@ target_compile_definitions(${EXECUTABLE} PRIVATE
     -DBACAPP_MINIMAL
 )
 
-# inhibit pedantic warnings
 target_compile_options(${EXECUTABLE} PRIVATE
-    -Wall
+    -Wall -Wextra -pedantic
+    -Wfloat-equal -Wconversion -Wredundant-decls 
+    -Wswitch-default
+    # don't warn about conversion, sign, compares, long long and attributes
+    # since they are common in embedded
+    -Wno-sign-conversion 
+    -Wno-conversion
+    -Wno-sign-compare 
+    -Wno-long-long
+    -Wno-attributes
+    # don't warn about implicit fallthrough since it is common in network protocols
+    -Wno-implicit-fallthrough
+    # the older Atmel SDK does not meet coding guidelines
     -Wno-comment
     -Wno-missing-braces
     -Wno-unused-variable

ports/at91sam7s/Makefile

@@ -38,8 +38,22 @@ CFLAGS = -fno-common $(INCLUDES) $(BACNET_FLAGS) -g
 CFLAGS += -mno-thumb-interwork
 # dead code removal
 CFLAGS += -fdata-sections -ffunction-sections
-# warning control
+
-CFLAGS += -Wall
+# enable all relevant warnings that find bugs
+WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion
+WARNING_ALL += -Wredundant-decls -Wswitch-default -pedantic
+# don't warn about conversion, sign, compares, long long and attributes
+# since they are common in embedded
+WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
+WARNING_ALL += -Wno-long-long -Wno-attributes
+# don't warn about implicit fallthrough since it's common in network protocols
+WARNING_ALL += -Wno-implicit-fallthrough
+# the older Atmel SDK does not meet coding guidelines
+WARNING_ALL += -Wno-comment -Wno-missing-braces
+WARNING_ALL += -Wno-unused-variable -Wno-char-subscripts
+#WARNING_ALL += -Werror
+CFLAGS += $(WARNING_ALL)
+
 CFLAGS += -Wno-char-subscripts
 
 LIBRARY = lib$(TARGET).a

ports/linux/bip-init.c

@@ -899,7 +899,7 @@ bool bip_init(char *ifname)
     int sock_fd = -1;
 
     if (ifname) {
-        strncpy(BIP_Interface_Name, ifname, sizeof(BIP_Interface_Name));
+        snprintf(BIP_Interface_Name, sizeof(BIP_Interface_Name), "%s", ifname);
         bip_set_interface(ifname);
     } else {
         bip_set_interface(ifname_default());

ports/stm32f10x/Makefile

@@ -165,25 +165,25 @@ CFLAGS += -ffunction-sections -fdata-sections -fno-strict-aliasing
 CFLAGS += -fno-builtin
 # place uninitialized global variables in the data section of the object file.
 CFLAGS += -fno-common
-WARNING_ALL := -Wall
 # enable all relevant warnings that find bugs
-#WARNING_ALL += -pedantic -Wextra -Wfloat-equal -Wconversion -Wparentheses
+WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion
-#WARNING_ALL += -Wunused-parameter -Wunused-variable -Wreturn-type
+WARNING_ALL += -Wredundant-decls -Wswitch-default -pedantic
-#WARNING_ALL += -Wunused-function -Wreturn-type -Wunused-value
+# don't warn about conversion, sign, compares, long long and attributes
-#WARNING_ALL += -Wswitch-default -Wuninitialized -Winit-self
+# since they are common in embedded
-#WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
+WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
-#WARNING_ALL += -Wno-long-long
+WARNING_ALL += -Wno-long-long -Wno-attributes
-#WARNING_ALL += -Wredundant-decls
+# don't warn about implicit fallthrough since it's common in network protocols
+WARNING_ALL += -Wno-implicit-fallthrough
+# don't warn about missing braces since GCC is over-achiever for this
+WARNING_ALL += -Wno-missing-braces
+# don't warn about missing prototypes since STM32 library doesn't have some
+WARNING_ALL += -Wno-missing-prototypes
+# don't warn about array subscript being char
+WARNING_ALL += -Wno-char-subscripts
+# FIXME later
+WARNING_ALL += -Wno-unused-parameter
 #WARNING_ALL += -Werror
 CFLAGS += $(WARNING_ALL)
-# don't warn about missing braces since GCC is over-achiever for this
-CFLAGS += -Wno-missing-braces
-# don't warn about missing prototypes since STM32 library doesn't have some
-CFLAGS += -Wno-missing-prototypes
-# don't warn about array subscript being char
-CFLAGS += -Wno-char-subscripts
-# FIXME later
-CFLAGS += -Wno-unused-parameter
 #  -Wa,<options> Pass comma-separated <options> on to the assembler
 AFLAGS = -Wa,-ahls,-mapcs-32,-adhlns=$(<:.s=.lst)
 

ports/stm32f4xx/CMakeLists.txt

@@ -226,10 +226,24 @@ target_compile_definitions(${EXECUTABLE} PRIVATE
 
 # inhibit pedantic warnings
 target_compile_options(${EXECUTABLE} PRIVATE
-    -Wall
+    -Wall -Wextra -pedantic
+    -Wfloat-equal -Wconversion -Wredundant-decls 
+    -Wswitch-default
+    # don't warn about conversion, sign, compares, long long and attributes
+    # since they are common in embedded
+    -Wno-sign-conversion 
+    -Wno-conversion
+    -Wno-sign-compare 
+    -Wno-long-long
+    -Wno-attributes
+    # don't warn about implicit fallthrough since it is common in network protocols
+    -Wno-implicit-fallthrough
+    # the SDK does not meet coding guidelines
     -Wno-comment
     -Wno-missing-braces
     -Wno-unused-variable
+    -Wno-unused-parameter
+    -Wno-char-subscripts
 )
 
 target_include_directories(${EXECUTABLE} PRIVATE

ports/stm32f4xx/Makefile

@@ -177,14 +177,25 @@ CFLAGS += -ffunction-sections -fdata-sections -fno-strict-aliasing
 CFLAGS += -fno-builtin
 # place uninitialized global variables in the data section of the object file.
 CFLAGS += -fno-common
-# enable all relevant warnings
+# enable all relevant warnings that find bugs
-CFLAGS += -Wall
+WARNING_ALL := -Wall -Wextra -Wfloat-equal -Wconversion
+WARNING_ALL += -Wredundant-decls -Wswitch-default -pedantic
+# don't warn about conversion, sign, compares, long long and attributes
+# since they are common in embedded
+WARNING_ALL += -Wno-sign-conversion -Wno-conversion -Wno-sign-compare
+WARNING_ALL += -Wno-long-long -Wno-attributes
+# don't warn about implicit fallthrough since it's common in network protocols
+WARNING_ALL += -Wno-implicit-fallthrough
 # don't warn about missing braces since GCC is over-achiever for this
-CFLAGS += -Wno-missing-braces
+WARNING_ALL += -Wno-missing-braces
 # don't warn about missing prototypes since STM32 library doesn't have some
-CFLAGS += -Wno-missing-prototypes
+WARNING_ALL += -Wno-missing-prototypes
 # don't warn about array subscript being char
-CFLAGS += -Wno-char-subscripts
+WARNING_ALL += -Wno-char-subscripts
+# FIXME later
+WARNING_ALL += -Wno-unused-parameter
+#WARNING_ALL += -Werror
+CFLAGS += $(WARNING_ALL)
 
 #  -Wa,<options> Pass comma-separated <options> on to the assembler
 AFLAGS = -Wa,-ahls,-mapcs-32,-adhlns=$(<:.s=.lst)

src/bacnet/bacapp.c

@@ -2520,6 +2520,14 @@ bool bacapp_print_value(
 
     return retval;
 }
+#else
+bool bacapp_print_value(
+    FILE *stream, BACNET_OBJECT_PROPERTY_VALUE *object_value)
+{
+    (void)stream;
+    (void)object_value;
+    return false;
+}
 #endif
 
 #ifdef BACAPP_PRINT_ENABLED
@@ -2952,6 +2960,16 @@ bool bacapp_parse_application_data(BACNET_APPLICATION_TAG tag_number,
 
     return status;
 }
+#else
+bool bacapp_parse_application_data(BACNET_APPLICATION_TAG tag_number,
+    char *argv,
+    BACNET_APPLICATION_DATA_VALUE *value)
+{
+    (void)tag_number;
+    (void)argv;
+    (void)value;
+    return false;
+}
 #endif /* BACAPP_PRINT_ENABLED */
 
 /**

src/bacnet/bacapp.h

@@ -42,6 +42,12 @@
 #include "bacnet/calendar_entry.h"
 #include "bacnet/special_event.h"
 
+#ifndef BACAPP_PRINT_ENABLED
+#if PRINT_ENABLED
+#define BACAPP_PRINT_ENABLED
+#endif
+#endif
+
 struct BACnet_Application_Data_Value;
 typedef struct BACnet_Application_Data_Value {
     bool context_specific;      /* true if context specific data */
@@ -309,12 +315,6 @@ extern "C" {
         unsigned max_apdu_len,
         BACNET_PROPERTY_ID property);
 
-#ifndef BACAPP_PRINT_ENABLED
-#if PRINT_ENABLED
-#define BACAPP_PRINT_ENABLED
-#endif
-#endif
-
     BACNET_STACK_EXPORT
     int bacapp_snprintf_shift(
         int len, 
@@ -326,21 +326,16 @@ extern "C" {
         size_t str_len,
         BACNET_OBJECT_PROPERTY_VALUE * object_value);
 
-#ifdef BACAPP_PRINT_ENABLED
     BACNET_STACK_EXPORT
     bool bacapp_parse_application_data(
         BACNET_APPLICATION_TAG tag_number,
         char *argv,
         BACNET_APPLICATION_DATA_VALUE * value);
+
     BACNET_STACK_EXPORT
     bool bacapp_print_value(
         FILE * stream,
         BACNET_OBJECT_PROPERTY_VALUE * value);
-#else
-/* Provide harmless return values */
-#define bacapp_parse_application_data(x,y,z)   false
-#define bacapp_print_value(x,y) 			   false
-#endif
 
     BACNET_STACK_EXPORT
     bool bacapp_same_value(

src/bacnet/bacdcode.h

@@ -131,10 +131,6 @@ BACNET_STACK_DEPRECATED("Use bacnet_is_closing_tag() instead")
 BACNET_STACK_EXPORT
 bool decode_is_closing_tag(uint8_t *apdu);
 
-BACNET_STACK_EXPORT
-int encode_application_null(uint8_t *apdu);
-BACNET_STACK_EXPORT
-int encode_context_null(uint8_t *apdu, uint8_t tag_number);
 BACNET_STACK_EXPORT
 int encode_application_null(uint8_t *apdu);
 BACNET_STACK_EXPORT

src/bacnet/bactext.h

@@ -65,12 +65,6 @@ extern "C" {
         const char *search_name,
         unsigned *found_index);
     BACNET_STACK_EXPORT
-    const char *bactext_notify_type_name(
-        unsigned index);
-    BACNET_STACK_EXPORT
-    const char *bactext_event_type_name(
-        unsigned index);
-    BACNET_STACK_EXPORT
     bool bactext_property_name_proprietary(
         unsigned index);
     BACNET_STACK_EXPORT

src/bacnet/basic/object/bo.h

@@ -92,14 +92,6 @@ extern "C" {
         uint32_t object_instance,
         char *new_name);
 
-    BACNET_STACK_EXPORT
-    char *Binary_Output_Description(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Description_Set(
-        uint32_t instance,
-        char *new_name);
-
     BACNET_STACK_EXPORT
     char *Binary_Output_Inactive_Text(
         uint32_t instance);
@@ -115,25 +107,6 @@ extern "C" {
         uint32_t instance,
         char *new_name);
 
-    BACNET_STACK_EXPORT
-    int Binary_Output_Read_Property(
-        BACNET_READ_PROPERTY_DATA * rpdata);
-
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Write_Property(
-        BACNET_WRITE_PROPERTY_DATA * wp_data);
-
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Encode_Value_List(
-        uint32_t object_instance,
-        BACNET_PROPERTY_VALUE * value_list);
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Change_Of_Value(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    void Binary_Output_Change_Of_Value_Clear(
-        uint32_t instance);
-
     BACNET_STACK_EXPORT
     BACNET_BINARY_PV Binary_Output_Present_Value(
         uint32_t instance);
@@ -170,21 +143,6 @@ extern "C" {
         uint32_t object_instance,
         char *text_string);
 
-    BACNET_STACK_EXPORT
-    char *Binary_Output_Inactive_Text(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Inactive_Text_Set(
-        uint32_t instance,
-        char *new_name);
-    BACNET_STACK_EXPORT
-    char *Binary_Output_Active_Text(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Output_Active_Text_Set(
-        uint32_t instance,
-        char *new_name);
-
     BACNET_STACK_EXPORT
     BACNET_POLARITY Binary_Output_Polarity(
         uint32_t instance);

src/bacnet/basic/object/bv.h

@@ -71,14 +71,6 @@ extern "C" {
     char *Binary_Value_Name(
         uint32_t object_instance);
 
-    BACNET_STACK_EXPORT
-    char *Binary_Value_Description(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Value_Description_Set(
-        uint32_t instance,
-        char *new_name);
-
     BACNET_STACK_EXPORT
     BACNET_RELIABILITY Binary_Value_Reliability(
         uint32_t object_instance);
@@ -87,21 +79,6 @@ extern "C" {
         uint32_t object_instance,
         BACNET_RELIABILITY value);
 
-    BACNET_STACK_EXPORT
-    char *Binary_Value_Inactive_Text(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Value_Inactive_Text_Set(
-        uint32_t instance,
-        char *new_name);
-    BACNET_STACK_EXPORT
-    char *Binary_Value_Active_Text(
-        uint32_t instance);
-    BACNET_STACK_EXPORT
-    bool Binary_Value_Active_Text_Set(
-        uint32_t instance,
-        char *new_name);
-
     BACNET_STACK_EXPORT
     int Binary_Value_Read_Property(
         BACNET_READ_PROPERTY_DATA * rpdata);

src/bacnet/basic/object/client/device-client.c

@@ -102,8 +102,7 @@ static uint32_t Database_Revision = 0;
 /* Slave_Address_Binding */
 /* Profile_Name */
 
-/* local forward (semi-private) and external prototypes */
+/* external prototypes */
-int Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
 extern int Routed_Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
 extern bool Routed_Device_Write_Property_Local(
     BACNET_WRITE_PROPERTY_DATA *wp_data);

src/bacnet/basic/object/device.c

@@ -72,9 +72,7 @@
 #include "bacnet/basic/object/color_temperature.h"
 #endif
 
-/* local forward (semi-private) and external prototypes */
+/* external prototypes */
-int Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
-bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data);
 extern int Routed_Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
 extern bool Routed_Device_Write_Property_Local(
     BACNET_WRITE_PROPERTY_DATA *wp_data);

src/bacnet/basic/object/device.h

@@ -507,10 +507,6 @@ extern "C" {
     BACNET_ADDRESS *Get_Routed_Device_Address(
         int idx);
 
-    BACNET_STACK_EXPORT
-    void routed_get_my_address(
-        BACNET_ADDRESS * my_address);
-
     BACNET_STACK_EXPORT
     bool Routed_Device_Address_Lookup(
         int idx,

src/bacnet/basic/object/gateway/gw_device.c

@@ -62,9 +62,7 @@
 /* os specific includes */
 #include "bacnet/basic/sys/mstimer.h"
 
-/* local forward and external prototypes */
+/* forward prototypes */
-extern int Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
-extern bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data);
 int Routed_Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata);
 bool Routed_Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data);
 

src/bacnet/create_object.h

@@ -84,11 +84,6 @@ BACNET_STACK_EXPORT
 int create_object_error_ack_encode(
     uint8_t *apdu, uint8_t invoke_id, BACNET_CREATE_OBJECT_DATA *data);
 
-BACNET_STACK_EXPORT
-int create_object_ack_encode(
-    uint8_t *apdu, uint8_t invoke_id,
-    BACNET_CREATE_OBJECT_DATA *data);
-
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */