Feature/app data buffer check (#79)

* Added comments and buffer overflow checks

* Removed backslashs from C-code.

src/bacnet/bacprop.c

@@ -62,6 +62,15 @@ PROP_TAG_DATA bacnet_object_device_property_tag_map[] = {
     { -1, -1 }
 };
 
+/**
+ * Search for the given index in the data list.
+ *
+ * @param data_list  Pointer to the list.
+ * @param index  Index to search for.
+ * @param default_ret  Default return value.
+ *
+ * @return Value found or the default value.
+ */
 signed bacprop_tag_by_index_default(
     PROP_TAG_DATA *data_list, signed index, signed default_ret)
 {
@@ -80,6 +89,14 @@ signed bacprop_tag_by_index_default(
     return pUnsigned ? pUnsigned : default_ret;
 }
 
+/**
+ * Return the value of the given property, if available.
+ *
+ * @param type Object type
+ * @param prop Property
+ *
+ * @return Value found or -1
+ */
 signed bacprop_property_tag(BACNET_OBJECT_TYPE type, signed prop)
 {
     switch (type) {

src/bacnet/bacstr.c

@@ -723,22 +723,23 @@ bool octetstring_init(
 {
     bool status = false; /* return value */
     size_t i; /* counter */
+    uint8_t *pb = NULL;
 
     if (octet_string && (length <= MAX_OCTET_STRING_BYTES)) {
         octet_string->length = 0;
         if (value) {
+            pb = octet_string->value;
             for (i = 0; i < MAX_OCTET_STRING_BYTES; i++) {
                 if (i < length) {
-                    octet_string->value[i] = value[i];
+                    *pb = value[i];
                 } else {
-                    octet_string->value[i] = 0;
+                    *pb = 0;
                 }
+                pb++;
             }
             octet_string->length = length;
         } else {
-            for (i = 0; i < MAX_OCTET_STRING_BYTES; i++) {
+            memset(octet_string->value, 0, MAX_OCTET_STRING_BYTES);
-                octet_string->value[i] = 0;
-            }
         }
         status = true;
     }

src/bacnet/basic/binding/address.c

@@ -84,11 +84,23 @@ static struct Address_Cache_Entry {
 #define BAC_ADDR_SHORT_TIME BAC_ADDR_SECS_1HOUR
 #define BAC_ADDR_FOREVER 0xFFFFFFFF /* Permenant entry */
 
+/**
+ * @brief Set the index of the first (top) address being protected.
+ *
+ * @param top_protected_entry_index  top protected index [0..n-1]
+ */
 void address_protected_entry_index_set(uint32_t top_protected_entry_index)
 {
-    Top_Protected_Entry = top_protected_entry_index;
+    if (top_protected_entry_index <= (MAX_ADDRESS_CACHE - 1)) {
+        Top_Protected_Entry = top_protected_entry_index;
+    }
 }
 
+/**
+ * @brief Set the address of our own device.
+ *
+ * @param own_id  Own device id
+ */
 void address_own_device_id_set(uint32_t own_id)
 {
     Own_Device_ID = own_id;
@@ -182,9 +194,8 @@ void address_remove_device(uint32_t device_id)
  * entry available to free up. Does not check for free entries as it is
  * assumed we are calling this due to the lack of those.
  *
- * @return Candidate for deletation.
+ * @return Pointer to the entry that has been removed or NULL.
  */
-
 static struct Address_Cache_Entry *address_remove_oldest(void)
 {
     struct Address_Cache_Entry *pMatch;
@@ -383,11 +394,10 @@ static void address_file_init(const char *pFilename)
 }
 #endif
 
-/****************************************************************************
+/**
- * Clear down the cache and make sure the full complement of entries are    *
+ * Clear down the cache and make sure the full complement of entries are
- * available. Assume no persistance of memory.                              *
+ * available. Assume no persistance of memory.
- ****************************************************************************/
+ */
-
 void address_init(void)
 {
     struct Address_Cache_Entry *pMatch;
@@ -405,14 +415,13 @@ void address_init(void)
     return;
 }
 
-/****************************************************************************
+/**
- * Clear down the cache of any non bound, expired  or reserved entries.     *
+ * Clear down the cache of any non bound, expired  or reserved entries.
- * Leave static and unexpired bound entries alone. For use where the cache  *
+ * Leave static and unexpired bound entries alone. For use where the cache
- * is held in persistant memory which can survive a reset or power cycle.   *
+ * is held in persistant memory which can survive a reset or power cycle.
- * This reduces the network traffic on restarts as the cache will have much *
+ * This reduces the network traffic on restarts as the cache will have much
- * of its entries intact.                                                   *
+ * of its entries intact.
- ****************************************************************************/
+ */
-
 void address_init_partial(void)
 {
     struct Address_Cache_Entry *pMatch;
@@ -441,13 +450,16 @@ void address_init_partial(void)
     return;
 }
 
-/****************************************************************************
+/**
- * Set the TTL info for the given device entry. If it is a bound entry we   *
+ * Set the TTL info for the given device entry. If it is a bound entry we
- * set it to static or normal and can change the TTL. If it is unbound we   *
+ * set it to static or normal and can change the TTL. If it is unbound we
- * can only set the TTL. This is done as a seperate function at the moment  *
+ * can only set the TTL. This is done as a seperate function at the moment
- * to avoid breaking the current API.                                       *
+ * to avoid breaking the current API.
- ****************************************************************************/
+ *
-
+ * @param device_id  Device-Id
+ * @param TimeOut  Timeout in seconds.
+ * @param StaticFlag  Flag indicating if being a static address.
+ */
 void address_set_device_TTL(
     uint32_t device_id, uint32_t TimeOut, bool StaticFlag)
 {
@@ -476,6 +488,13 @@ void address_set_device_TTL(
     }
 }
 
+/**
+ * Return the cached address for the given device-id
+ *
+ * @param device_id  Device-Id
+ * @param max_apdu  Pointer to a variable, taking the maximum APDU size.
+ * @param src  Pointer to address structure for return.
+ */
 bool address_get_by_device(
     uint32_t device_id, unsigned *max_apdu, BACNET_ADDRESS *src)
 {
@@ -500,8 +519,14 @@ bool address_get_by_device(
     return found;
 }
 
-/* find a device id from a given MAC address */
+/**
-
+ * Find a device id from a given MAC address.
+ *
+ * @param src  Pointer to address structure to search for.
+ * @param device_id  Pointer to the device id variable for return.
+ *
+ * @return true/false
+ */
 bool address_get_device_id(BACNET_ADDRESS *src, uint32_t *device_id)
 {
     struct Address_Cache_Entry *pMatch;
@@ -525,6 +550,13 @@ bool address_get_device_id(BACNET_ADDRESS *src, uint32_t *device_id)
     return found;
 }
 
+/**
+ * Add a device using the given id, max_apdu and address.
+ *
+ * @param device_id  Pointer to the device id variable for return.
+ * @param max_apdu  Maximum APDU size.
+ * @param src  Pointer to address structure to add.
+ */
 void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
 {
     bool found = false; /* return value */
@@ -543,6 +575,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
     /* existing device or bind request outstanding - update address */
     pMatch = Address_Cache;
     while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
+        /* Device already in the list, then update the values. */
         if (((pMatch->Flags & BAC_ADDR_IN_USE) != 0) &&
             (pMatch->device_id == device_id)) {
             bacnet_address_copy(&pMatch->address, src);
@@ -572,7 +605,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
         pMatch++;
     }
 
-    /* new device - add to cache if there is room */
+    /* New device - add to cache if there is room. */
     if (!found) {
         pMatch = Address_Cache;
         while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
@@ -591,7 +624,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
         }
     }
 
-    /* See if we can squeeze it in */
+    /* If adding has failed, see if we can squeeze it in by removed the oldest entry. */
     if (!found) {
         pMatch = address_remove_oldest();
         if (pMatch != NULL) {
@@ -606,8 +639,19 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
     return;
 }
 
-/* returns true if device is already bound */
+/**
-/* also returns the address and max apdu if already bound */
+ * Check if the device is in the list. If yes, return the values.
+ * Otherwise add the device to the list.
+ * Returns true if device is already bound.
+ * Also returns the address and max apdu if already bound.
+ *
+ * @param device_id  Pointer to the device id variable for return.
+ * @param device_ttl  Pointer to a variable taking the time to live in seconds.
+ * @param max_apdu  Max APDU size of the device.
+ * @param src  Pointer to the BACnet address.
+ *
+ * @return true if device is already bound
+ */
 bool address_device_bind_request(uint32_t device_id,
     uint32_t *device_ttl,
     unsigned *max_apdu,
@@ -675,14 +719,31 @@ bool address_device_bind_request(uint32_t device_id,
     return (false);
 }
 
-/* returns true if device is already bound */
+/**
-/* also returns the address and max apdu if already bound */
+ * Check if the device is in the list. If yes, return the values.
+ * Otherwise add the device to the list.
+ * Returns true if device is already bound.
+ * Also returns the address and max apdu if already bound.
+ *
+ * @param device_id  Pointer to the device id variable for return.
+ * @param max_apdu  Max APDU size of the device.
+ * @param src  Pointer to the BACnet address.
+ *
+ * @return true if device is already bound
+ */
 bool address_bind_request(
     uint32_t device_id, unsigned *max_apdu, BACNET_ADDRESS *src)
 {
     return address_device_bind_request(device_id, NULL, max_apdu, src);
 }
 
+/**
+ * For an existing device add a binding.
+ *
+ * @param device_id  Pointer to the device id variable for return.
+ * @param max_apdu  Max APDU size of the device.
+ * @param src  Pointer to the BACnet address.
+ */
 void address_add_binding(
     uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src)
 {
@@ -709,6 +770,17 @@ void address_add_binding(
     return;
 }
 
+/**
+ * Return the device information from the given index in the table.
+ *
+ * @param index  Table index [0..MAX_ADDRESS_CACHE-1]
+ * @param device_id  Pointer to the variable taking the device id.
+ * @param device_ttl  Pointer to the variable taking the Time To Life for the device.
+ * @param max_apdu  Pointer to the variable taking the max APDU size of the device.
+ * @param src  Pointer to the BACnet address.
+ *
+ * @return true/false
+ */
 bool address_device_get_by_index(unsigned index,
     uint32_t *device_id,
     uint32_t *device_ttl,
@@ -741,6 +813,16 @@ bool address_device_get_by_index(unsigned index,
     return found;
 }
 
+/**
+ * Return the device information from the given index in the table.
+ *
+ * @param index  Table index [0..MAX_ADDRESS_CACHE-1]
+ * @param device_id  Pointer to the variable taking the device id.
+ * @param max_apdu  Pointer to the variable taking the max APDU size of the device.
+ * @param src  Pointer to the BACnet address.
+ *
+ * @return true/false
+ */
 bool address_get_by_index(unsigned index,
     uint32_t *device_id,
     unsigned *max_apdu,
@@ -749,6 +831,11 @@ bool address_get_by_index(unsigned index,
     return address_device_get_by_index(index, device_id, NULL, max_apdu, src);
 }
 
+/**
+ * Return the count of cached addresses.
+ *
+ * @return A value between zero and MAX_ADDRESS_CACHE.
+ */
 unsigned address_count(void)
 {
     struct Address_Cache_Entry *pMatch;
@@ -768,43 +855,55 @@ unsigned address_count(void)
     return count;
 }
 
-/****************************************************************************
+/**
- * Build a list of the current bindings for the device address binding      *
+ * Build a list of the current bindings for the device address binding
- * property.                                                                *
+ * property. Basically encode the address list to be send out.
- ****************************************************************************/
+ *
-
+ * @param apdu  Pointer to the APDU
+ * @param apdu_len  Remaining buffer size.
+ *
+ * @return Count of encoded bytes.
+ */
 int address_list_encode(uint8_t *apdu, unsigned apdu_len)
 {
     int iLen = 0;
     struct Address_Cache_Entry *pMatch;
     BACNET_OCTET_STRING MAC_Address;
 
-    /* FIXME: I really shouild check the length remaining here but it is
+    /* Look for matching address. */
-       fairly pointless until we have the true length remaining in
-       the packet to work with as at the moment it is just MAX_APDU */
-    (void)apdu_len;
-    /* look for matching address */
     pMatch = Address_Cache;
     while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
         if ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) ==
             BAC_ADDR_IN_USE) {
             iLen += encode_application_object_id(
-                &apdu[iLen], OBJECT_DEVICE, pMatch->device_id);
+                    &apdu[iLen], OBJECT_DEVICE, pMatch->device_id);
-            iLen +=
+            iLen += encode_application_unsigned(&apdu[iLen], pMatch->address.net);
-                encode_application_unsigned(&apdu[iLen], pMatch->address.net);
+            if ((unsigned)iLen >= apdu_len) {
+                break;
+            }
 
             /* pick the appropriate type of entry from the cache */
 
             if (pMatch->address.len != 0) {
+                /* BAC */
+                if ((unsigned)(iLen + pMatch->address.len) >= apdu_len) {
+                    break;
+                }
                 octetstring_init(
                     &MAC_Address, pMatch->address.adr, pMatch->address.len);
-                iLen +=
+                iLen += encode_application_octet_string(&apdu[iLen], &MAC_Address);
-                    encode_application_octet_string(&apdu[iLen], &MAC_Address);
             } else {
+                /* MAC*/
+                if ((unsigned)(iLen + pMatch->address.mac_len) >= apdu_len) {
+                    break;
+                }
                 octetstring_init(
                     &MAC_Address, pMatch->address.mac, pMatch->address.mac_len);
-                iLen +=
+                iLen += encode_application_octet_string(&apdu[iLen], &MAC_Address);
-                    encode_application_octet_string(&apdu[iLen], &MAC_Address);
+            }
+            /* Any space left? */
+            if ((unsigned)iLen >= apdu_len) {
+                break;
             }
         }
         pMatch++;
@@ -813,29 +912,33 @@ int address_list_encode(uint8_t *apdu, unsigned apdu_len)
     return (iLen);
 }
 
-/****************************************************************************
+/**
- * Build a list of the current bindings for the device address binding      *
+ * Build a list of the current bindings for the device address binding
- * property as required for the ReadsRange functionality.                   *
+ * property as required for the ReadsRange functionality.
- * We assume we only get called for "Read All" or "By Position" requests.   *
+ * We assume we only get called for "Read All" or "By Position" requests.
- *                                                                          *
+ *
- * We need to treat the address cache as a contiguous array but in reality  *
+ * We need to treat the address cache as a contiguous array but in reality
- * it could be sparsely populated. We can get the count but we can only     *
+ * it could be sparsely populated. We can get the count but we can only
- * extract entries by doing a linear scan starting from the first entry in  *
+ * extract entries by doing a linear scan starting from the first entry in
- * the cache and picking them off one by one.                               *
+ * the cache and picking them off one by one.
- *                                                                          *
+ *
- * We do assume the list cannot change whilst we are accessing it so would  *
+ * We do assume the list cannot change whilst we are accessing it so would
- * not be multithread safe if there are other tasks that change the cache.  *
+ * not be multithread safe if there are other tasks that change the cache.
- *                                                                          *
+ *
  * We take the simple approach here to filling the buffer by taking a max   *
- * size for a single entry and then stopping if there is less than that     *
+ * size for a single entry and then stopping if there is less than that
- * left in the buffer. You could build each entry in a seperate buffer and  *
+ * left in the buffer. You could build each entry in a seperate buffer and
- * determine the exact length before copying but this is time consuming,    *
+ * determine the exact length before copying but this is time consuming,
- * requires more memory and would probably only let you sqeeeze one more    *
+ * requires more memory and would probably only let you sqeeeze one more
- * entry in on occasion. The value is calculated as 5 bytes for the device  *
+ * entry in on occasion. The value is calculated as 5 bytes for the device
- * ID + 3 bytes for the network number and nine bytes for the MAC address   *
+ * ID + 3 bytes for the network number and nine bytes for the MAC address
- * oct string to give 17 bytes (the minimum possible is 5 + 2 + 3 = 10).    *
+ * oct string to give 17 bytes (the minimum possible is 5 + 2 + 3 = 10).
- ****************************************************************************/
+ *
-
+ * @param apdu  Pointer to the encode buffer.
+ * @param pRequest  Pointer to the read_range request structure.
+ *
+ * @return Bytes encoded.
+ */
 #define ACACHE_MAX_ENC 17 /* Maximum size of encoded cache entry, see above */
 
 int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
@@ -851,6 +954,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
     uint32_t uiTarget = 0; /* Last entry we are required to encode */
     uint32_t uiRemaining = 0; /* Amount of unused space in packet */
 
+    if ((!pRequest) || (!apdu)) {
+        return 0;
+    }
+
     /* Initialise result flags to all false */
     bitstring_init(&pRequest->ResultFlags);
     bitstring_set_bit(&pRequest->ResultFlags, RESULT_FLAG_FIRST_ITEM, false);
@@ -861,7 +968,7 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
 
     pRequest->ItemCount = 0; /* Start out with nothing */
     uiTotal = address_count(); /* What do we have to work with here ? */
-    if (uiTotal == 0) { /* Bail out now if nowt */
+    if (uiTotal == 0) { /* Bail out now if not. */
         return (0);
     }
 
@@ -921,6 +1028,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
     while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) !=
         BAC_ADDR_IN_USE) { /* Find first bound entry */
         pMatch++;
+        /* Shall not happen as the count has been checked first. */
+        if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
+            return(0); /* Issue with the table. */
+        }
     }
 
     /* Seek to start position */
@@ -932,6 +1043,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
         } else {
             pMatch++;
         }
+        /* Shall not happen as the count has been checked first. */
+        if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
+            return(0); /* Issue with the table. */
+        }
     }
 
     uiFirst = uiIndex; /* Record where we started from */
@@ -976,6 +1091,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
         while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) !=
             BAC_ADDR_IN_USE) { /* Find next bound entry */
             pMatch++;
+            /* Can normally not happen. */
+            if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
+                return(0); /* Issue with the table. */
+            }
         }
     }
 
@@ -991,15 +1110,16 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
     return (iLen);
 }
 
-/****************************************************************************
+/**
- * Scan the cache and eliminate any expired entries. Should be called       *
+ * Scan the cache and eliminate any expired entries. Should be called
- * periodically to ensure the cache is managed correctly. If this function  *
+ * periodically to ensure the cache is managed correctly. If this function
- * is never called at all the whole cache is effectivly rendered static and *
+ * is never called at all the whole cache is effectivly rendered static and
- * entries never expire unless explictely deleted.                          *
+ * entries never expire unless explictely deleted.
- ****************************************************************************/
+ *
-
+ * @param uSeconds  Approximate number of seconds since last call to this function
+ */
 void address_cache_timer(uint16_t uSeconds)
-{ /* Approximate number of seconds since last call to this function */
+{
     struct Address_Cache_Entry *pMatch;
 
     pMatch = Address_Cache;

src/bacnet/basic/object/bv.c

@@ -422,7 +422,7 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
     if (wp_data == NULL) {
         return false;
     }
-    if ((wp_data->application_data == NULL) || \
+    if ((wp_data->application_data == NULL) ||
         (wp_data->application_data_len == 0)) {
         return false;
     }

src/bacnet/basic/object/csv.c

@@ -384,7 +384,7 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     if (rpdata == NULL) {
         return 0;
     }
-    if ((rpdata->application_data == NULL) || \
+    if ((rpdata->application_data == NULL) ||
         (rpdata->application_data_len == 0)) {
         return 0;
     }
@@ -488,7 +488,7 @@ bool CharacterString_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
     if (wp_data == NULL) {
         return false;
     }
-    if ((wp_data->application_data == NULL) || \
+    if ((wp_data->application_data == NULL) ||
         (wp_data->application_data_len == 0)) {
         return false;
     }

src/bacnet/basic/service/h_rpm.c

@@ -182,6 +182,7 @@ void handler_read_property_multiple(uint8_t *service_request,
     BACNET_ADDRESS *src,
     BACNET_CONFIRMED_SERVICE_DATA *service_data)
 {
+    bool berror = false;
     int len = 0;
     uint16_t copy_len = 0;
     uint16_t decode_len = 0;
@@ -194,234 +195,258 @@ void handler_read_property_multiple(uint8_t *service_request,
     int npdu_len = 0;
     int error = 0;
 
-    /* jps_debug - see if we are utilizing all the buffer */
+    if (service_data && (service_len > 0)) {
-    /* memset(&Handler_Transmit_Buffer[0], 0xff,
+        /* jps_debug - see if we are utilizing all the buffer */
-     * sizeof(Handler_Transmit_Buffer)); */
+        /* memset(&Handler_Transmit_Buffer[0], 0xff,
-    /* encode the NPDU portion of the packet */
+         * sizeof(Handler_Transmit_Buffer)); */
-    datalink_get_my_address(&my_address);
+        /* encode the NPDU portion of the packet */
-    npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL);
+        datalink_get_my_address(&my_address);
-    npdu_len = npdu_encode_pdu(
+        npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL);
-        &Handler_Transmit_Buffer[0], src, &my_address, &npdu_data);
+        npdu_len = npdu_encode_pdu(
-    if (service_data->segmented_message) {
+            &Handler_Transmit_Buffer[0], src, &my_address, &npdu_data);
-        rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-        error = BACNET_STATUS_ABORT;
-#if PRINT_ENABLED
-        fprintf(stderr, "RPM: Segmented message. Sending Abort!\r\n");
-#endif
-        goto RPM_FAILURE;
-    }
-    /* decode apdu request & encode apdu reply
-       encode complex ack, invoke id, service choice */
-    apdu_len = rpm_ack_encode_apdu_init(
-        &Handler_Transmit_Buffer[npdu_len], service_data->invoke_id);
-    for (;;) {
-        /* Start by looking for an object ID */
-        len = rpm_decode_object_id(
-            &service_request[decode_len], service_len - decode_len, &rpmdata);
-        if (len >= 0) {
-            /* Got one so skip to next stage */
-            decode_len += len;
-        } else {
-            /* bad encoding - skip to error/reject/abort handling */
-#if PRINT_ENABLED
-            fprintf(stderr, "RPM: Bad Encoding.\n");
-#endif
-            error = len;
-            goto RPM_FAILURE;
-        }
 
-        /* Test for case of indefinite Device object instance */
+        if (service_data->segmented_message) {
-        if ((rpmdata.object_type == OBJECT_DEVICE) &&
-            (rpmdata.object_instance == BACNET_MAX_INSTANCE)) {
-            rpmdata.object_instance = Device_Object_Instance_Number();
-        }
-
-        /* Stick this object id into the reply - if it will fit */
-        len = rpm_ack_encode_apdu_object_begin(&Temp_Buf[0], &rpmdata);
-        copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], &Temp_Buf[0],
-            apdu_len, len, MAX_APDU);
-        if (copy_len == 0) {
-#if PRINT_ENABLED
-            fprintf(stderr, "RPM: Response too big!\r\n");
-#endif
             rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
             error = BACNET_STATUS_ABORT;
-            goto RPM_FAILURE;
+    #if PRINT_ENABLED
-        }
+            fprintf(stderr, "RPM: Segmented message. Sending Abort!\r\n");
+    #endif
+        } else {
+            /* decode apdu request & encode apdu reply
+               encode complex ack, invoke id, service choice */
+            apdu_len = rpm_ack_encode_apdu_init(
+                &Handler_Transmit_Buffer[npdu_len], service_data->invoke_id);
 
-        apdu_len += copy_len;
+            for (;;) {
-        /* do each property of this object of the RPM request */
+                /* Start by looking for an object ID */
-        for (;;) {
+                len = rpm_decode_object_id(
-            /* Fetch a property */
+                    &service_request[decode_len], service_len - decode_len, &rpmdata);
-            len = rpm_decode_object_property(&service_request[decode_len],
+                if (len >= 0) {
-                service_len - decode_len, &rpmdata);
+                    /* Got one so skip to next stage */
-            if (len < 0) {
+                    decode_len += len;
-                /* bad encoding - skip to error/reject/abort handling */
-#if PRINT_ENABLED
-                fprintf(stderr, "RPM: Bad Encoding.\n");
-#endif
-                error = len;
-                goto RPM_FAILURE;
-            }
-            decode_len += len;
-            /* handle the special properties */
-            if ((rpmdata.object_property == PROP_ALL) ||
-                (rpmdata.object_property == PROP_REQUIRED) ||
-                (rpmdata.object_property == PROP_OPTIONAL)) {
-                struct special_property_list_t property_list;
-                unsigned property_count = 0;
-                unsigned index = 0;
-                BACNET_PROPERTY_ID special_object_property;
-
-                if (rpmdata.array_index != BACNET_ARRAY_ALL) {
-                    /*  No array index options for this special property.
-                       Encode error for this object property response */
-                    len = rpm_ack_encode_apdu_object_property(&Temp_Buf[0],
-                        rpmdata.object_property, rpmdata.array_index);
-                    copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
-                        &Temp_Buf[0], apdu_len, len, MAX_APDU);
-                    if (copy_len == 0) {
-#if PRINT_ENABLED
-                        fprintf(
-                            stderr, "RPM: Too full to encode property!\r\n");
-#endif
-                        rpmdata.error_code =
-                            ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-                        error = BACNET_STATUS_ABORT;
-                        goto RPM_FAILURE;
-                    }
-                    apdu_len += len;
-                    len = rpm_ack_encode_apdu_object_property_error(
-                        &Temp_Buf[0], ERROR_CLASS_PROPERTY,
-                        ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY);
-                    copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
-                        &Temp_Buf[0], apdu_len, len, MAX_APDU);
-                    if (copy_len == 0) {
-#if PRINT_ENABLED
-                        fprintf(stderr, "RPM: Too full to encode error!\r\n");
-#endif
-                        rpmdata.error_code =
-                            ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-                        error = BACNET_STATUS_ABORT;
-                        goto RPM_FAILURE;
-                    }
-                    apdu_len += len;
                 } else {
-                    special_object_property = rpmdata.object_property;
+                    /* bad encoding - skip to error/reject/abort handling */
-                    Device_Objects_Property_List(rpmdata.object_type,
+        #if PRINT_ENABLED
-                        rpmdata.object_instance, &property_list);
+                    fprintf(stderr, "RPM: Bad Encoding.\n");
-                    property_count = RPM_Object_Property_Count(
+        #endif
-                        &property_list, special_object_property);
+                    error = len;
-                    if (property_count == 0) {
+                    berror = true;
-                        /* this only happens with the OPTIONAL property */
+                    break;
-                        /* 135-2016bl-2. Clarify ReadPropertyMultiple
+                }
-                           response on OPTIONAL when empty.  */
+
-                        /* If no optional properties are supported then
+                /* Test for case of indefinite Device object instance */
-                           an empty 'List of Results' shall be returned
+                if ((rpmdata.object_type == OBJECT_DEVICE) &&
-                           for the specified property.*/
+                    (rpmdata.object_instance == BACNET_MAX_INSTANCE)) {
-                    } else {
+                    rpmdata.object_instance = Device_Object_Instance_Number();
-                        for (index = 0; index < property_count; index++) {
+                }
-                            rpmdata.object_property = RPM_Object_Property(
+
-                                &property_list, special_object_property, index);
+                /* Stick this object id into the reply - if it will fit */
-                            len = RPM_Encode_Property(
+                len = rpm_ack_encode_apdu_object_begin(&Temp_Buf[0], &rpmdata);
-                                &Handler_Transmit_Buffer[npdu_len],
+                copy_len =
-                                (uint16_t)apdu_len, MAX_APDU, &rpmdata);
+                    memcopy(&Handler_Transmit_Buffer[npdu_len], &Temp_Buf[0],
-                            if (len > 0) {
+                    apdu_len, len, MAX_APDU);
-                                apdu_len += len;
+                if (copy_len == 0) {
-                            } else {
+        #if PRINT_ENABLED
-#if PRINT_ENABLED
+                    fprintf(stderr, "RPM: Response too big!\r\n");
+        #endif
+                    rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
+                    error = BACNET_STATUS_ABORT;
+                    berror = true;
+                    break;
+                }
+
+                apdu_len += copy_len;
+                /* do each property of this object of the RPM request */
+                for (;;) {
+                    /* Fetch a property */
+                    len = rpm_decode_object_property(&service_request[decode_len],
+                          service_len - decode_len, &rpmdata);
+                    if (len < 0) {
+                        /* bad encoding - skip to error/reject/abort handling */
+        #if PRINT_ENABLED
+                        fprintf(stderr, "RPM: Bad Encoding.\n");
+        #endif
+                        error = len;
+                        berror = true;
+                        break; // The berror flag ensures that both loops will be broken!
+                    }
+                    decode_len += len;
+                    /* handle the special properties */
+                    if ((rpmdata.object_property == PROP_ALL) ||
+                        (rpmdata.object_property == PROP_REQUIRED) ||
+                        (rpmdata.object_property == PROP_OPTIONAL)) {
+                        struct special_property_list_t property_list;
+                        unsigned property_count = 0;
+                        unsigned index = 0;
+                        BACNET_PROPERTY_ID special_object_property;
+
+                        if (rpmdata.array_index != BACNET_ARRAY_ALL) {
+
+                            /* No array index options for this special property.
+                               Encode error for this object property response */
+                            len = rpm_ack_encode_apdu_object_property(&Temp_Buf[0],
+                                  rpmdata.object_property, rpmdata.array_index);
+
+                            copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
+                                               &Temp_Buf[0], apdu_len, len, MAX_APDU);
+
+                            if (copy_len == 0) {
+        #if PRINT_ENABLED
                                 fprintf(
-                                    stderr, "RPM: Too full for property!\r\n");
+                                    stderr, "RPM: Too full to encode property!\r\n");
-#endif
+        #endif
-                                error = len;
+                                rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-                                goto RPM_FAILURE;
+                                error = BACNET_STATUS_ABORT;
+                                berror = true;
+                                break; // The berror flag ensures that both loops will be broken!
+                            }
+
+                            apdu_len += len;
+                            len = rpm_ack_encode_apdu_object_property_error(
+                                  &Temp_Buf[0], ERROR_CLASS_PROPERTY,
+                                  ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY);
+
+                            copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
+                                               &Temp_Buf[0], apdu_len, len, MAX_APDU);
+
+                            if (copy_len == 0) {
+        #if PRINT_ENABLED
+                                fprintf(stderr, "RPM: Too full to encode error!\r\n");
+        #endif
+                                rpmdata.error_code =
+                                    ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
+                                error = BACNET_STATUS_ABORT;
+                                berror = true;
+                                break; // The berror flag ensures that both loops will be broken!
+                            }
+                            apdu_len += len;
+                        } else {
+                            special_object_property = rpmdata.object_property;
+                            Device_Objects_Property_List(rpmdata.object_type,
+                                rpmdata.object_instance, &property_list);
+                            property_count = RPM_Object_Property_Count(
+                                &property_list, special_object_property);
+
+                            if (property_count == 0) {
+                                /* This only happens with the OPTIONAL property */
+                                /* 135-2016bl-2. Clarify ReadPropertyMultiple
+                                   response on OPTIONAL when empty. */
+                                /* If no optional properties are supported then
+                                   an empty 'List of Results' shall be returned
+                                   for the specified property.*/
+                            } else {
+                                for (index = 0; index < property_count; index++) {
+                                    rpmdata.object_property = RPM_Object_Property(
+                                        &property_list, special_object_property, index);
+                                    len = RPM_Encode_Property(
+                                          &Handler_Transmit_Buffer[npdu_len],
+                                          (uint16_t)apdu_len, MAX_APDU, &rpmdata);
+                                    if (len > 0) {
+                                        apdu_len += len;
+                                    } else {
+        #if PRINT_ENABLED
+                                        fprintf(
+                                            stderr, "RPM: Too full for property!\r\n");
+        #endif
+                                        error = len;
+                                        berror = true;
+                                        break; // The berror flag ensures that both loops will be broken!
+                                    }
+                                }
                             }
                         }
+                    } else {
+                        /* handle an individual property */
+                        len = RPM_Encode_Property(&Handler_Transmit_Buffer[npdu_len],
+                              (uint16_t)apdu_len, MAX_APDU, &rpmdata);
+                        if (len > 0) {
+                            apdu_len += len;
+                        } else {
+        #if PRINT_ENABLED
+                            fprintf(
+                                stderr, "RPM: Too full for individual property!\r\n");
+        #endif
+                            error = len;
+                            berror = true;
+                            break; // The berror flag ensures that both loops will be broken!
+                        }
                     }
+
+                    if (decode_is_closing_tag_number(&service_request[decode_len], 1)) {
+                        /* Reached end of property list so cap the result list */
+                        decode_len++;
+                        len = rpm_ack_encode_apdu_object_end(&Temp_Buf[0]);
+                        copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
+                                   &Temp_Buf[0], apdu_len, len, MAX_APDU);
+                        if (copy_len == 0) {
+        #if PRINT_ENABLED
+                            fprintf(stderr, "RPM: Too full to encode object end!\r\n");
+        #endif
+                            rpmdata.error_code =
+                                ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
+                            error = BACNET_STATUS_ABORT;
+                            berror = true;
+                            break; // The berror flag ensures that both loops will be broken!
+                        } else {
+                            apdu_len += copy_len;
+                        }
+                        break; /* finished with this property list */
+                    }
+                } // for(;;)
+                if (berror) {
+                    break;
                 }
-            } else {
+                if (decode_len >= service_len) {
-                /* handle an individual property */
+                    /* Reached the end so finish up */
-                len = RPM_Encode_Property(&Handler_Transmit_Buffer[npdu_len],
+                    break;
-                    (uint16_t)apdu_len, MAX_APDU, &rpmdata);
-                if (len > 0) {
-                    apdu_len += len;
-                } else {
-#if PRINT_ENABLED
-                    fprintf(
-                        stderr, "RPM: Too full for individual property!\r\n");
-#endif
-                    error = len;
-                    goto RPM_FAILURE;
                 }
-            }
+            } // for(;;)
-            if (decode_is_closing_tag_number(&service_request[decode_len], 1)) {
+
-                /* Reached end of property list so cap the result list */
+            /* If not having an error so far, check the remaining space. */
-                decode_len++;
+            if (!berror) {
-                len = rpm_ack_encode_apdu_object_end(&Temp_Buf[0]);
+                if (apdu_len > service_data->max_resp) {
-                copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len],
+                    /* too big for the sender - send an abort */
-                    &Temp_Buf[0], apdu_len, len, MAX_APDU);
+                    rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-                if (copy_len == 0) {
-#if PRINT_ENABLED
-                    fprintf(stderr, "RPM: Too full to encode object end!\r\n");
-#endif
-                    rpmdata.error_code =
-                        ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
                     error = BACNET_STATUS_ABORT;
-                    goto RPM_FAILURE;
+            #if PRINT_ENABLED
-                } else {
+                    fprintf(stderr, "RPM: Message too large.  Sending Abort!\n");
-                    apdu_len += copy_len;
+            #endif
                 }
-                break; /* finished with this property list */
             }
         }
-        if (decode_len >= service_len) {
+
-            /* Reached the end so finish up */
+        /* Error fallback. */
-            break;
+        if (error) {
+            if (error == BACNET_STATUS_ABORT) {
+                apdu_len = abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                           service_data->invoke_id,
+                           abort_convert_error_code(rpmdata.error_code), true);
+    #if PRINT_ENABLED
+                fprintf(stderr, "RPM: Sending Abort!\n");
+    #endif
+            } else if (error == BACNET_STATUS_ERROR) {
+                apdu_len = bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                           service_data->invoke_id, SERVICE_CONFIRMED_READ_PROP_MULTIPLE,
+                           rpmdata.error_class, rpmdata.error_code);
+    #if PRINT_ENABLED
+                fprintf(stderr, "RPM: Sending Error!\n");
+    #endif
+            } else if (error == BACNET_STATUS_REJECT) {
+                apdu_len = reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                           service_data->invoke_id,
+                           reject_convert_error_code(rpmdata.error_code));
+    #if PRINT_ENABLED
+                fprintf(stderr, "RPM: Sending Reject!\n");
+    #endif
+            }
         }
-    }
 
-    if (apdu_len > service_data->max_resp) {
+        pdu_len = apdu_len + npdu_len;
-        /* too big for the sender - send an abort */
+        bytes_sent = datalink_send_pdu(src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len);
-        rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
+        if (bytes_sent <= 0) {
-        error = BACNET_STATUS_ABORT;
+    #if PRINT_ENABLED
-#if PRINT_ENABLED
+            fprintf(stderr, "RPM: Failed to send PDU (%s)!\n", strerror(errno));
-        fprintf(stderr, "RPM: Message too large.  Sending Abort!\n");
+    #endif
-#endif
-        goto RPM_FAILURE;
-    }
-
-RPM_FAILURE:
-    if (error) {
-        if (error == BACNET_STATUS_ABORT) {
-            apdu_len = abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
-                service_data->invoke_id,
-                abort_convert_error_code(rpmdata.error_code), true);
-#if PRINT_ENABLED
-            fprintf(stderr, "RPM: Sending Abort!\n");
-#endif
-        } else if (error == BACNET_STATUS_ERROR) {
-            apdu_len = bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
-                service_data->invoke_id, SERVICE_CONFIRMED_READ_PROP_MULTIPLE,
-                rpmdata.error_class, rpmdata.error_code);
-#if PRINT_ENABLED
-            fprintf(stderr, "RPM: Sending Error!\n");
-#endif
-        } else if (error == BACNET_STATUS_REJECT) {
-            apdu_len = reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
-                service_data->invoke_id,
-                reject_convert_error_code(rpmdata.error_code));
-#if PRINT_ENABLED
-            fprintf(stderr, "RPM: Sending Reject!\n");
-#endif
         }
     }
