Bugfix/add makefile lint fix warnings (#39)

* fix warnings indicated by scan-build lint tool

* add lint target to makefile using scan-build tool

* add lint into build script stages (note: uses scan-build-7)

* fix potential use of memory after it is freed

.travis.yml

@@ -30,8 +30,7 @@ jobs:
     - stage: lint
       os: linux
       compiler: clang
-      # script: scan-build --status-bugs -analyze-headers -v make -j2 clean server
+      script: make clean lint
-      script: skip
     - stage: ports-arm
       os: linux
       before_script:

Makefile

@@ -115,6 +115,10 @@ tidy:
 	clang-tidy {} -fix-errors -checks="readability-braces-around-statements" \
 	-- -Isrc -Iports/linux \;
 
+.PHONY : lint
+lint:
+	scan-build --status-bugs -analyze-headers make -j2 clean server
+
 .PHONY: clean
 clean:
 	$(MAKE) -s -C src clean

src/bacnet/basic/binding/address.c

@@ -234,13 +234,15 @@ void address_mac_init(BACNET_MAC_ADDRESS *mac, uint8_t *adr, uint8_t len)
 {
     uint8_t i = 0;
 
-    if (mac && adr && (len <= sizeof(mac->adr))) {
+    if (mac) {
-        for (i = 0; i < len; i++) {
+        if (adr && (len <= sizeof(mac->adr))) {
-            mac->adr[i] = adr[i];
+            for (i = 0; i < len; i++) {
+                mac->adr[i] = adr[i];
+            }
+            mac->len = len;
+        } else {
+            mac->len = 0;
         }
-        mac->len = len;
-    } else {
-        mac->len = 0;
     }
 }
 

src/bacnet/basic/object/bacfile.c

@@ -394,7 +394,8 @@ uint32_t bacfile_instance_from_tsm(uint8_t invokeID)
             len = apdu_decode_confirmed_service_request(&apdu[0], apdu_len,
                 &service_data, &service_choice, &service_request,
                 &service_request_len);
-            if (service_choice == SERVICE_CONFIRMED_ATOMIC_READ_FILE) {
+            if ((len > 0) &&
+                (service_choice == SERVICE_CONFIRMED_ATOMIC_READ_FILE)) {
                 len = arf_decode_service_request(
                     service_request, service_request_len, &data);
                 if (len > 0) {

src/bacnet/basic/object/channel.c

@@ -1213,8 +1213,10 @@ bool Channel_Present_Value_Set(
             if (wp_data->priority != 6 /* reserved */) {
                 status =
                     Channel_Value_Copy(&Channel[index].Present_Value, value);
+                (void)status;
                 status = Channel_Write_Members(
                     &Channel[index], value, wp_data->priority);
+                (void)status;
                 status = true;
             } else {
                 /* Command priority 6 is reserved for use by Minimum On/Off

src/bacnet/basic/object/device.c

@@ -1757,11 +1757,11 @@ void Device_COV_Clear(BACNET_OBJECT_TYPE object_type, uint32_t object_instance)
 #if defined(INTRINSIC_REPORTING)
 void Device_local_reporting(void)
 {
-    struct object_functions *pObject;
+    struct object_functions *pObject = NULL;
-    uint32_t objects_count;
+    uint32_t objects_count = 0;
-    uint32_t object_instance;
+    uint32_t object_instance = 0;
-    BACNET_OBJECT_TYPE object_type;
+    BACNET_OBJECT_TYPE object_type = OBJECT_NONE;
-    uint32_t idx;
+    uint32_t idx = 0;
 
     objects_count = Device_Object_List_Count();
 

src/bacnet/basic/object/lc.c

@@ -901,7 +901,11 @@ bool Load_Control_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
             len = bacapp_decode_context_data(wp_data->application_data,
                 wp_data->application_data_len, &value,
                 PROP_REQUESTED_SHED_LEVEL);
-            if (value.context_tag == 0) {
+            if (len == BACNET_STATUS_ERROR) {
+                /* error! */
+                wp_data->error_class = ERROR_CLASS_PROPERTY;
+                wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE;
+            } else if (value.context_tag == 0) {
                 /* percent - Unsigned */
                 Requested_Shed_Level[object_index].type =
                     BACNET_SHED_TYPE_PERCENT;

src/bacnet/basic/object/lo.c

@@ -1034,6 +1034,15 @@ int Lighting_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
                     } else {
                         len = encode_application_null(&apdu[apdu_len]);
                     }
+                    /* add it if we have room */
+                    if ((apdu_len + len) < MAX_APDU) {
+                        apdu_len += len;
+                    } else {
+                        rpdata->error_code =
+                            ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
+                        apdu_len = BACNET_STATUS_ABORT;
+                        break;
+                    }
                 } else {
                     rpdata->error_class = ERROR_CLASS_PROPERTY;
                     rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX;

src/bacnet/basic/object/netport.c

@@ -930,9 +930,12 @@ bool Network_Port_IP_Subnet(
     if (index < BACNET_NETWORK_PORTS_MAX) {
         if (Object_List[index].Network_Type == PORT_TYPE_BIP) {
             prefix = Object_List[index].Network.IPv4.IP_Subnet_Prefix;
-            mask = (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+            if ((prefix > 0) && (prefix <= 32)) {
-            encode_unsigned32(ip_mask, mask);
+                mask = (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
-            status = octetstring_init(subnet_mask, ip_mask, sizeof(ip_mask));
+                encode_unsigned32(ip_mask, mask);
+                status = octetstring_init(subnet_mask, ip_mask,
+                    sizeof(ip_mask));
+            }
         }
     }
 
@@ -979,7 +982,7 @@ bool Network_Port_IP_Subnet_Prefix_Set(uint32_t object_instance, uint8_t value)
     index = Network_Port_Instance_To_Index(object_instance);
     if (index < BACNET_NETWORK_PORTS_MAX) {
         if (Object_List[index].Network_Type == PORT_TYPE_BIP) {
-            if (value <= 32) {
+            if ((value > 0) && (value <= 32)) {
                 if (Object_List[index].Network.IPv4.IP_Subnet_Prefix != value) {
                     Object_List[index].Changes_Pending = true;
                 }
@@ -1423,7 +1426,7 @@ bool Network_Port_IPv6_Subnet_Prefix_Set(
     index = Network_Port_Instance_To_Index(object_instance);
     if (index < BACNET_NETWORK_PORTS_MAX) {
         if (Object_List[index].Network_Type == PORT_TYPE_BIP6) {
-            if (value <= 128) {
+            if ((value > 0) && (value <= 128)) {
                 if (Object_List[index].Network.IPv6.IP_Subnet_Prefix != value) {
                     Object_List[index].Changes_Pending = true;
                 }

src/bacnet/basic/service/h_apdu.c

@@ -314,21 +314,31 @@ uint16_t apdu_decode_confirmed_service_request(uint8_t *apdu, /* APDU data */
 {
     uint16_t len = 0; /* counts where we are in PDU */
 
-    service_data->segmented_message = (apdu[0] & BIT(3)) ? true : false;
+    if (apdu_len >= 3) {
-    service_data->more_follows = (apdu[0] & BIT(2)) ? true : false;
+        service_data->segmented_message = (apdu[0] & BIT(3)) ? true : false;
-    service_data->segmented_response_accepted =
+        service_data->more_follows = (apdu[0] & BIT(2)) ? true : false;
-        (apdu[0] & BIT(1)) ? true : false;
+        service_data->segmented_response_accepted =
-    service_data->max_segs = decode_max_segs(apdu[1]);
+            (apdu[0] & BIT(1)) ? true : false;
-    service_data->max_resp = decode_max_apdu(apdu[1]);
+        service_data->max_segs = decode_max_segs(apdu[1]);
-    service_data->invoke_id = apdu[2];
+        service_data->max_resp = decode_max_apdu(apdu[1]);
-    len = 3;
+        service_data->invoke_id = apdu[2];
-    if (service_data->segmented_message) {
+        len = 3;
-        service_data->sequence_number = apdu[len++];
+        if (service_data->segmented_message) {
-        service_data->proposed_window_number = apdu[len++];
+            if (apdu_len >= (len+2)) {
+                service_data->sequence_number = apdu[len++];
+                service_data->proposed_window_number = apdu[len++];
+            } else {
+                return 0;
+            }
+        }
+        if (apdu_len >= (len+2)) {
+            *service_choice = apdu[len++];
+            *service_request = &apdu[len];
+            *service_request_len = apdu_len - len;
+        } else {
+            return 0;
+        }
     }
-    *service_choice = apdu[len++];
-    *service_request = &apdu[len];
-    *service_request_len = apdu_len - len;
 
     return len;
 }

src/bacnet/basic/service/h_rp_a.c

@@ -184,7 +184,6 @@ int rp_ack_fully_decode_service_request(
         vlen = rp1data.application_data_len;
         value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE));
         rp1_property->value = value;
-        old_value = value;
         while (value && vdata && (vlen > 0)) {
             if (IS_CONTEXT_SPECIFIC(*vdata)) {
                 len = bacapp_decode_context_data(

src/bacnet/basic/service/h_rpm.c

@@ -119,7 +119,6 @@ static int RPM_Encode_Property(
         return BACNET_STATUS_ABORT;
     }
     apdu_len += len;
-    len = 0;
     rpdata.error_class = ERROR_CLASS_OBJECT;
     rpdata.error_code = ERROR_CODE_UNKNOWN_OBJECT;
     rpdata.object_type = rpmdata->object_type;

src/bacnet/basic/service/h_rpm_a.c

@@ -25,7 +25,6 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <stdlib.h>
-#include <assert.h>
 #include "bacnet/config.h"
 #include "bacnet/bacdef.h"
 #include "bacnet/bacdcode.h"
@@ -66,7 +65,9 @@ int rpm_ack_decode_service_request(
     BACNET_APPLICATION_DATA_VALUE *value;
     BACNET_APPLICATION_DATA_VALUE *old_value;
 
-    assert(read_access_data != NULL);
+    if (!read_access_data) {
+        return 0;
+    }
     rpm_object = read_access_data;
     old_rpm_object = rpm_object;
     while (rpm_object && apdu_len) {
@@ -74,7 +75,11 @@ int rpm_ack_decode_service_request(
             &rpm_object->object_instance);
         if (len <= 0) {
             old_rpm_object->next = NULL;
-            free(rpm_object);
+            if (rpm_object != read_access_data) {
+                /* don't free original */
+                free(rpm_object);
+                rpm_object = NULL;
+            }
             break;
         }
         decoded_len += len;
@@ -94,6 +99,7 @@ int rpm_ack_decode_service_request(
                     rpm_object->listOfProperties = NULL;
                 }
                 free(rpm_property);
+                rpm_property = NULL;
                 break;
             }
             decoded_len += len;
@@ -108,7 +114,6 @@ int rpm_ack_decode_service_request(
                    more than one element to decode */
                 value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE));
                 rpm_property->value = value;
-                old_value = value;
                 while (value && (apdu_len > 0)) {
                     if (IS_CONTEXT_SPECIFIC(*apdu)) {
                         len = bacapp_decode_context_data(apdu, apdu_len, value,
@@ -199,8 +204,8 @@ void rpm_ack_print_data(BACNET_READ_ACCESS_DATA *rpm_data)
 #ifdef BACAPP_PRINT_ENABLED
     BACNET_OBJECT_PROPERTY_VALUE object_value; /* for bacapp printing */
 #endif
-    BACNET_PROPERTY_REFERENCE *listOfProperties;
+    BACNET_PROPERTY_REFERENCE *listOfProperties = NULL;
-    BACNET_APPLICATION_DATA_VALUE *value;
+    BACNET_APPLICATION_DATA_VALUE *value = NULL;
 #if PRINT_ENABLED
     bool array_value = false;
 #endif
@@ -283,6 +288,44 @@ void rpm_ack_print_data(BACNET_READ_ACCESS_DATA *rpm_data)
     }
 }
 
+/**
+ * Free the allocated memory from a ReadPropertyMultiple ACK.
+ * @param rpm_data - #BACNET_READ_ACCESS_DATA
+ * @return RPM data from the next element in the linked list
+ */
+static BACNET_READ_ACCESS_DATA *rpm_data_free(
+    BACNET_READ_ACCESS_DATA *rpm_data)
+{
+    BACNET_READ_ACCESS_DATA *old_rpm_data = NULL;
+    BACNET_PROPERTY_REFERENCE *rpm_property = NULL;
+    BACNET_PROPERTY_REFERENCE *old_rpm_property = NULL;
+    BACNET_APPLICATION_DATA_VALUE *value = NULL;
+    BACNET_APPLICATION_DATA_VALUE *old_value = NULL;
+
+    if (rpm_data) {
+        rpm_property = rpm_data->listOfProperties;
+        while (rpm_property) {
+            value = rpm_property->value;
+            while (value) {
+                old_value = value;
+                value = value->next;
+                free(old_value);
+                old_value = NULL;
+            }
+            old_rpm_property = rpm_property;
+            rpm_property = rpm_property->next;
+            free(old_rpm_property);
+            old_rpm_property = NULL;
+        }
+        old_rpm_data = rpm_data;
+        rpm_data = rpm_data->next;
+        free(old_rpm_data);
+        old_rpm_data = NULL;
+    }
+
+    return rpm_data;
+}
+
 /** Handler for a ReadPropertyMultiple ACK.
  * @ingroup DSRPM
  * For each read property, print out the ACK'd data for debugging,
@@ -300,12 +343,7 @@ void handler_read_property_multiple_ack(uint8_t *service_request,
     BACNET_CONFIRMED_SERVICE_ACK_DATA *service_data)
 {
     int len = 0;
-    BACNET_READ_ACCESS_DATA *rpm_data;
+    BACNET_READ_ACCESS_DATA * rpm_data;
-    BACNET_READ_ACCESS_DATA *old_rpm_data;
-    BACNET_PROPERTY_REFERENCE *rpm_property;
-    BACNET_PROPERTY_REFERENCE *old_rpm_property;
-    BACNET_APPLICATION_DATA_VALUE *value;
-    BACNET_APPLICATION_DATA_VALUE *old_value;
 
     (void)src;
     (void)service_data; /* we could use these... */
@@ -314,49 +352,18 @@ void handler_read_property_multiple_ack(uint8_t *service_request,
     if (rpm_data) {
         len = rpm_ack_decode_service_request(
             service_request, service_len, rpm_data);
-    }
+        if (len > 0) {
-#if 1
+            while (rpm_data) {
-    fprintf(stderr, "Received Read-Property-Multiple Ack!\n");
+                rpm_ack_print_data(rpm_data);
-#endif
+                rpm_data = rpm_data_free(rpm_data);
-    if (len > 0) {
-        while (rpm_data) {
-            rpm_ack_print_data(rpm_data);
-            rpm_property = rpm_data->listOfProperties;
-            while (rpm_property) {
-                value = rpm_property->value;
-                while (value) {
-                    old_value = value;
-                    value = value->next;
-                    free(old_value);
-                }
-                old_rpm_property = rpm_property;
-                rpm_property = rpm_property->next;
-                free(old_rpm_property);
             }
-            old_rpm_data = rpm_data;
+        } else {
-            rpm_data = rpm_data->next;
+    #if 1
-            free(old_rpm_data);
+            fprintf(stderr, "RPM Ack Malformed! Freeing memory...\n");
-        }
+    #endif
-    } else {
+            while (rpm_data) {
-#if 1
+                rpm_data = rpm_data_free(rpm_data);
-        fprintf(stderr, "RPM Ack Malformed! Freeing memory...\n");
-#endif
-        while (rpm_data) {
-            rpm_property = rpm_data->listOfProperties;
-            while (rpm_property) {
-                value = rpm_property->value;
-                while (value) {
-                    old_value = value;
-                    value = value->next;
-                    free(old_value);
-                }
-                old_rpm_property = rpm_property;
-                rpm_property = rpm_property->next;
-                free(old_rpm_property);
             }
-            old_rpm_data = rpm_data;
-            rpm_data = rpm_data->next;
-            free(old_rpm_data);
         }
     }
 }

src/bacnet/basic/sys/keylist.c

@@ -75,7 +75,7 @@ static int CheckArraySize(OS_Keylist list)
 {
     int new_size = 0; /* set it up so that no size change is the default */
     const int chunk = 8; /* minimum number of nodes to allocate memory for */
-    struct Keylist_Node **new_array; /* new array of nodes, if needed */
+    struct Keylist_Node **new_array = NULL; /* new array of nodes, if needed */
     int i; /* counter */
     if (!list) {
         return FALSE;
@@ -91,7 +91,8 @@ static int CheckArraySize(OS_Keylist list)
     }
     if (new_size) {
         /* Allocate more room for node pointer array */
-        new_array = calloc((size_t)new_size, sizeof(new_array));
+        new_array = calloc((size_t)new_size,
+            sizeof(struct Keylist_Node *));
 
         /* See if we got the memory we wanted */
         if (!new_array) {

src/bacnet/datalink/dlenv.c

@@ -220,7 +220,7 @@ static void dlenv_network_port_init(void)
     uint32_t address = 0;
     uint32_t broadcast = 0;
     uint32_t test_broadcast = 0;
-    uint32_t mask = 0;
+    uint32_t mask = 0xFFFFFFFE;
     uint16_t port = 0;
     uint8_t mac[4 + 2] = { 0 };
     uint8_t prefix = 0;
@@ -235,12 +235,13 @@ static void dlenv_network_port_init(void)
     memcpy(&mac[4], &port, 2);
     Network_Port_MAC_Address_Set(instance, &mac[0], 6);
     broadcast = bip_get_broadcast_addr();
-    for (prefix = 0; prefix < 32; prefix++) {
+    /* calculate the subnet prefix from the broadcast address */
-        mask = htonl((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
+    for (prefix = 1; prefix <= 32; prefix++) {
         test_broadcast = (address & mask) | (~mask);
         if (test_broadcast == broadcast) {
             break;
         }
+        mask = mask<<1;
     }
     Network_Port_IP_Subnet_Prefix_Set(instance, prefix);
     Network_Port_Link_Speed_Set(instance, 0.0);

src/bacnet/readrange.c

@@ -428,7 +428,6 @@ int rr_ack_decode_service_request(uint8_t *apdu,
             decode_unsigned(&apdu[len], len_value_type, &rrdata->FirstSequence);
     }
 
-    len = apdu_len; /* There should be nothing left to see here */
     return len;
 }
 

src/bacnet/timestamp.c

@@ -104,7 +104,7 @@ int bacapp_encode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
 
             default:
                 len = 0;
-                assert(0);
+                assert(len);
                 break;
         }
     }

src/bacnet/wpm.c

@@ -58,7 +58,7 @@ int wpm_decode_object_id(
     BACNET_OBJECT_TYPE object_type = OBJECT_NONE;
     uint16_t len = 0;
 
-    if (apdu && (apdu_len > 5) && wp_data) {
+    if (apdu && (apdu_len > 5)) {
         /* Context tag 0 - Object ID */
         len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
         if ((tag_number == 0) && (apdu_len > len)) {
@@ -66,26 +66,36 @@ int wpm_decode_object_id(
             if (apdu_len >= 4) {
                 len += decode_object_id(
                     &apdu[len], &object_type, &object_instance);
-                wp_data->object_type = object_type;
+                if (wp_data) {
-                wp_data->object_instance = object_instance;
+                    wp_data->object_type = object_type;
+                    wp_data->object_instance = object_instance;
+                }
                 apdu_len -= len;
             } else {
-                wp_data->error_code =
+                if (wp_data) {
-                    ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+                    wp_data->error_code =
+                        ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+                }
                 return BACNET_STATUS_REJECT;
             }
         } else {
-            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            if (wp_data) {
+                wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            }
             return BACNET_STATUS_REJECT;
         }
         /* just test for the next tag - no need to decode it here */
         /* Context tag 1: sequence of BACnetPropertyValue */
         if (apdu_len && !decode_is_opening_tag_number(&apdu[len], 1)) {
-            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            if (wp_data) {
+                wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            }
             return BACNET_STATUS_REJECT;
         }
     } else {
-        wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        if (wp_data) {
+            wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        }
         return BACNET_STATUS_REJECT;
     }
 
@@ -156,7 +166,9 @@ int wpm_decode_object_property(
             len--;
         }
     } else {
-        wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        if (wp_data) {
+            wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        }
         return BACNET_STATUS_REJECT;
     }