-
-    pdu_len = apdu_len + npdu_len;
-    bytes_sent = datalink_send_pdu(
-        src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len);
-    if (bytes_sent <= 0) {
-#if PRINT_ENABLED
-        fprintf(stderr, "RPM: Failed to send PDU (%s)!\n", strerror(errno));
-#endif
-    }
 }

src/bacnet/basic/service/h_wp.c

@@ -136,7 +136,7 @@ void handler_write_property(uint8_t *service_request,
 
     /* Send PDU */
     pdu_len += len;
-    bytes_sent = datalink_send_pdu( \
+    bytes_sent = datalink_send_pdu(
                  src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len);
     if (bytes_sent <= 0) {
 #if PRINT_ENABLED

src/bacnet/basic/sys/keylist.c

@@ -56,21 +56,33 @@
 /* Generic node routines */
 /******************************************************************** */
 
-/* grab memory for a node */
+/** Grab memory for a node (Keylist_Node).
+ *
+ * @return Pointer to the allocated memory or
+ *         NULL under an Out Of Memory situation.
+ */
 static struct Keylist_Node *NodeCreate(void)
 {
     return calloc(1, sizeof(struct Keylist_Node));
 }
 
-/* grab memory for a list */
+/** Grab memory for a list (Keylist).
+ *
+ * @return Pointer to the allocated memory or
+ *         NULL under an Out Of Memory situation.
+ */
 static struct Keylist *KeylistCreate(void)
 {
     return calloc(1, sizeof(struct Keylist));
 }
 
-/* check to see if the array is big enough for an addition */
+/** Check to see if the array is big enough for an addition
-/* or is too big when we are deleting and we can shrink */
+ * or is too big when we are deleting and we can shrink.
-/* returns TRUE if success, FALSE if failed */
+ *
+ * @param list  Pointer to the list to be tested.
+ *
+ * @return Returns TRUE if success, FALSE if failed
+ */
 static int CheckArraySize(OS_Keylist list)
 {
     int new_size = 0; /* set it up so that no size change is the default */
@@ -112,13 +124,20 @@ static int CheckArraySize(OS_Keylist list)
     return TRUE;
 }
 
-/* find the index of the key that we are looking for */
+/** Find the index of the key that we are looking for.
-/* since it is sorted, we can optimize the search */
+ * Since it is sorted, we can optimize the search.
-/* returns TRUE if found, and FALSE not found */
+ * returns TRUE if found, and FALSE not found.
-/* returns the found key and the index where it was found in parameters */
+ * Returns the found key and the index where it was found in parameters.
-/* If the key is not found, the nearest index from the bottom will be returned,
+ * If the key is not found, the nearest index from the bottom will be returned,
+ * allowing the ability to find where an key should go into the list.
+ *
+ * @param list  Pointer to the list
+ * @param key  Key to search for
+ * @param pIndex  Pointer to the variable taking the index were the key
+ *                had been found.
+ *
+ * @return TRUE if found, and FALSE if not
  */
-/* allowing the ability to find where an key should go into the list. */
 static int FindIndex(OS_Keylist list, KEY key, int *pIndex)
 {
     struct Keylist_Node *node; /* holds the new node */
@@ -127,7 +146,12 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex)
     int index = 0; /* our current search place in the array */
     KEY current_key = 0; /* place holder for current node key */
     int status = FALSE; /* return value */
-    if (!list || !list->array || !list->count) {
+
+    if (!list) {
+        *pIndex = 0;
+        return (FALSE);
+    }
+    if (!list->array || !list->count) {
         *pIndex = 0;
         return (FALSE);
     }
@@ -148,12 +172,12 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex)
             left = index + 1;
         }
     } while ((key != current_key) && (left <= right));
+
     if (key == current_key) {
         status = TRUE;
         *pIndex = index;
-    }
 
-    else {
+    } else {
         /* where the index should be */
         if (key > current_key) {
             *pIndex = index + 1;
@@ -168,7 +192,15 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex)
 /******************************************************************** */
 /* list data functions */
 /******************************************************************** */
-/* inserts a node into its sorted position */
+/** Inserts a node into its sorted position.
+ *
+ * @param list  Pointer to the list
+ * @param key  Key to be inserted
+ * @param data  Pointer to the data hold by the key.
+ *              This pointer needs to be poiting to static memory
+ *              as it will be stored in the list and later used
+ *              by retrieving the key again.
+ */
 int Keylist_Data_Add(OS_Keylist list, KEY key, void *data)
 {
     struct Keylist_Node *node; /* holds the new node */
@@ -191,9 +223,7 @@ int Keylist_Data_Add(OS_Keylist list, KEY key, void *data)
             for (i = list->count; i > index; i--) {
                 list->array[i] = list->array[i - 1];
             }
-        }
+        } else {
-
-        else {
             index = 0;
         }
 
@@ -209,138 +239,199 @@ int Keylist_Data_Add(OS_Keylist list, KEY key, void *data)
     return index;
 }
 
-/* deletes a node specified by its index */
+/** Deletes a node specified by its index
-/* returns the data from the node */
+ * returns the data from the node
+ *
+ * @param list  Pointer to the list
+ * @param index Index of the key to be deleted
+ *
+ * @returns Pointer to the data of the deleted key or NULL.
+ */
 void *Keylist_Data_Delete_By_Index(OS_Keylist list, int index)
 {
     struct Keylist_Node *node;
     void *data = NULL;
 
-    if (list && list->array && list->count && (index >= 0) &&
+    if (list) {
-        (index < list->count)) {
+        if (list->array && list->count && (index >= 0) &&
-        node = list->array[index];
+            (index < list->count)) {
-        if (node)
+            node = list->array[index];
-            data = node->data;
+            if (node) {
-
+                data = node->data;
-        /* move the nodes to account for the deleted one */
+            }
-        if (list->count == 1) {
+            /* move the nodes to account for the deleted one */
-            /* There is no node shifting to do */
+            if (list->count == 1) {
-        } else if (index == (list->count - 1)) {
+                /* There is no node shifting to do */
-            /* We are the last one */
+            } else if (index == (list->count - 1)) {
-            /* There is no node shifting to do */
+                /* We are the last one */
-        } else {
+                /* There is no node shifting to do */
-            /* Move all the nodes down one */
+            } else {
-            int i; /* counter */
+                /* Move all the nodes down one */
-            int count = list->count - 1;
+                int i; /* counter */
-            for (i = index; i < count; i++) {
+                int count = list->count - 1;
-                list->array[i] = list->array[i + 1];
+                for (i = index; i < count; i++) {
+                    list->array[i] = list->array[i + 1];
+                }
+            }
+            list->count--;
+            if (node) {
+                free(node);
             }
-        }
-        list->count--;
-        if (node) {
-            free(node);
-        }
 
-        /* potentially reduce the size of the array */
+            /* potentially reduce the size of the array */
-        (void)CheckArraySize(list);
+            (void)CheckArraySize(list);
+        }
     }
     return (data);
 }
 
-/* deletes a node specified by its key */
+/** Deletes a node specified by its key/
-/* returns the data from the node */
+ * returns the data from the node
+ *
+ * @param list  Pointer to the list
+ * @param key  Key to be deleted
+ *
+ * @returns Pointer to the data of the deleted key or NULL.
+ */
 void *Keylist_Data_Delete(OS_Keylist list, KEY key)
 {
     void *data = NULL; /* return value */
     int index; /* where the node is in the array */
 
     if (list) {
-        if (FindIndex(list, key, &index))
+        if (FindIndex(list, key, &index)) {
             data = Keylist_Data_Delete_By_Index(list, index);
+        }
     }
 
     return data;
 }
 
-/* returns the data from last node, and removes it from the list */
+/** Returns the data from last node, and
+ * removes it from the list.
+ *
+ * @param list  Pointer to the list
+ *
+ * @return Pointer to the data that might be NULL.
+ */
 void *Keylist_Data_Pop(OS_Keylist list)
 {
     void *data = NULL; /* return value */
     int index; /* position in the array */
 
-    if (list && list->count) {
+    if (list) {
-        index = list->count - 1;
+        if (list->count) {
-        data = Keylist_Data_Delete_By_Index(list, index);
+            index = list->count - 1;
+            data = Keylist_Data_Delete_By_Index(list, index);
+        }
     }
 
     return data;
 }
 
-/* returns the data from the node specified by key */
+/** Returns the data from the node specified by key.
+ *
+ * @param list  Pointer to the list
+ * @param key  Key to be deleted
+ *
+ * @returns Pointer to the data, that might be NULL.
+ */
 void *Keylist_Data(OS_Keylist list, KEY key)
 {
     struct Keylist_Node *node = NULL;
     int index = 0; /* used to look up the index of node */
 
-    if (list && list->array && list->count) {
+    if (list) {
-        if (FindIndex(list, key, &index))
+        if (list->array && list->count) {
-            node = list->array[index];
+            if (FindIndex(list, key, &index)) {
+                node = list->array[index];
+            }
+        }
     }
-
     return node ? node->data : NULL;
 }
 
-/* returns the index from the node specified by key */
+/** Returns the index from the node specified by key.
+ *
+ * @param list  Pointer to the list
+ * @param key  Key whose index shall be retrieved.
+ *
+ * @return Index of the key or -1, if not found.
+ */
 int Keylist_Index(OS_Keylist list, KEY key)
 {
     int index = -1; /* used to look up the index of node */
 
-    if (list && list->array && list->count) {
+    if (list) {
-        if (!FindIndex(list, key, &index)) {
+        if (list->array && list->count) {
-            index = -1;
+            if (!FindIndex(list, key, &index)) {
+                index = -1;
+            }
         }
     }
-
     return index;
 }
 
-/* returns the data specified by index */
+/** Returns the data specified by index
+ *
+ * @param list  Pointer to the list
+ * @param index  Key whose data shall be retrieved.
+ *
+ * @return Pointer to the data that might be NULL.
+ */
 void *Keylist_Data_Index(OS_Keylist list, int index)
 {
     struct Keylist_Node *node = NULL;
 
-    if (list && list->array && list->count && (index >= 0) &&
+    if (list) {
-        (index < list->count))
+        if (list->array && list->count && (index >= 0) &&
-        node = list->array[index];
+            (index < list->count)) {
-
+            node = list->array[index];
+        }
+    }
     return node ? node->data : NULL;
 }
 
-/* return the key at the given index */
+/** Return the key at the given index.
+ *
+ * @param list  Pointer to the list
+ * @param index  Index that shall be returned
+ *
+ * @return Key for the index or 0.
+ */
 KEY Keylist_Key(OS_Keylist list, int index)
 {
     KEY key = 0; /* return value */
     struct Keylist_Node *node;
 
-    if (list && list->array && list->count && (index >= 0) &&
+    if (list) {
-        (index < list->count)) {
+        if (list->array && list->count && (index >= 0) &&
-        node = list->array[index];
+            (index < list->count)) {
-        if (node)
+            node = list->array[index];
-            key = node->key;
+            if (node) {
+                key = node->key;
+            }
+        }
     }
-
     return key;
 }
 
-/* returns the next empty key from the list */
+/** Returns the next empty key from the list.
+ *
+ * @param list  Pointer to the list
+ * @param key  Key whose index shall be retrieved.
+ *
+ * @return Next empty key or 'key=key' if there is none.
+ */
 KEY Keylist_Next_Empty_Key(OS_Keylist list, KEY key)
 {
     int index;
 
     if (list) {
         while (FindIndex(list, key, &index)) {
-            if (KEY_LAST(key))
+            if (KEY_LAST(key)) {
                 break;
+            }
             key++;
         }
     }
@@ -348,29 +439,47 @@ KEY Keylist_Next_Empty_Key(OS_Keylist list, KEY key)
     return key;
 }
 
-/* return the number of nodes in this list */
+/** Return the number of nodes in this list.
+ *
+ * @param list  Pointer to the list
+ *
+ * @return Count of key in the list.
+ */
 int Keylist_Count(OS_Keylist list)
 {
-    return list->count;
+    int cnt = 0;
+
+    if (list) {
+        cnt = list->count;
+    }
+
+    return(cnt);
 }
 
 /******************************************************************** */
 /* Public List functions */
 /******************************************************************** */
 
-/* returns head of the list or NULL on failure. */
+/** Returns head of the list or NULL on failure.
+ *
+ * @return Pointer to the key list or NUL if creation failed.
+ */
 OS_Keylist Keylist_Create(void)
 {
     struct Keylist *list;
 
     list = KeylistCreate();
-    if (list)
+    if (list) {
         CheckArraySize(list);
+    }
 
     return list;
 }
 
-/* delete specified list */
+/** Delete specified list.
+ *
+ * @param list  Pointer to the list
+ */
 void Keylist_Delete(OS_Keylist list)
 { /* list number to be deleted */
     if (list) {
@@ -378,8 +487,9 @@ void Keylist_Delete(OS_Keylist list)
         while (list->count) {
             (void)Keylist_Data_Delete_By_Index(list, 0);
         }
-        if (list->array)
+        if (list->array) {
             free(list->array);
+        }
         free(list);
     }
 

src/bacnet/cov.c

@@ -48,6 +48,15 @@ COV Subscribe Property
 COV Notification
 Unconfirmed COV Notification
 */
+
+/**
+ * Encode APDU for notification.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param data  Pointer to the data to encode.
+ *
+ * @return bytes encoded or zero on error.
+ */
 static int notify_encode_apdu(
     uint8_t *apdu, unsigned max_apdu_len, BACNET_COV_DATA *data)
 {
@@ -122,6 +131,16 @@ static int notify_encode_apdu(
     return apdu_len;
 }
 
+/**
+ * Encode APDU for confirmed notification.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param max_apdu_len  Buffer size.
+ * @param invoke_id  ID to invoke for notification
+ * @param data  Pointer to the data to encode.
+ *
+ * @return bytes encoded or zero on error.
+ */
 int ccov_notify_encode_apdu(uint8_t *apdu,
     unsigned max_apdu_len,
     uint8_t invoke_id,
@@ -149,6 +168,15 @@ int ccov_notify_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
+/**
+ * Encode APDU for unconfirmed notification.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param max_apdu_len  Buffer size.
+ * @param data  Pointer to the data to encode.
+ *
+ * @return bytes encoded or zero on error.
+ */
 int ucov_notify_encode_apdu(
     uint8_t *apdu, unsigned max_apdu_len, BACNET_COV_DATA *data)
 {
@@ -172,8 +200,16 @@ int ucov_notify_encode_apdu(
     return apdu_len;
 }
 
-/* decode the service request only */
+/**
-/* COV and Unconfirmed COV are the same */
+ * Decode the COV-service request only.
+ * Note: COV and Unconfirmed COV are the same.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param apdu_len  Count of valid bytes in the buffer.
+ * @param data  Pointer to the data to store the decoded values.
+ *
+ * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error.
+ */
 int cov_notify_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_COV_DATA *data)
 {
@@ -187,7 +223,7 @@ int cov_notify_decode_service_request(
     BACNET_PROPERTY_VALUE *value = NULL; /* value in list */
     BACNET_APPLICATION_DATA_VALUE *app_data = NULL;
 
-    if (apdu_len && data) {
+    if ((apdu_len > 2) && data) {
         /* tag 0 - subscriberProcessIdentifier */
         if (decode_is_context_tag(&apdu[len], 0)) {
             len += decode_tag_number_and_value(
@@ -198,6 +234,9 @@ int cov_notify_decode_service_request(
             return BACNET_STATUS_ERROR;
         }
         /* tag 1 - initiatingDeviceIdentifier */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_ERROR;
+        }
         if (decode_is_context_tag(&apdu[len], 1)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -210,6 +249,9 @@ int cov_notify_decode_service_request(
             return BACNET_STATUS_ERROR;
         }
         /* tag 2 - monitoredObjectIdentifier */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_ERROR;
+        }
         if (decode_is_context_tag(&apdu[len], 2)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -220,6 +262,9 @@ int cov_notify_decode_service_request(
             return BACNET_STATUS_ERROR;
         }
         /* tag 3 - timeRemaining */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_ERROR;
+        }
         if (decode_is_context_tag(&apdu[len], 3)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -242,6 +287,9 @@ int cov_notify_decode_service_request(
         }
         while (value != NULL) {
             /* tag 0 - propertyIdentifier */
+            if (len >= (int)apdu_len) {
+                return BACNET_STATUS_ERROR;
+            }
             if (decode_is_context_tag(&apdu[len], 0)) {
                 len += decode_tag_number_and_value(
                     &apdu[len], &tag_number, &len_value);
@@ -251,6 +299,9 @@ int cov_notify_decode_service_request(
                 return BACNET_STATUS_ERROR;
             }
             /* tag 1 - propertyArrayIndex OPTIONAL */
+            if (len >= (int)apdu_len) {
+                return BACNET_STATUS_ERROR;
+            }
             if (decode_is_context_tag(&apdu[len], 1)) {
                 len += decode_tag_number_and_value(
                     &apdu[len], &tag_number, &len_value);
@@ -260,6 +311,9 @@ int cov_notify_decode_service_request(
                 value->propertyArrayIndex = BACNET_ARRAY_ALL;
             }
             /* tag 2: opening context tag - value */
+            if (len >= (int)apdu_len) {
+                return BACNET_STATUS_ERROR;
+            }
             if (!decode_is_opening_tag_number(&apdu[len], 2)) {
                 return BACNET_STATUS_ERROR;
             }
@@ -283,6 +337,9 @@ int cov_notify_decode_service_request(
             /* a tag number of 2 is not extended so only one octet */
             len++;
             /* tag 3 - priority OPTIONAL */
+            if (len >= (int)apdu_len) {
+                return BACNET_STATUS_ERROR;
+            }
             if (decode_is_context_tag(&apdu[len], 3)) {
                 len += decode_tag_number_and_value(
                     &apdu[len], &tag_number, &len_value);
@@ -330,6 +387,16 @@ SubscribeCOV-Request ::= SEQUENCE {
         }
 */
 
+/**
+ * Encode the COV-service request.
+ * Note: COV and Unconfirmed COV are the same.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param invoke_id  Invoke ID
+ * @param data  Pointer to the data to store the decoded values.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int cov_subscribe_encode_apdu(uint8_t *apdu,
     unsigned max_apdu_len,
     uint8_t invoke_id,
@@ -372,7 +439,15 @@ int cov_subscribe_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
-/* decode the service request only */
+/**
+ * Decode the subscribe-service request only.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param apdu_len  Count of valid bytes in the buffer.
+ * @param data  Pointer to the data to store the decoded values.
+ *
+ * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error.
+ */
 int cov_subscribe_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_SUBSCRIBE_COV_DATA *data)
 {
@@ -382,7 +457,7 @@ int cov_subscribe_decode_service_request(
     BACNET_UNSIGNED_INTEGER unsigned_value = 0;
     BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE;
 
-    if (apdu_len && data) {
+    if ((apdu_len > 2) && data) {
         /* tag 0 - subscriberProcessIdentifier */
         if (decode_is_context_tag(&apdu[len], 0)) {
             len += decode_tag_number_and_value(
@@ -394,6 +469,9 @@ int cov_subscribe_decode_service_request(
             return BACNET_STATUS_REJECT;
         }
         /* tag 1 - monitoredObjectIdentifier */
+        if ((unsigned) len >= apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 1)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -418,11 +496,15 @@ int cov_subscribe_decode_service_request(
                 data->cancellationRequest = true;
             }
             /* tag 3 - lifetime - optional */
-            if (decode_is_context_tag(&apdu[len], 3)) {
+            if ((unsigned) len < apdu_len) {
-                len += decode_tag_number_and_value(
+                if (decode_is_context_tag(&apdu[len], 3)) {
-                    &apdu[len], &tag_number, &len_value);
+                    len += decode_tag_number_and_value(
-                len += decode_unsigned(&apdu[len], len_value, &unsigned_value);
+                        &apdu[len], &tag_number, &len_value);
-                data->lifetime = unsigned_value;
+                    len += decode_unsigned(&apdu[len], len_value, &unsigned_value);
+                    data->lifetime = unsigned_value;
+                } else {
+                    data->lifetime = 0;
+                }
             } else {
                 data->lifetime = 0;
             }
@@ -453,6 +535,16 @@ BACnetPropertyReference ::= SEQUENCE {
 
 */
 
+/**
+ * Encode the properties for subscription into the APDU.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param max_apdu_len  Buffer size.
+ * @param invoke_id  Invoke Id.
+ * @param data  Pointer to the data to encode.
+ *
+ * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error.
+ */
 int cov_subscribe_property_encode_apdu(uint8_t *apdu,
     unsigned max_apdu_len,
     uint8_t invoke_id,
@@ -509,7 +601,15 @@ int cov_subscribe_property_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
-/* decode the service request only */
+/**
+ * Decode the COV-service property request only.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param apdu_len  Count of valid bytes in the buffer.
+ * @param data  Pointer to the data to store the decoded values.
+ *
+ * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error.
+ */
 int cov_subscribe_property_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_SUBSCRIBE_COV_DATA *data)
 {
@@ -520,7 +620,7 @@ int cov_subscribe_property_decode_service_request(
     BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; /* for decoding */
     uint32_t property = 0; /* for decoding */
 
-    if (apdu_len && data) {
+    if ((apdu_len > 2) && data) {
         /* tag 0 - subscriberProcessIdentifier */
         if (decode_is_context_tag(&apdu[len], 0)) {
             len += decode_tag_number_and_value(
@@ -532,6 +632,9 @@ int cov_subscribe_property_decode_service_request(
             return BACNET_STATUS_REJECT;
         }
         /* tag 1 - monitoredObjectIdentifier */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 1)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -543,6 +646,9 @@ int cov_subscribe_property_decode_service_request(
             return BACNET_STATUS_REJECT;
         }
         /* tag 2 - issueConfirmedNotifications - optional */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 2)) {
             data->cancellationRequest = false;
             len += decode_tag_number_and_value(
@@ -554,6 +660,9 @@ int cov_subscribe_property_decode_service_request(
             data->cancellationRequest = true;
         }
         /* tag 3 - lifetime - optional */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 3)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -563,6 +672,9 @@ int cov_subscribe_property_decode_service_request(
             data->lifetime = 0;
         }
         /* tag 4 - monitoredPropertyIdentifier */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (!decode_is_opening_tag_number(&apdu[len], 4)) {
             data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
             return BACNET_STATUS_REJECT;
@@ -570,6 +682,9 @@ int cov_subscribe_property_decode_service_request(
         /* a tag number of 4 is not extended so only one octet */
         len++;
         /* the propertyIdentifier is tag 0 */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 0)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -581,6 +696,9 @@ int cov_subscribe_property_decode_service_request(
             return BACNET_STATUS_REJECT;
         }
         /* the optional array index is tag 1 */
+        if (len >= (int)apdu_len) {
+            return BACNET_STATUS_REJECT;
+        }
         if (decode_is_context_tag(&apdu[len], 1)) {
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value);
@@ -597,11 +715,15 @@ int cov_subscribe_property_decode_service_request(
         /* a tag number of 4 is not extended so only one octet */
         len++;
         /* tag 5 - covIncrement - optional */
-        if (decode_is_context_tag(&apdu[len], 5)) {
+        if (len < (int)apdu_len) {
-            data->covIncrementPresent = true;
+            if (decode_is_context_tag(&apdu[len], 5)) {
-            len += decode_tag_number_and_value(
+                data->covIncrementPresent = true;
-                &apdu[len], &tag_number, &len_value);
+                len += decode_tag_number_and_value(
-            len += decode_real(&apdu[len], &data->covIncrement);
+                    &apdu[len], &tag_number, &len_value);
+                len += decode_real(&apdu[len], &data->covIncrement);
+            } else {
+                data->covIncrementPresent = false;
+            }
         } else {
             data->covIncrementPresent = false;
         }

src/bacnet/datetime.c

@@ -288,9 +288,16 @@ bool datetime_is_valid(BACNET_DATE *bdate, BACNET_TIME *btime)
     return datetime_date_is_valid(bdate) && datetime_time_is_valid(btime);
 }
 
-/* if the date1 is the same as date2, return is 0
+/**
-   if date1 is after date2, returns positive
+ * If the date1 is the same as date2, return is 0.
-   if date1 is before date2, returns negative */
+ * If date1 is after date2, returns positive.
+ * if date1 is before date2, returns negative.
+ *
+ * @param date1 - Pointer to a BACNET_DATE structure
+ * @param date2 - Pointer to a BACNET_DATE structure
+ *
+ * @return -/0/+
+ */
 int datetime_compare_date(BACNET_DATE *date1, BACNET_DATE *date2)
 {
     int diff = 0;
@@ -308,9 +315,16 @@ int datetime_compare_date(BACNET_DATE *date1, BACNET_DATE *date2)
     return diff;
 }
 
-/* if the time1 is the same as time2, return is 0
+/**
-   if time1 is after time2, returns positive
+ * If the time1 is the same as time2, return is 0.
-   if time1 is before time2, returns negative */
+ * If time1 is after time2, returns positive.
+ * if time1 is before time2, returns negative.
+ *
+ * @param time1 - Pointer to a BACNET_TIME structure
+ * @param time2 - Pointer to a BACNET_TIME structure
+ *
+ * @return -/0/+
+ */
 int datetime_compare_time(BACNET_TIME *time1, BACNET_TIME *time2)
 {
     int diff = 0;
@@ -331,9 +345,16 @@ int datetime_compare_time(BACNET_TIME *time1, BACNET_TIME *time2)
     return diff;
 }
 
-/* if the datetime1 is the same as datetime2, return is 0
+/**
-   if datetime1 is before datetime2, returns negative
+ * If the datetime1 is the same datetime2, return is 0.
-   if datetime1 is after datetime2, returns positive */
+ * If datetime1 is after datetime2, returns positive.
+ * if datetime1 is before datetime2, returns negative.
+ *
+ * @param datetime1 - Pointer to a BACNET_DATE_TIME structure
+ * @param datetime2 - Pointer to a BACNET_DATE_TIME structure
+ *
+ * @return -/0/+
+ */
 int datetime_compare(BACNET_DATE_TIME *datetime1, BACNET_DATE_TIME *datetime2)
 {
     int diff = 0;

src/bacnet/dcc.c

@@ -49,46 +49,73 @@ static BACNET_COMMUNICATION_ENABLE_DISABLE DCC_Enable_Disable =
     COMMUNICATION_ENABLE;
 /* password is optionally supported */
 
+/**
+ * Returns, the network communications enable/disable status.
+ *
+ * @return BACnet communication status
+ */
 BACNET_COMMUNICATION_ENABLE_DISABLE dcc_enable_status(void)
 {
     return DCC_Enable_Disable;
 }
 
+/**
+ * Returns, if network communications is enabled.
+ *
+ * @return true, if communication has been enabled.
+ */
 bool dcc_communication_enabled(void)
 {
     return (DCC_Enable_Disable == COMMUNICATION_ENABLE);
 }
 
-/* When network communications are completely disabled,
+/**
-   only DeviceCommunicationControl and ReinitializeDevice APDUs
+ * When network communications are completely disabled,
-   shall be processed and no messages shall be initiated.*/
+ * only DeviceCommunicationControl and ReinitializeDevice APDUs
+ * shall be processed and no messages shall be initiated.
+ *
+ * @return true, if communication has been disabled, false otherwise.
+ */
 bool dcc_communication_disabled(void)
 {
     return (DCC_Enable_Disable == COMMUNICATION_DISABLE);
 }
 
-/* When the initiation of communications is disabled,
+/**
-   all APDUs shall be processed and responses returned as
+ * When the initiation of communications is disabled,
-   required and no messages shall be initiated with the
+ * all APDUs shall be processed and responses returned as
-   exception of I-Am requests, which shall be initiated only in
+ * required and no messages shall be initiated with the
-   response to Who-Is messages. In this state, a device that
+ * exception of I-Am requests, which shall be initiated only in
-   supports I-Am request initiation shall send one I-Am request
+ * response to Who-Is messages. In this state, a device that
-   for any Who-Is request that is received if and only if
+ * supports I-Am request initiation shall send one I-Am request
-   the Who-Is request does not contain an address range or
+ * for any Who-Is request that is received if and only if
-   the device is included in the address range. */
+ * the Who-Is request does not contain an address range or
+ * the device is included in the address range.
+ *
+ * @return true, if disabling initiation is set, false otherwise.
+ */
 bool dcc_communication_initiation_disabled(void)
 {
     return (DCC_Enable_Disable == COMMUNICATION_DISABLE_INITIATION);
 }
 
-/* note: 0 indicates either expired, or infinite duration */
+/**
+ * Returns the time duration in seconds.
+ * Note: 0 indicates either expired, or infinite duration.
+ *
+ * @return time in seconds
+ */
 uint32_t dcc_duration_seconds(void)
 {
     return DCC_Time_Duration_Seconds;
 }
 
-/* called every second or so.  If more than one second,
+/**
-  then seconds should be the number of seconds to tick away */
+ * Called every second or so.  If more than one second,
+ * then seconds should be the number of seconds to tick away.
+ *
+ * @param seconds  Time passed in seconds, since last call.
+ */
 void dcc_timer_seconds(uint32_t seconds)
 {
     if (DCC_Time_Duration_Seconds) {
@@ -104,6 +131,14 @@ void dcc_timer_seconds(uint32_t seconds)
     }
 }
 
+/**
+ * Set DCC status using duration.
+ *
+ * @param status Enable/disable communication
+ * @param minutes  Duration in minutes
+ *
+ * @return true/false
+ */
 bool dcc_set_status_duration(
     BACNET_COMMUNICATION_ENABLE_DISABLE status, uint16_t minutes)
 {
@@ -124,7 +159,17 @@ bool dcc_set_status_duration(
 }
 
 #if BACNET_SVC_DCC_A
-/* encode service */
+/**
+ * Encode service
+ *
+ * @param apdu  Pointer to the APDU buffer used for encoding.
+ * @param invoke_id  Invoke-Id
+ * @param timeDuration  Optional time duration in minutes.
+ * @param enable_disable  Enable/disable communication
+ * @param password  Pointer to an optional password.
+ *
+ * @return Bytes encoded or zero on an error.
+ */
 int dcc_encode_apdu(uint8_t *apdu,
     uint8_t invoke_id,
     uint16_t timeDuration, /* 0=optional */
@@ -150,9 +195,10 @@ int dcc_encode_apdu(uint8_t *apdu,
         apdu_len += len;
         /* optional password */
         if (password) {
-            /* FIXME: must be at least 1 character, limited to 20 characters */
+            if ((password->length >= 1) && (password->length <= 20)) {
-            len = encode_context_character_string(&apdu[apdu_len], 2, password);
+                len = encode_context_character_string(&apdu[apdu_len], 2, password);
-            apdu_len += len;
+                apdu_len += len;
+            }
         }
     }
 
@@ -160,7 +206,17 @@ int dcc_encode_apdu(uint8_t *apdu,
 }
 #endif
 
-/* decode the service request only */
+/**
+ * Decode the service request only
+ *
+ * @param apdu  Pointer to the received request.
+ * @param apdu_len_max  Valid count of bytes in the buffer.
+ * @param timeDuration  Pointer to the duration given in minutes [optional]
+ * @param enable_disable  Pointer to the variable takingthe communication enable/disable.
+ * @param password  Pointer to the password [optional]
+ *
+ * @return Bytes decoded.
+ */
 int dcc_decode_service_request(uint8_t *apdu,
     unsigned apdu_len_max,
     uint16_t *timeDuration,
@@ -174,7 +230,7 @@ int dcc_decode_service_request(uint8_t *apdu,
     BACNET_UNSIGNED_INTEGER decoded_unsigned = 0;
     uint32_t decoded_enum = 0;
 
-    if (apdu_len_max) {
+    if (apdu && apdu_len_max) {
         /* Tag 0: timeDuration, in minutes --optional-- */
         len = bacnet_unsigned_context_decode(
             &apdu[apdu_len], apdu_len_max - apdu_len, 0, &decoded_unsigned);

src/bacnet/getevent.c

@@ -39,7 +39,14 @@
 
 /** @file getevent.c  Encode/Decode GetEvent services */
 
-/* encode service */
+/** Encode service
+ *
+ * @param apdu APDU buffer to encode to.
+ * @param invoke_id Invoke ID
+ * @param lastReceivedObjectIdentifier  Object identifier
+ *
+ * @return Bytes encoded.
+ */
 int getevent_encode_apdu(uint8_t *apdu,
     uint8_t invoke_id,
     BACNET_OBJECT_ID *lastReceivedObjectIdentifier)
@@ -65,7 +72,14 @@ int getevent_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
-/* decode the service request only */
+/** Decode the service request only
+ *
+ * @param apdu APDU buffer to encode to.
+ * @param apdu_len Valid bytes in the buffer.
+ * @param lastReceivedObjectIdentifier  Object identifier
+ *
+ * @return Bytes encoded.
+ */
 int getevent_decode_service_request(uint8_t *apdu,
     unsigned apdu_len,
     BACNET_OBJECT_ID *lastReceivedObjectIdentifier)
@@ -78,8 +92,10 @@ int getevent_decode_service_request(uint8_t *apdu,
         if (!decode_is_context_tag(&apdu[len++], 0)) {
             return -1;
         }
-        len += decode_object_id(&apdu[len], &lastReceivedObjectIdentifier->type,
+        if (len < apdu_len) {
-            &lastReceivedObjectIdentifier->instance);
+            len += decode_object_id(&apdu[len], &lastReceivedObjectIdentifier->type,
+                &lastReceivedObjectIdentifier->instance);
+        }
     }
 
     return (int)len;

src/bacnet/ihave.c

@@ -39,6 +39,14 @@
 
 /** @file ihave.c  Encode/Decode I-Have service */
 
+/**
+ * Encode the I Have request
+ *
+ * @param apdu  Pointer to the APDU buffer
+ * @param data  Pointer to the I Have data structure.
+ *
+ * @return Bytes encoded.
+ */
 int ihave_encode_apdu(uint8_t *apdu, BACNET_I_HAVE_DATA *data)
 {
     int len = 0; /* length of each encoding */
@@ -67,7 +75,15 @@ int ihave_encode_apdu(uint8_t *apdu, BACNET_I_HAVE_DATA *data)
 
 #if BACNET_SVC_I_HAVE_A
 
-/* decode the service request only */
+/**
+ * Decode the I Have request only
+ *
+ * @param apdu  Pointer to the APDU buffer
+ * @param apdu_len  Valid bytes in the buffer
+ * @param data  Pointer to the I Have data structure.
+ *
+ * @return Bytes decoded.
+ */
 int ihave_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_I_HAVE_DATA *data)
 {
@@ -76,7 +92,7 @@ int ihave_decode_service_request(
     uint32_t len_value = 0;
     BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; /* for decoding */
 
-    if (apdu_len && data) {
+    if ((apdu_len >= 2) && data) {
         /* deviceIdentifier */
         len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
         if (tag_number == BACNET_APPLICATION_TAG_OBJECT_ID) {
@@ -87,6 +103,9 @@ int ihave_decode_service_request(
             return -1;
         }
         /* objectIdentifier */
+        if ((unsigned)len >= apdu_len) {
+            return -1;
+        }
         len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
         if (tag_number == BACNET_APPLICATION_TAG_OBJECT_ID) {
             len += decode_object_id(
@@ -96,6 +115,9 @@ int ihave_decode_service_request(
             return -1;
         }
         /* objectName */
+        if ((unsigned)len >= apdu_len) {
+            return -1;
+        }
         len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
         if (tag_number == BACNET_APPLICATION_TAG_CHARACTER_STRING) {
             len += decode_character_string(
@@ -110,12 +132,21 @@ int ihave_decode_service_request(
     return len;
 }
 
+/**
+ * Decode the I Have
+ *
+ * @param apdu  Pointer to the APDU buffer
+ * @param apdu_len  Valid bytes in the buffer
+ * @param data  Pointer to the I Have data structure.
+ *
+ * @return Bytes decoded.
+ */
 int ihave_decode_apdu(
     uint8_t *apdu, unsigned apdu_len, BACNET_I_HAVE_DATA *data)
 {
     int len = 0;
 
-    if (!apdu) {
+    if ((!apdu) || (apdu_len < 2)) {
         return -1;
     }
     /* optional checking - most likely was already done prior to this call */

src/bacnet/rd.c

@@ -32,6 +32,7 @@
  -------------------------------------------
 ####COPYRIGHTEND####*/
 #include <stdint.h>
+
 #include "bacnet/bacenum.h"
 #include "bacnet/bacdcode.h"
 #include "bacnet/bacdef.h"
@@ -39,7 +40,16 @@
 
 /** @file rd.c  Encode/Decode Reinitialize Device APDUs */
 #if BACNET_SVC_RD_A
-/* encode service */
+
+/** Encode Reinitialize Device service
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param invoke_id Invoke-Id
+ * @param state  Reinitialization state
+ * @param password  Pointer to the pass phrase.
+ *
+ * @return Bytes encoded.
+ */
 int rd_encode_apdu(uint8_t *apdu,
     uint8_t invoke_id,
     BACNET_REINITIALIZED_STATE state,
@@ -58,9 +68,11 @@ int rd_encode_apdu(uint8_t *apdu,
         apdu_len += len;
         /* optional password */
         if (password) {
-            /* FIXME: must be at least 1 character, limited to 20 characters */
+            /* Must be at least 1 character, limited to 20 characters */
-            len = encode_context_character_string(&apdu[apdu_len], 1, password);
+            if ((password->length >= 1) && (password->length <= 20)) {
-            apdu_len += len;
+                len = encode_context_character_string(&apdu[apdu_len], 1, password);
+                apdu_len += len;
+            }
         }
     }
 
@@ -68,7 +80,15 @@ int rd_encode_apdu(uint8_t *apdu,
 }
 #endif
 
-/* decode the service request only */
+/** Decode Reinitialize Device service
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param apdu_len Valid bytes in the buffer
+ * @param state  Pointer to the Reinitialization state
+ * @param password  Pointer to the pass phrase.
+ *
+ * @return Bytes encoded.
+ */
 int rd_decode_service_request(uint8_t *apdu,
     unsigned apdu_len,
     BACNET_REINITIALIZED_STATE *state,
@@ -80,7 +100,7 @@ int rd_decode_service_request(uint8_t *apdu,
     uint32_t value = 0;
 
     /* check for value pointers */
-    if (apdu_len) {
+    if ((apdu) && (apdu_len >= 2)) {
         /* Tag 0: reinitializedStateOfDevice */
         if (!decode_is_context_tag(&apdu[len], 0)) {
             return -1;
@@ -98,8 +118,13 @@ int rd_decode_service_request(uint8_t *apdu,
             }
             len += decode_tag_number_and_value(
                 &apdu[len], &tag_number, &len_value_type);
-            len +=
+            if (len < apdu_len) {
-                decode_character_string(&apdu[len], len_value_type, password);
+                if (password) {
+                    len += decode_character_string(&apdu[len],
+                           len_value_type,
+                           password);
+                }
+            }
         }
     }
 

src/bacnet/readrange.c

@@ -61,10 +61,15 @@
  * }
  */
 
-/*****************************************************************************
+/**
- *  Build a ReadRange request packet.                                        *
+ *  Build a ReadRange request packet.
- *****************************************************************************/
+ *
-
+ *  @param apdu  Pointer to the APDU buffer.
+ *  @param invoke_id  Invoke ID
+ *  @param rrdata  Pointer to the data used for encoding.
+ *
+ *  @return Bytes encoded.
+ */
 int rr_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_READ_RANGE_DATA *rrdata)
 {
@@ -134,10 +139,15 @@ int rr_encode_apdu(
     return apdu_len;
 }
 
-/*****************************************************************************
+/**
- * Decode the received ReadRange request                                     *
+ * Decode the received ReadRange request
- *****************************************************************************/
+ *
-
+ *  @param apdu  Pointer to the APDU buffer.
+ *  @param apdu_len  Bytes valid in the APDU buffer.
+ *  @param rrdata  Pointer to the data used for encoding.
+ *
+ *  @return Bytes encoded.
+ */
 int rr_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_READ_RANGE_DATA *rrdata)
 {
@@ -150,7 +160,7 @@ int rr_decode_service_request(
     BACNET_UNSIGNED_INTEGER unsigned_value;
 
     /* check for value pointers */
-    if (apdu_len && rrdata) {
+    if ((apdu_len >= 5) && apdu && rrdata) {
         /* Tag 0: Object ID */
         if (!decode_is_context_tag(&apdu[len++], 0)) {
             return -1;
@@ -158,6 +168,9 @@ int rr_decode_service_request(
         len += decode_object_id(&apdu[len], &type, &rrdata->object_instance);
         rrdata->object_type = type;
         /* Tag 1: Property ID */
+        if (len >= apdu_len) {
+            return(-1);
+        }
         len += decode_tag_number_and_value(
             &apdu[len], &tag_number, &len_value_type);
         if (tag_number != 1) {
@@ -199,30 +212,60 @@ int rr_decode_service_request(
             switch (tag_number) {
                 case 3: /* ReadRange by position */
                     rrdata->RequestType = RR_BY_POSITION;
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_unsigned(
                         &apdu[len], len_value_type, &unsigned_value);
                     rrdata->Range.RefIndex = (uint32_t)unsigned_value;
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_signed(
                         &apdu[len], len_value_type, &rrdata->Count);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
                     break;
 
                 case 6: /* ReadRange by sequence number */
                     rrdata->RequestType = RR_BY_SEQUENCE;
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_unsigned(
                         &apdu[len], len_value_type, &unsigned_value);
                     rrdata->Range.RefSeqNum = (uint32_t)unsigned_value;
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_signed(
                         &apdu[len], len_value_type, &rrdata->Count);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
                     /* Allow for this in the response */
@@ -231,17 +274,38 @@ int rr_decode_service_request(
 
                 case 7: /* ReadRange by time stamp */
                     rrdata->RequestType = RR_BY_TIME;
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_date(&apdu[len], &rrdata->Range.RefTime.date);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_bacnet_time(
                         &apdu[len], &rrdata->Range.RefTime.time);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_signed(
                         &apdu[len], len_value_type, &rrdata->Count);
+                    if (len >= apdu_len) {
+                        break;
+                    }
                     len += decode_tag_number_and_value(
                         &apdu[len], &tag_number, &len_value_type);
                     /* Allow for this in the response */
@@ -253,6 +317,8 @@ int rr_decode_service_request(
                     break;
             }
         }
+    } else {
+        return(-1);
     }
 
     return (int)len;
@@ -272,13 +338,19 @@ int rr_decode_service_request(
  * }
  */
 
-/*****************************************************************************
+/**
- * Build a ReadRange response packet                                         *
+ * Build a ReadRange response packet
- *****************************************************************************/
+ *
-
+ * @param apdu  Pointer to the buffer.
+ * @param invoke_id  ID invoked.
+ * @param rrdata  Pointer to the read range data structure used for encoding.
+ *
+ * @return The count of encoded bytes.
+ */
 int rr_ack_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_READ_RANGE_DATA *rrdata)
 {
+    int imax = 0;
     int len = 0; /* length of each encoding */
     int apdu_len = 0; /* total length of the apdu, return value */
 
@@ -309,7 +381,11 @@ int rr_ack_encode_apdu(
          */
         apdu_len += encode_opening_tag(&apdu[apdu_len], 5);
         if (rrdata->ItemCount != 0) {
-            for (len = 0; len < rrdata->application_data_len; len++) {
+            imax = rrdata->application_data_len;
+            if (imax > (MAX_APDU - apdu_len - 2 /*closing*/)) {
+                imax = (MAX_APDU - apdu_len - 2);
+            }
+            for (len = 0; len < imax; len++) {
                 apdu[apdu_len++] = rrdata->application_data[len];
             }
         }
@@ -319,18 +395,25 @@ int rr_ack_encode_apdu(
             (rrdata->RequestType != RR_BY_POSITION) &&
             (rrdata->RequestType != RR_READ_ALL)) {
             /* Context 6 Sequence number of first item */
-            apdu_len += encode_context_unsigned(
+            if (apdu_len < (MAX_APDU - 4)) {
-                &apdu[apdu_len], 6, rrdata->FirstSequence);
+                apdu_len += encode_context_unsigned(
+                    &apdu[apdu_len], 6, rrdata->FirstSequence);
+            }
         }
     }
 
     return apdu_len;
 }
 
-/*****************************************************************************
+/**
- * Decode the received ReadRange response                                    *
+ * Decode the received ReadRange response
- *****************************************************************************/
+ *
-
+ *  @param apdu  Pointer to the APDU buffer.
+ *  @param apdu_len  Bytes valid in the APDU buffer.
+ *  @param rrdata  Pointer to the data filled while decoding.
+ *
+ *  @return Bytes decoded.
+ */
 int rr_ack_decode_service_request(uint8_t *apdu,
     int apdu_len, /* total length of the apdu */
     BACNET_READ_RANGE_DATA *rrdata)
@@ -344,90 +427,111 @@ int rr_ack_decode_service_request(uint8_t *apdu,
     uint32_t property = 0; /* for decoding */
     BACNET_UNSIGNED_INTEGER unsigned_value;
 
-    /* FIXME: check apdu_len against the len during decode   */
+    /* Check apdu_len against the len during decode. */
-    /* Tag 0: Object ID */
+    if (apdu && (apdu_len >= 5 /* minimum */)) {
-    if (!decode_is_context_tag(&apdu[0], 0)) {
+        /* Tag 0: Object ID */
-        return -1;
+        if (!decode_is_context_tag(&apdu[0], 0)) {
-    }
-    len = 1;
-    len += decode_object_id(&apdu[len], &object_type, &rrdata->object_instance);
-    rrdata->object_type = object_type;
-
-    /* Tag 1: Property ID */
-    len +=
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-    if (tag_number != 1) {
-        return -1;
-    }
-    len += decode_enumerated(&apdu[len], len_value_type, &property);
-    rrdata->object_property = (BACNET_PROPERTY_ID)property;
-
-    /* Tag 2: Optional Array Index */
-    tag_len =
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-    if (tag_number == 2) {
-        len += tag_len;
-        len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
-        rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
-    } else {
-        rrdata->array_index = BACNET_ARRAY_ALL;
-    }
-
-    /* Tag 3: Result Flags */
-    len +=
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-    if (tag_number != 3) {
-        return -1;
-    }
-
-    len += decode_bitstring(&apdu[len], len_value_type, &rrdata->ResultFlags);
-
-    /* Tag 4: Item count */
-    len +=
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-    if (tag_number != 4) {
-        return -1;
-    }
-
-    len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
-    rrdata->ItemCount = (uint32_t)unsigned_value;
-
-    if (decode_is_opening_tag_number(&apdu[len], 5)) {
-        len++; /* a tag number of 5 is not extended so only one octet */
-        /* Setup the start position and length of the data returned from the
-         * request don't decode the application tag number or its data here */
-        rrdata->application_data = &apdu[len];
-        start_len = len;
-        while (len < apdu_len) {
-            if (IS_CONTEXT_SPECIFIC(apdu[len]) &&
-                (decode_is_closing_tag_number(&apdu[len], 5))) {
-                rrdata->application_data_len = len - start_len;
-                len++; /* Step over single byte closing tag */
-                break;
-            } else {
-                /* Don't care about tag number, just skipping over anyway */
-                len += decode_tag_number_and_value(
-                    &apdu[len], NULL, &len_value_type);
-                len += len_value_type; /* Skip over data value as well */
-                if (len >= apdu_len) { /* APDU is exhausted so we have failed to
-                                        find closing tag */
-                    return (-1);
-                }
-            }
-        }
-    } else {
-        return -1;
-    }
-    if (len < apdu_len) { /* Still something left to look at? */
-        /* Tag 6: Item count */
-        len += decode_tag_number_and_value(
-            &apdu[len], &tag_number, &len_value_type);
-        if (tag_number != 6) {
             return -1;
         }
+        len = 1;
+        len += decode_object_id(&apdu[len], &object_type, &rrdata->object_instance);
+        rrdata->object_type = object_type;
 
+        /* Tag 1: Property ID */
+        if (len >= apdu_len) {
+            return -1;
+        }
+        len +=
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (tag_number != 1) {
+            return -1;
+        }
+        len += decode_enumerated(&apdu[len], len_value_type, &property);
+        rrdata->object_property = (BACNET_PROPERTY_ID)property;
+
+        /* Tag 2: Optional Array Index */
+        if (len >= apdu_len) {
+            return -1;
+        }
+        tag_len =
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (tag_number == 2) {
+            len += tag_len;
+            len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
+            rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
+        } else {
+            rrdata->array_index = BACNET_ARRAY_ALL;
+        }
+
+        /* Tag 3: Result Flags */
+        if (len >= apdu_len) {
+            return -1;
+        }
+        len +=
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (tag_number != 3) {
+            return -1;
+        }
+        if (len >= apdu_len) {
+            return -1;
+        }
+        len += decode_bitstring(&apdu[len], len_value_type, &rrdata->ResultFlags);
+
+        /* Tag 4: Item count */
+        if (len >= apdu_len) {
+            return -1;
+        }
+        len +=
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (tag_number != 4) {
+            return -1;
+        }
+        if (len >= apdu_len) {
+            return -1;
+        }
         len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
-        rrdata->FirstSequence = (uint32_t)unsigned_value;
+        rrdata->ItemCount = (uint32_t)unsigned_value;
+        if (len >= apdu_len) {
+            return -1;
+        }
+        if (decode_is_opening_tag_number(&apdu[len], 5)) {
+            len++; /* A tag number of 5 is not extended so only one octet
+                    * Setup the start position and length of the data returned from the
+                    * request don't decode the application tag number or its data here. */
+            rrdata->application_data = &apdu[len];
+            start_len = len;
+            while (len < apdu_len) {
+                if (IS_CONTEXT_SPECIFIC(apdu[len]) &&
+                    (decode_is_closing_tag_number(&apdu[len], 5))) {
+                    rrdata->application_data_len = len - start_len;
+                    len++; /* Step over single byte closing tag */
+                    break;
+                } else {
+                    /* Don't care about tag number, just skipping over anyway */
+                    len += decode_tag_number_and_value(
+                        &apdu[len], NULL, &len_value_type);
+                    len += len_value_type; /* Skip over data value as well */
+                    if (len >= apdu_len) { /* APDU is exhausted so we have failed to
+                                            * find closing tag */
+                        return (-1);
+                    }
+                }
+            }
+        } else {
+            return -1;
+        }
+        if (len < apdu_len) { /* Still something left to look at? */
+            /* Tag 6: Item count */
+            len += decode_tag_number_and_value(
+                &apdu[len], &tag_number, &len_value_type);
+            if (tag_number != 6) {
+                return -1;
+            }
+            if (len < apdu_len) {
+                len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
+                rrdata->FirstSequence = (uint32_t)unsigned_value;
+            }
+        }
     }
 
     return len;

src/bacnet/reject.c

@@ -141,7 +141,14 @@ BACNET_ERROR_CODE reject_convert_to_error_code(BACNET_REJECT_REASON reject_code)
     return (error_code);
 }
 
-/* encode service */
+/** Encode service
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param invoke_id  Invoke-Id.
+ * @param reject_reason  Reason for having rejected.
+ *
+ * @return Bytes encoded, typically 3.
+ */
 int reject_encode_apdu(uint8_t *apdu, uint8_t invoke_id, uint8_t reject_reason)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -157,7 +164,17 @@ int reject_encode_apdu(uint8_t *apdu, uint8_t invoke_id, uint8_t reject_reason)
 }
 
 #if !BACNET_SVC_SERVER
-/* decode the service request only */
+
+/** Decode the service request only.
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param apdu_len  Bytes valid in the buffer
+ * @param invoke_id  Invoke-Id.
+ * @param reject_reason  Pointer to the variable taking
+ *        the reason for having rejected.
+ *
+ * @return Bytes encoded, typically 3.
+ */
 int reject_decode_service_request(uint8_t *apdu,
     unsigned apdu_len,
     uint8_t *invoke_id,
@@ -170,7 +187,11 @@ int reject_decode_service_request(uint8_t *apdu,
             *invoke_id = apdu[0];
         }
         if (reject_reason) {
-            *reject_reason = apdu[1];
+            if (apdu_len > 1) {
+                *reject_reason = apdu[1];
+            } else {
+                *reject_reason = 0;
+            }
         }
     }
 

src/bacnet/rp.c

@@ -40,7 +40,15 @@
 /** @file rp.c  Encode/Decode Read Property and RP ACKs */
 
 #if BACNET_SVC_RP_A
-/* encode service */
+
+/** Encode the service
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param invoke_id  Invoke ID
+ * @param rpdata  Pointer to the property data to be encoded.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rp_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata)
 {
@@ -60,6 +68,7 @@ int rp_encode_apdu(
                 rpdata->object_type, rpdata->object_instance);
             apdu_len += len;
         }
+        /* The value should be in the range of 0 to 4194303. */
         if (rpdata->object_property <= MAX_BACNET_PROPERTY_ID) {
             /* check bounds so that we could create malformed
                messages for testing */
@@ -79,7 +88,14 @@ int rp_encode_apdu(
 }
 #endif
 
-/* decode the service request only */
+/** Decode the service request only
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param apdu_len  Count of valid bytes inthe buffer.
+ * @param rpdata  Pointer to the property data to be encoded.
+ *
+ * @return Bytes decoded or zero on error.
+ */
 int rp_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_READ_PROPERTY_DATA *rpdata)
 {
@@ -143,7 +159,14 @@ int rp_decode_service_request(
     return (int)len;
 }
 
-/* alternate method to encode the ack without extra buffer */
+/** Alternate method to encode the ack without extra buffer.
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param invoke_id  Invoke Id
+ * @param rpdata  Pointer to the property data to be encoded.
+ *
+ * @return Bytes decoded or zero on error.
+ */
 int rp_ack_encode_apdu_init(
     uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata)
 {
@@ -176,7 +199,15 @@ int rp_ack_encode_apdu_init(
     return apdu_len;
 }
 
-/* note: encode the application tagged data yourself */
+/** Encode the closing tag for the object property.
+ *  Note: Encode the application tagged data yourself.
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param invoke_id  Invoke Id
+ * @param rpdata  Pointer to the property data to be encoded.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rp_ack_encode_apdu_object_property_end(uint8_t *apdu)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -188,17 +219,31 @@ int rp_ack_encode_apdu_object_property_end(uint8_t *apdu)
     return apdu_len;
 }
 
+/** Encode the acknowledge.
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param invoke_id  Invoke Id
+ * @param rpdata  Pointer to the property data to be encoded.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rp_ack_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata)
 {
+    int imax = 0;
     int len = 0; /* length of each encoding */
     int apdu_len = 0; /* total length of the apdu, return value */
 
     if (apdu) {
         /* Do the initial encoding */
         apdu_len = rp_ack_encode_apdu_init(apdu, invoke_id, rpdata);
-        /* propertyValue */
+        /* propertyValue
-        for (len = 0; len < rpdata->application_data_len; len++) {
+         * double check maximum possible */
+        imax = rpdata->application_data_len;
+        if (imax > (MAX_APDU - apdu_len - 2)) {
+            imax = (MAX_APDU - apdu_len - 2);
+        }
+        for (len = 0; len < imax; len++) {
             apdu[apdu_len++] = rpdata->application_data[len];
         }
         apdu_len += encode_closing_tag(&apdu[apdu_len], 3);
@@ -231,41 +276,62 @@ int rp_ack_decode_service_request(uint8_t *apdu,
     uint32_t property = 0; /* for decoding */
     BACNET_UNSIGNED_INTEGER unsigned_value = 0; /* for decoding */
 
-    /* FIXME: check apdu_len against the len during decode   */
+    /* Check basics. */
-    /* Tag 0: Object ID */
+    if (apdu && (apdu_len >= 8 /*minimum*/)) {
-    if (!decode_is_context_tag(&apdu[0], 0)) {
+        /* Tag 0: Object ID */
-        return -1;
+        if (!decode_is_context_tag(&apdu[0], 0)) {
-    }
+            return -1;
-    len = 1;
+        }
-    len += decode_object_id(&apdu[len], &object_type, &rpdata->object_instance);
+        len = 1;
-    rpdata->object_type = object_type;
+        len += decode_object_id(&apdu[len], &object_type, &rpdata->object_instance);
-    /* Tag 1: Property ID */
+        rpdata->object_type = object_type;
-    len +=
+        /* Tag 1: Property ID */
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (len >= apdu_len) {
-    if (tag_number != 1) {
+            return -1;
-        return -1;
+        }
-    }
+        len +=
-    len += decode_enumerated(&apdu[len], len_value_type, &property);
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-    rpdata->object_property = (BACNET_PROPERTY_ID)property;
+        if (tag_number != 1) {
-    /* Tag 2: Optional Array Index */
+            return -1;
-    tag_len =
+        }
-        decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
+        if (len >= apdu_len) {
-    if (tag_number == 2) {
+            return -1;
-        len += tag_len;
+        }
-        len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
+        len += decode_enumerated(&apdu[len], len_value_type, &property);
-        rpdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
+        rpdata->object_property = (BACNET_PROPERTY_ID)property;
-    } else {
+        /* Tag 2: Optional Array Index */
-        rpdata->array_index = BACNET_ARRAY_ALL;
+        if (len >= apdu_len) {
-    }
+            return -1;
-    /* Tag 3: opening context tag */
+        }
-    if (decode_is_opening_tag_number(&apdu[len], 3)) {
+        tag_len =
-        /* a tag number of 3 is not extended so only one octet */
+            decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type);
-        len++;
+        if (tag_number == 2) {
-        /* don't decode the application tag number or its data here */
+            len += tag_len;
-        rpdata->application_data = &apdu[len];
+            len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
-        rpdata->application_data_len = apdu_len - len - 1 /*closing tag */;
+            rpdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
-        /* len includes the data and the closing tag */
+        } else {
-        len = apdu_len;
+            rpdata->array_index = BACNET_ARRAY_ALL;
+        }
+        /* Tag 3: opening context tag */
+        if (len >= apdu_len) {
+            return -1;
+        }
+        if (decode_is_opening_tag_number(&apdu[len], 3)) {
+            /* a tag number of 3 is not extended so only one octet */
+            len++;
+            /* don't decode the application tag number or its data here */
+            rpdata->application_data = &apdu[len];
+            /* Just to ensure we do not create a wrapped over value here. */
+            if (len < apdu_len) {
+                rpdata->application_data_len = apdu_len - len - 1 /*closing tag */;
+            } else {
+                rpdata->application_data_len = 0;
+            }
+            /* len includes the data and the closing tag */
+            len = apdu_len;
+        } else {
+            return -1;
+        }
     } else {
         return -1;
     }

src/bacnet/rpm.c

@@ -43,7 +43,14 @@
 /** @file rpm.c  Encode/Decode Read Property Multiple and RPM ACKs  */
 
 #if BACNET_SVC_RPM_A
-/* encode the initial portion of the service */
+
+/** Encode the initial portion of the service
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param invoke_id  Invoke ID
+ *
+ * @return Bytes encoded (usually 4) or zero on error.
+ */
 int rpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -59,6 +66,15 @@ int rpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
     return apdu_len;
 }
 
+/** Encode the beginning, including
+ *  Object-id and Read-Access of the service.
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param object_type  Object type to encode
+ * @param object_instance  Object instance to encode
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rpm_encode_apdu_object_begin(
     uint8_t *apdu, BACNET_OBJECT_TYPE object_type, uint32_t object_instance)
 {
@@ -74,6 +90,14 @@ int rpm_encode_apdu_object_begin(
     return apdu_len;
 }
 
+/** Encode the object properties of the service.
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ * @param object_property  Object property.
+ * @param array_index Optional array index.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rpm_encode_apdu_object_property(
     uint8_t *apdu, BACNET_PROPERTY_ID object_property, BACNET_ARRAY_INDEX array_index)
 {
@@ -91,6 +115,12 @@ int rpm_encode_apdu_object_property(
     return apdu_len;
 }
 
+/** Encode the end (closing tag) of the service
+ *
+ * @param apdu  Pointer to the buffer for encoding.
+ *
+ * @return Bytes encoded or zero on error.
+ */
 int rpm_encode_apdu_object_end(uint8_t *apdu)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -130,6 +160,8 @@ int rpm_encode_apdu(uint8_t *apdu,
     apdu_len += len;
     rpm_object = read_access_data;
     while (rpm_object) {
+        /* The encode function will return a length not more than 12. So the temp buffer
+         * being 16 bytes is fine enought. */
         len = encode_context_object_id(&apdu_temp[0], 0,
             rpm_object->object_type, rpm_object->object_instance);
         len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len,
@@ -148,7 +180,9 @@ int rpm_encode_apdu(uint8_t *apdu,
         apdu_len += len;
         rpm_property = rpm_object->listOfProperties;
         while (rpm_property) {
-            /* stuff as many properties into it as APDU length will allow */
+            /* The encode function will return a length not more than 12. So the temp buffer
+             * being 16 bytes is fine enought.
+             * Stuff as many properties into it as APDU length will allow. */
             len = encode_context_enumerated(
                 &apdu_temp[0], 0, rpm_property->propertyIdentifier);
             len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len,
@@ -169,6 +203,10 @@ int rpm_encode_apdu(uint8_t *apdu,
                 apdu_len += len;
             }
             rpm_property = rpm_property->next;
+            /* Full? */
+            if ((unsigned)apdu_len >= max_apdu) {
+                return 0;
+            }
         }
         len = encode_closing_tag(&apdu_temp[0], 1);
         len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len,
@@ -185,8 +223,14 @@ int rpm_encode_apdu(uint8_t *apdu,
 
 #endif
 
-/* decode the object portion of the service request only. Bails out if
+/** Decode the object portion of the service request only. Bails out if
- * tags are wrong or missing/incomplete
+ *  tags are wrong or missing/incomplete.
+ *
+ * @param apdu [in] Buffer of bytes received.
+ * @param apdu_len [in] Count of valid bytes in the buffer.
+ * @param rpmdata [in] The data structure to be filled.
+ *
+ * @return Length of decoded bytes, or 0 on failure.
  */
 int rpm_decode_object_id(
     uint8_t *apdu, unsigned apdu_len, BACNET_RPM_DATA *rpmdata)
@@ -218,6 +262,13 @@ int rpm_decode_object_id(
     return len;
 }
 
+/** Decode the end portion of the service request only.
+ *
+ * @param apdu [in] Buffer of bytes received.
+ * @param apdu_len [in] Count of valid bytes in the buffer.
+ *
+ * @return Length of decoded bytes (usually 1), or 0 on failure.
+ */
 int rpm_decode_object_end(uint8_t *apdu, unsigned apdu_len)
 {
     int len = 0; /* total length of the apdu, return value */
@@ -231,14 +282,20 @@ int rpm_decode_object_end(uint8_t *apdu, unsigned apdu_len)
     return len;
 }
 
-/* decode the object property portion of the service request only */
+/** Decode the object property portion of the service request only
-/*  BACnetPropertyReference ::= SEQUENCE {
+ *  BACnetPropertyReference ::= SEQUENCE {
-        propertyIdentifier [0] BACnetPropertyIdentifier,
+ *      propertyIdentifier [0] BACnetPropertyIdentifier,
-        propertyArrayIndex [1] Unsigned OPTIONAL
+ *      propertyArrayIndex [1] Unsigned OPTIONAL
-        --used only with array datatype
+ *      --used only with array datatype
-        -- if omitted with an array the entire array is referenced
+ *      -- if omitted with an array the entire array is referenced
-    }
+ *  }
-*/
+ *
+ *  @param apdu  Pointer to received bytes.
+ *  @param apdu_len  Count of received bytes.
+ *  @param rpmdata  Pointer to the data structure to be filled.
+ *
+ *  @return Bytes decoded or zero on failure.
+ */
 int rpm_decode_object_property(
     uint8_t *apdu, unsigned apdu_len, BACNET_RPM_DATA *rpmdata)
 {
@@ -294,6 +351,13 @@ int rpm_decode_object_property(
     return len;
 }
 
+/** Encode the acknowledge for a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ * @param invoke_id [in] Invoke Id.
+ *
+ * @return Length of encoded bytes (usually 3) or 0 on failure.
+ */
 int rpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -308,6 +372,13 @@ int rpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
     return apdu_len;
 }
 
+/** Encode the object type for an acknowledge of a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ * @param rpmdata [in] Pointer to the data used to fill in the APDU.
+ *
+ * @return Length of encoded bytes or 0 on failure.
+ */
 int rpm_ack_encode_apdu_object_begin(uint8_t *apdu, BACNET_RPM_DATA *rpmdata)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -323,6 +394,14 @@ int rpm_ack_encode_apdu_object_begin(uint8_t *apdu, BACNET_RPM_DATA *rpmdata)
     return apdu_len;
 }
 
+/** Encode the object property for an acknowledge of a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ * @param object_property [in] Object property ID.
+ * @param array_index  Optional array index
+ *
+ * @return Length of encoded bytes or 0 on failure.
+ */
 int rpm_ack_encode_apdu_object_property(
     uint8_t *apdu, BACNET_PROPERTY_ID object_property, BACNET_ARRAY_INDEX array_index)
 {
@@ -333,14 +412,21 @@ int rpm_ack_encode_apdu_object_property(
         apdu_len = encode_context_enumerated(&apdu[0], 2, object_property);
         /* Tag 3: optional propertyArrayIndex */
         if (array_index != BACNET_ARRAY_ALL) {
-            apdu_len +=
+            apdu_len += encode_context_unsigned(&apdu[apdu_len], 3, array_index);
-                encode_context_unsigned(&apdu[apdu_len], 3, array_index);
         }
     }
 
     return apdu_len;
 }
 
+/** Encode the object property value for an acknowledge of a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ * @param application_data [in] Pointer to the application data used to fill in the APDU.
+ * @param application_data_len [in] Length of the application data.
+ *
+ * @return Length of encoded bytes or 0 on failure.
+ */
 int rpm_ack_encode_apdu_object_property_value(
     uint8_t *apdu, uint8_t *application_data, unsigned application_data_len)
 {
@@ -364,6 +450,14 @@ int rpm_ack_encode_apdu_object_property_value(
     return apdu_len;
 }
 
+/** Encode the object property error for an acknowledge of a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ * @param error_class [in] Error Class
+ * @param error_code [in] Error Code
+ *
+ * @return Length of encoded bytes or 0 on failure.
+ */
 int rpm_ack_encode_apdu_object_property_error(
     uint8_t *apdu, BACNET_ERROR_CLASS error_class, BACNET_ERROR_CODE error_code)
 {
@@ -380,6 +474,12 @@ int rpm_ack_encode_apdu_object_property_error(
     return apdu_len;
 }
 
+/** Encode the end tag for an acknowledge of a RPM.
+ *
+ * @param apdu [in] Buffer of bytes to transmit.
+ *
+ * @return Length of encoded bytes or 0 on failure.
+ */
 int rpm_ack_encode_apdu_object_end(uint8_t *apdu)
 {
     int apdu_len = 0; /* total length of the apdu, return value */

src/bacnet/wp.c

@@ -38,13 +38,22 @@
 #include "bacnet/wp.h"
 
 /** @file wp.c  Encode/Decode BACnet Write Property APDUs  */
+
 #if BACNET_SVC_WP_A
-/* encode service */
+/** Initialize the APDU for encode service.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param invoke_id  ID of service invoked.
+ * @param wpdata  Pointer to the write property data.
+ *
+ * @return Bytes encoded
+ */
 int wp_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_WRITE_PROPERTY_DATA *wpdata)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
     int len = 0; /* total length of the apdu, return value */
+    int imax = 0; /* maximum application data length */
 
     if (apdu) {
         apdu[0] = PDU_TYPE_CONFIRMED_SERVICE_REQUEST;
@@ -67,10 +76,14 @@ int wp_encode_apdu(
         /* propertyValue */
         len = encode_opening_tag(&apdu[apdu_len], 3);
         apdu_len += len;
-        for (len = 0; len < wpdata->application_data_len; len++) {
+        imax = wpdata->application_data_len;
+        if (imax > (MAX_APDU - 2 /*closing*/ - apdu_len)) {
+            imax = MAX_APDU - 2 - apdu_len;
+        }
+        for (len = 0; len < imax; len++) {
             apdu[apdu_len + len] = wpdata->application_data[len];
         }
-        apdu_len += wpdata->application_data_len;
+        apdu_len += imax;
         len = encode_closing_tag(&apdu[apdu_len], 3);
         apdu_len += len;
         /* optional priority - 0 if not set, 1..16 if set */
@@ -84,9 +97,17 @@ int wp_encode_apdu(
 }
 #endif
 
-/* decode the service request only */
+/** Decode the service request only
-/* FIXME: there could be various error messages returned
+ *
-   using unique values less than zero */
+ * FIXME: there could be various error messages returned
+ * using unique values less than zero.
+ *
+ * @param apdu  Pointer to the buffer.
+ * @param apdu_len  Valid bytes in the buffer
+ * @param wpdata  Pointer to the write property data.
+ *
+ * @return Bytes encoded or a negative value as error.
+ */
 int wp_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_WRITE_PROPERTY_DATA *wpdata)
 {
@@ -98,6 +119,7 @@ int wp_decode_service_request(
     uint32_t property = 0; /* for decoding */
     BACNET_UNSIGNED_INTEGER unsigned_value = 0;
     int i = 0; /* loop counter */
+    int imax = 0; /* max application data length */
 
     /* check for value pointers */
     if (apdu_len && wpdata) {
@@ -132,16 +154,23 @@ int wp_decode_service_request(
             return -1;
         }
         /* determine the length of the data blob */
-        wpdata->application_data_len = bacapp_data_len(
+        imax = bacapp_data_len( \
-            &apdu[len], apdu_len - len, (BACNET_PROPERTY_ID)property);
+               &apdu[len], apdu_len - len, (BACNET_PROPERTY_ID)property);
+        if (imax == BACNET_STATUS_ERROR) {
+            return -2;
+        }
         /* a tag number of 3 is not extended so only one octet */
         len++;
         /* copy the data from the APDU */
-        for (i = 0; i < wpdata->application_data_len; i++) {
+        if (imax > (MAX_APDU - len - 1 /*closing*/)) {
+            imax = (MAX_APDU - len - 1);
+        }
+        for (i = 0; i < imax; i++) {
             wpdata->application_data[i] = apdu[len + i];
         }
+        wpdata->application_data_len = imax;
         /* add on the data length */
-        len += wpdata->application_data_len;
+        len += imax;
         if (!decode_is_closing_tag_number(&apdu[len], 3)) {
             return -2;
         }

src/bacnet/wpm.c

@@ -46,8 +46,10 @@
  *
  * @param apdu [in] The contents of the APDU buffer.
  * @param apdu_len [in] The length of the APDU buffer.
- * @param data [out] The BACNET_WRITE_PROPERTY_DATA structure
+ * @param wp_data [out] The BACNET_WRITE_PROPERTY_DATA structure
  *    which will contain the reponse values or error.
+ *
+ * @return Count of decoded bytes.
  */
 int wpm_decode_object_id(
     uint8_t *apdu, uint16_t apdu_len, BACNET_WRITE_PROPERTY_DATA *wp_data)
@@ -102,6 +104,14 @@ int wpm_decode_object_id(
     return (int)len;
 }
 
+/** Decoding for an object property.
+ *
+ * @param apdu [in] The contents of the APDU buffer.
+ * @param apdu_len [in] The length of the APDU buffer.
+ * @param wp_data [out] The BACNET_WRITE_PROPERTY_DATA structure.
+ *
+ * @return Bytes decoded
+ */
 int wpm_decode_object_property(
     uint8_t *apdu, uint16_t apdu_len, BACNET_WRITE_PROPERTY_DATA *wp_data)
 {
@@ -109,7 +119,7 @@ int wpm_decode_object_property(
     uint32_t len_value = 0;
     uint32_t enum_value = 0;
     BACNET_UNSIGNED_INTEGER unsigned_value = 0;
-    int len = 0, i = 0;
+    int len = 0, i = 0, imax = 0;
 
     if ((apdu) && (apdu_len) && (wp_data)) {
         wp_data->array_index = BACNET_ARRAY_ALL;
@@ -137,19 +147,28 @@ int wpm_decode_object_property(
         /* tag 2 - Property Value */
         if ((tag_number == 2) && (decode_is_opening_tag(&apdu[len - 1]))) {
             len--;
-            wp_data->application_data_len = bacapp_data_len(&apdu[len],
+            imax = bacapp_data_len(&apdu[len],
                 (unsigned)(apdu_len - len), wp_data->object_property);
             len++;
 
-            /* copy application data */
+            /* copy application data, check max lengh */
-            for (i = 0; i < wp_data->application_data_len; i++) {
+            if (imax > (apdu_len - len)) {
+                imax = (apdu_len - len);
+            }
+            for (i = 0; i < imax; i++) {
                 wp_data->application_data[i] = apdu[len + i];
             }
-            len += wp_data->application_data_len;
+            wp_data->application_data_len = imax;
-            len += decode_tag_number_and_value(
+            len += imax;
-                &apdu[len], &tag_number, &len_value);
+            if (len < apdu_len) {
-            /* closing tag 2 */
+                len += decode_tag_number_and_value(
-            if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) {
+                    &apdu[len], &tag_number, &len_value);
+                /* closing tag 2 */
+                if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) {
+                    wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+                    return BACNET_STATUS_REJECT;
+                }
+            } else {
                 wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
                 return BACNET_STATUS_REJECT;
             }
@@ -159,12 +178,14 @@ int wpm_decode_object_property(
         }
 
         /* tag 3 - Priority - optional */
-        len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
+        if (len < apdu_len) {
-        if (tag_number == 3) {
+            len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
-            len += decode_unsigned(&apdu[len], len_value, &unsigned_value);
+            if (tag_number == 3) {
-            wp_data->priority = (uint8_t)unsigned_value;
+                len += decode_unsigned(&apdu[len], len_value, &unsigned_value);
-        } else {
+                wp_data->priority = (uint8_t)unsigned_value;
-            len--;
+            } else {
+                len--;
+            }
         }
     } else {
         if (wp_data) {
@@ -176,7 +197,13 @@ int wpm_decode_object_property(
     return len;
 }
 
-/* encode functions */
+/** Init the APDU for encoding.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param invoke_id [in] The ID of the saervice invoked.
+ *
+ * @return Bytes encoded, usually 4 on success.
+ */
 int wpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -192,6 +219,14 @@ int wpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
     return apdu_len;
 }
 
+/** Decode the very begin of an object in the APDU.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param object_type [in] The object type to decode.
+ * @param object_instance [in] The object instance.
+ *
+ * @return Bytes encoded.
+ */
 int wpm_encode_apdu_object_begin(
     uint8_t *apdu, BACNET_OBJECT_TYPE object_type, uint32_t object_instance)
 {
@@ -207,6 +242,12 @@ int wpm_encode_apdu_object_begin(
     return apdu_len;
 }
 
+/** Decode the very end of an object in the APDU.
+ *
+ * @param apdu [in] The APDU buffer.
+ *
+ * @return Bytes encoded.
+ */
 int wpm_encode_apdu_object_end(uint8_t *apdu)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
@@ -218,11 +259,19 @@ int wpm_encode_apdu_object_end(uint8_t *apdu)
     return apdu_len;
 }
 
+/** Encode the object property into the APDU.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param wpdata [in] Pointer to the property data.
+ *
+ * @return Bytes encoded.
+ */
 int wpm_encode_apdu_object_property(
     uint8_t *apdu, BACNET_WRITE_PROPERTY_DATA *wpdata)
 {
     int apdu_len = 0; /* total length of the apdu, return value */
     int len = 0;
+    int imax;
 
     if (apdu) {
         apdu_len =
@@ -233,20 +282,35 @@ int wpm_encode_apdu_object_property(
                 &apdu[apdu_len], 1, wpdata->array_index);
         }
         apdu_len += encode_opening_tag(&apdu[apdu_len], 2);
-        for (len = 0; len < wpdata->application_data_len; len++) {
+        imax = wpdata->application_data_len;
+        if (imax > (MAX_APDU - apdu_len - 2)) {
+            imax = (MAX_APDU - apdu_len - 2);
+        }
+        for (len = 0; len < imax; len++) {
             apdu[apdu_len] = wpdata->application_data[len];
             apdu_len++;
         }
         apdu_len += encode_closing_tag(&apdu[apdu_len], 2);
         if (wpdata->priority != BACNET_NO_PRIORITY) {
-            apdu_len +=
+            if (apdu_len < MAX_APDU) {
-                encode_context_unsigned(&apdu[apdu_len], 3, wpdata->priority);
+                apdu_len +=
+                    encode_context_unsigned(&apdu[apdu_len], 3, wpdata->priority);
+            }
         }
     }
 
     return apdu_len;
 }
 
+/** Encode the request into the APDU.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param max_apdu [in] Maximum space in the buffer.
+ * @param invoke_id [in] Invoked service ID.
+ * @param write_access_data [in] Access data.
+ *
+ * @return Bytes encoded.
+ */
 int wpm_encode_apdu(uint8_t *apdu,
     size_t max_apdu,
     uint8_t invoke_id,
@@ -254,6 +318,7 @@ int wpm_encode_apdu(uint8_t *apdu,
 {
     int apdu_len = 0;
     int len = 0;
+    size_t usize;
     BACNET_WRITE_ACCESS_DATA *wpm_object; /* current object */
     uint8_t apdu_temp[MAX_APDU]; /* temp for data before copy */
     BACNET_PROPERTY_VALUE *wpm_property; /* current property */
@@ -271,6 +336,9 @@ int wpm_encode_apdu(uint8_t *apdu,
             len = wpm_encode_apdu_object_begin(&apdu[apdu_len],
                 wpm_object->object_type, wpm_object->object_instance);
             apdu_len += len;
+            if (apdu_len >= (int)max_apdu) {
+                break;
+            }
 
             wpm_property = wpm_object->listOfProperties;
 
@@ -278,12 +346,18 @@ int wpm_encode_apdu(uint8_t *apdu,
                 wpdata.object_property = wpm_property->propertyIdentifier;
                 wpdata.array_index = wpm_property->propertyArrayIndex;
                 wpdata.priority = wpm_property->priority;
-                wpdata.application_data_len =
+                usize = (size_t)bacapp_encode_data(&apdu_temp[0], &wpm_property->value);
-                    bacapp_encode_data(&apdu_temp[0], &wpm_property->value);
+                if (usize > sizeof(wpdata.application_data))
-                memcpy(&wpdata.application_data[0], &apdu_temp[0],
+                {
-                    (size_t)wpdata.application_data_len);
+                    usize = sizeof(wpdata.application_data);
+                }
+                wpdata.application_data_len = (int)usize;
+                memcpy(&wpdata.application_data[0], &apdu_temp[0], usize);
                 len = wpm_encode_apdu_object_property(&apdu[apdu_len], &wpdata);
                 apdu_len += len;
+                if (apdu_len >= (int)max_apdu) {
+                    break;
+                }
 
                 wpm_property = wpm_property->next;
             }
@@ -298,6 +372,13 @@ int wpm_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
+/** Init the APDU for encoding the confiremd write property multiple service.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param invoke_id [in] Invoked service ID.
+ *
+ * @return Bytes encoded, usually 3.
+ */
 int wpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
 {
     int len = 0;
@@ -311,6 +392,14 @@ int wpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id)
     return len;
 }
 
+/** Encode an Error acknowledge in the APDU.
+ *
+ * @param apdu [in] The APDU buffer.
+ * @param invoke_id [in] Invoked service ID.
+ * @param wp_data [in] Data of the invoked property.
+ *
+ * @return Bytes encoded.
+ */
 int wpm_error_ack_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_WRITE_PROPERTY_DATA *wp_data)
 {