Feature/comments and buffer checks (#73)

* Added comments

* Replaced Goto construct

* Added apdu_len check

* Added comments

* Added string limit and reworked printable check

* Mainly comments

* Just comments

* Just comments

* decode service request returns now non zero on success

* eliminated warnings

* Added character string init with length check.

* Paranoic length check

* Comments and object index checking on read/write.

* Check name/desc strings before returning.

* Eliminated Goto

ports/linux/bip-init.c

@@ -422,7 +422,9 @@ static int get_local_ifr_ioctl(char *ifname, struct ifreq *ifr, int request)
     int fd;
     int rv; /* return value */
 
-    strncpy(ifr->ifr_name, ifname, sizeof(ifr->ifr_name));
+    strncpy(ifr->ifr_name, ifname, sizeof(ifr->ifr_name) - 1);
+    ifr->ifr_name[sizeof(ifr->ifr_name) - 1] = 0;
+
     fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);
     if (fd < 0) {
         rv = fd;
@@ -443,7 +445,7 @@ static int get_local_ifr_ioctl(char *ifname, struct ifreq *ifr, int request)
  */
 int bip_get_local_address_ioctl(char *ifname, struct in_addr *addr, int request)
 {
-    struct ifreq ifr = { { { 0 } } };
+    struct ifreq ifr = { {{0}}, {{0}} };
     struct sockaddr_in *tcpip_address;
     int rv; /* return value */
 
@@ -535,7 +537,7 @@ bool bip_init(char *ifname)
     struct sockaddr_in sin;
     int sockopt = 0;
     int sock_fd = -1;
-    char *ifname_default = "eth0";
+    static char *ifname_default = "eth0";
 
     if (ifname) {
         bip_set_interface(ifname);

src/bacnet/abort.c

@@ -133,7 +133,17 @@ BACNET_ERROR_CODE abort_convert_to_error_code(BACNET_ABORT_REASON abort_code)
     return (error_code);
 }
 
-/* encode service */
+/**
+ * @brief Encode the BACnet Abort Service, returning the reason
+ *        for the operation being aborted.
+ *
+ * @param apdu  Transmit buffer
+ * @param invoke_id  ID invoked
+ * @param abort_reason  Abort reason, see ABORT_REASON_X enumeration for details
+ * @param server  True, if the abort has been issued by this device.
+ *
+ * @return Total length of the apdu, typically 3 on success, zero otherwise.
+ */
 int abort_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, uint8_t abort_reason, bool server)
 {
@@ -154,7 +164,17 @@ int abort_encode_apdu(
 }
 
 #if !BACNET_SVC_SERVER
-/* decode the service request only */
+/**
+ * @brief Decode the BACnet Abort Service, returning the reason
+ *        for the operation being aborted.
+ *
+ * @param apdu  Receive buffer
+ * @param apdu_len  Count of bytes valid in the received buffer.
+ * @param invoke_id  Pointer to a variable, taking the invoked ID from the message.
+ * @param abort_reason  Pointer to a variable, taking the abort reason, see ABORT_REASON_X enumeration for details
+ *
+ * @return Total length of the apdu, typically 2 on success, zero otherwise.
+ */
 int abort_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, uint8_t *invoke_id, uint8_t *abort_reason)
 {
@@ -164,9 +184,15 @@ int abort_decode_service_request(
         if (invoke_id) {
             *invoke_id = apdu[0];
         }
+        len++;
+
+        if (apdu_len > 1) {
             if (abort_reason) {
                 *abort_reason = apdu[1];
             }
+
+            len++;
+        }
     }
 
     return len;

src/bacnet/alarm_ack.c

@@ -37,11 +37,15 @@
 
 /** @file alarm_ack.c  Handles Event Notifications (ACKs) */
 
-/***************************************************
+/**
-**
+ * @brief Creates an Unconfirmed Event Notification APDU
-** Creates an Unconfirmed Event Notification APDU
+ *
-**
+ * @param apdu  Transmission buffer
-****************************************************/
+ * @param invoke_id  Invoked ID
+ * @param data  Alarm acknowledge data that will be copied into the payload.
+ *
+ * @return Length of the APDU
+ */
 int alarm_ack_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, BACNET_ALARM_ACK_DATA *data)
 {
@@ -62,11 +66,15 @@ int alarm_ack_encode_apdu(
     return apdu_len;
 }
 
-/***************************************************
+/**
-**
+ * @brief Encodes the service data part of Event Notification.
-** Encodes the service data part of Event Notification
+ *
-**
+ * @param apdu  Receive buffer
-****************************************************/
+ * @param data  Alarm acknowledge data that will be filled in
+ *              with the data from the payload.
+ *
+ * @return Total length of the apdu
+ */
 int alarm_ack_encode_service_request(uint8_t *apdu, BACNET_ALARM_ACK_DATA *data)
 {
     int len = 0; /* length of each encoding */
@@ -102,11 +110,15 @@ int alarm_ack_encode_service_request(uint8_t *apdu, BACNET_ALARM_ACK_DATA *data)
     return apdu_len;
 }
 
-/***************************************************
+/**
-**
+ * @brief Decodes the service data part of Event Notification.
-** Decodes the service data part of Event Notification
+ *
-**
+ * @param apdu  Receive buffer
-****************************************************/
+ * @param apdu_len  Bytes valid in the buffer.
+ * @param data  Alarm acknowledge data that will be filled.
+ *
+ * @return Total length of the apdu
+ */
 int alarm_ack_decode_service_request(
     uint8_t *apdu, unsigned apdu_len, BACNET_ALARM_ACK_DATA *data)
 {

src/bacnet/bacapp.c

@@ -158,8 +158,18 @@ int bacapp_encode_application_data(
     return apdu_len;
 }
 
-/* decode the data and store it into value.
+/**
-   Return the number of octets consumed. */
+ * @brief Decode the data and store it into value.
+ *        Return the number of octets consumed.
+ *
+ * @param apdu  Receive buffer
+ * @param tag_data_type  Data type of the given tag
+ * @param len_value_type  Count of bytes of given tag
+ * @param value  Pointer to the application value structure,
+ *               used to store the decoded value to.
+ *
+ * @return Number of octets consumed.
+ */
 int bacapp_decode_data(uint8_t *apdu,
     uint8_t tag_data_type,
     uint32_t len_value_type,

src/bacnet/bacdcode.c

@@ -1981,9 +1981,15 @@ int encode_context_signed(uint8_t *apdu, uint8_t tag_number, int32_t value)
 }
 #endif
 
-/* from clause 20.2.6 Encoding of a Real Number Value */
+/**
-/* and 20.2.1 General Rules for Encoding BACnet Tags */
+ * @brief Encode a real floating value. From clause 20.2.6 Encoding of a
-/* returns the number of apdu bytes consumed */
+ *        Real Number Value and 20.2.1 General Rules for Encoding BACnet Tags.
+ *
+ * @param apdu  Transmit buffer
+ * @param value The float value to be encoded.
+ *
+ * @return Returns the number of apdu bytes consumed.
+ */
 int encode_application_real(uint8_t *apdu, float value)
 {
     int len = 0;
@@ -1996,6 +2002,17 @@ int encode_application_real(uint8_t *apdu, float value)
     return len;
 }
 
+/**
+ * @brief Encode a real floating value with a given tag. From clause 20.2.6
+ *        Encoding of a Real Number Value and 20.2.1 General Rules for
+ *        Encoding BACnet Tags.
+ *
+ * @param apdu  Transmit buffer
+ * @param tag_number  Tag number to be used
+ * @param value The float value to be encoded.
+ *
+ * @return Returns the number of apdu bytes consumed.
+ */
 int encode_context_real(uint8_t *apdu, uint8_t tag_number, float value)
 {
     int len = 0;
@@ -2356,7 +2373,15 @@ int decode_context_date(uint8_t *apdu, uint8_t tag_number, BACNET_DATE *bdate)
     return len;
 }
 
-/* returns the number of apdu bytes consumed */
+/**
+ * Encode a simple ACK and returns the number of apdu bytes consumed.
+ *
+ * @param apdu  Transmit buffer
+ * @param invoke_id  ID invoked
+ * @param service_choice  Service being acked
+ *
+ * @return number of apdu bytes consumed
+ */
 int encode_simple_ack(uint8_t *apdu, uint8_t invoke_id, uint8_t service_choice)
 {
     apdu[0] = PDU_TYPE_SIMPLE_ACK;
@@ -2366,11 +2391,20 @@ int encode_simple_ack(uint8_t *apdu, uint8_t invoke_id, uint8_t service_choice)
     return 3;
 }
 
-/* BACnetAddress */
+/**
+ * Encode a BACnetAddress and returns the number of apdu bytes consumed.
+ *
+ * @param apdu  Transmit buffer
+ * @param destination  Pointer to the destination address to be encoded.
+ *
+ * @return number of apdu bytes created
+ */
 int encode_bacnet_address(uint8_t *apdu, BACNET_ADDRESS *destination)
 {
     int apdu_len = 0;
     BACNET_OCTET_STRING mac_addr;
+
+    if (destination) {
         /* network number */
         apdu_len += encode_application_unsigned(&apdu[apdu_len], destination->net);
         /* encode mac address as an octet-string */
@@ -2380,10 +2414,18 @@ int encode_bacnet_address(uint8_t *apdu, BACNET_ADDRESS *destination)
             octetstring_init(&mac_addr, destination->mac, destination->mac_len);
         }
         apdu_len += encode_application_octet_string(&apdu[apdu_len], &mac_addr);
+    }
     return apdu_len;
 }
 
-/* BACnetAddress */
+/**
+ * Dencode a BACnetAddress and returns the number of apdu bytes consumed.
+ *
+ * @param apdu  Receive buffer
+ * @param destination  Pointer to the destination address structure to be filled in.
+ *
+ * @return number of apdu bytes consumed
+ */
 int decode_bacnet_address(uint8_t *apdu, BACNET_ADDRESS *destination)
 {
     int len = 0;

src/bacnet/bacstr.c

@@ -299,6 +299,15 @@ bool characterstring_init(BACNET_CHARACTER_STRING *char_string,
     return status;
 }
 
+bool characterstring_init_ansi_safe(
+    BACNET_CHARACTER_STRING * char_string,
+    const char *value,
+    size_t tmax)
+{
+    return characterstring_init(char_string, CHARACTER_ANSI_X34, value,
+        value ? strnlen(value, tmax) : 0);
+}
+
 bool characterstring_init_ansi(
     BACNET_CHARACTER_STRING *char_string, const char *value)
 {
@@ -431,7 +440,13 @@ bool characterstring_truncate(
     return status;
 }
 
-/* Returns the value. */
+/**
+ * @brief Returns the pointer to the C-string for the given BACnet string.
+ *
+ * @param char_string  Pointer to the character string.
+ *
+ * @return Pointer to a zero-terminated C-string.
+ */
 char *characterstring_value(BACNET_CHARACTER_STRING *char_string)
 {
     char *value = NULL;
@@ -443,19 +458,37 @@ char *characterstring_value(BACNET_CHARACTER_STRING *char_string)
     return value;
 }
 
-/* returns the length. */
+/**
+ * @brief Returns the length for the given BACnet string.
+ *
+ * @param char_string  Pointer to the character string.
+ *
+ * @return Length of the charcater string, but
+ *         maximum MAX_CHARACTER_STRING_BYTES.
+ */
 size_t characterstring_length(BACNET_CHARACTER_STRING *char_string)
 {
     size_t length = 0;
 
     if (char_string) {
-        /* FIXME: validate length is within bounds? */
         length = char_string->length;
+
+        /* Length within bounds? */
+        if (length > CHARACTER_STRING_CAPACITY) {
+            length = CHARACTER_STRING_CAPACITY;
+        }
     }
 
     return length;
 }
 
+/**
+ * @brief Returns the possible capacity for the given BACnet string.
+ *
+ * @param char_string  Pointer to the character string.
+ *
+ * @return MAX_CHARACTER_STRING_BYTES
+ */
 size_t characterstring_capacity(BACNET_CHARACTER_STRING *char_string)
 {
     size_t length = 0;
@@ -467,7 +500,13 @@ size_t characterstring_capacity(BACNET_CHARACTER_STRING *char_string)
     return length;
 }
 
-/* returns the encoding. */
+/**
+ * @brief Returns the character encoding for the given BACnet string.
+ *
+ * @param char_string  Pointer to the character string.
+ *
+ * @return Encoding, like CHARACTER_ANSI_X34
+ */
 uint8_t characterstring_encoding(BACNET_CHARACTER_STRING *char_string)
 {
     uint8_t encoding = 0;
@@ -479,7 +518,14 @@ uint8_t characterstring_encoding(BACNET_CHARACTER_STRING *char_string)
     return encoding;
 }
 
-/* returns the encoding. */
+/**
+ * @brief Set the character encoding for the given BACnet string.
+ *
+ * @param char_string  Pointer to the character string.
+ * @param Encoding, like CHARACTER_ANSI_X34
+ *
+ * @return true/false on error
+ */
 bool characterstring_set_encoding(
     BACNET_CHARACTER_STRING *char_string, uint8_t encoding)
 {
@@ -493,32 +539,43 @@ bool characterstring_set_encoding(
     return status;
 }
 
-/* returns true if string is printable */
+/**
-/* used to assist in the requirement that
+ * @brief Returns true if string is printable.
-   "The set of characters used in the Object_Name shall be
+ *
-   restricted to printable characters." */
+ * Used to assist in the requirement that
-/* printable character: a character that represents a printable
+ * "The set of characters used in the Object_Name shall be
-symbol as opposed to a device control character. These
+ * restricted to printable characters."
-include, but are not limited to, upper- and lowercase letters,
+ *
-punctuation marks, and mathematical symbols. The exact set
+ * Printable character: a character that represents a printable
-depends upon the character set being used. In ANSI X3.4 the
+ * symbol as opposed to a device control character. These
-printable characters are represented by single octets in the range
+ * include, but are not limited to, upper- and lowercase letters,
-X'20' - X'7E'.*/
+ * punctuation marks, and mathematical symbols. The exact set
+ * depends upon the character set being used. In ANSI X3.4 the
+ * printable characters are represented by single octets in the range
+ * X'20' - X'7E'.
+ *
+ * @param char_string  Pointer to the character string.
+ *
+ * @return true/false on error
+ */
 bool characterstring_printable(BACNET_CHARACTER_STRING *char_string)
 {
     bool status = false; /* return value */
     size_t i; /* counter */
+    size_t imax;
+    char chr;
 
     if (char_string) {
         if (char_string->encoding == CHARACTER_ANSI_X34) {
             status = true;
-            for (i = 0; i < MAX_CHARACTER_STRING_BYTES; i++) {
+            imax = char_string->length;
-                if (i < char_string->length) {
+            if (imax > CHARACTER_STRING_CAPACITY) {
-                    if ((char_string->value[i] < 0x20) ||
+                imax = CHARACTER_STRING_CAPACITY;
-                        (char_string->value[i] > 0x7E)) {
-                        status = false;
             }
-                } else {
+            for (i = 0; i < imax; i++) {
+                chr = char_string->value[i];
+                if ((chr < 0x20) || (chr > 0x7E)) {
+                    status = false;
                     break;
                 }
             }

src/bacnet/bacstr.h

@@ -120,6 +120,10 @@ extern "C" {
     bool characterstring_init_ansi(
         BACNET_CHARACTER_STRING * char_string,
         const char *value);
+    bool characterstring_init_ansi_safe(
+        BACNET_CHARACTER_STRING * char_string,
+        const char *value,
+        size_t tmax);
     bool characterstring_copy(
         BACNET_CHARACTER_STRING * dest,
         BACNET_CHARACTER_STRING * src);

src/bacnet/basic/binding/address.c

@@ -94,11 +94,23 @@ void address_own_device_id_set(uint32_t own_id)
     Own_Device_ID = own_id;
 }
 
+/**
+ * @brief Check if the given source and destination address can be matched
+ *        by checking the length, net and MAC address.
+ *
+ * @param dest  Destination address
+ * @param src   Source address
+ *
+ * @return true for a match, false otherwise
+ */
 bool address_match(BACNET_ADDRESS *dest, BACNET_ADDRESS *src)
 {
     uint8_t i = 0;
     uint8_t max_len = 0;
 
+    if (dest == src) {
+        return(true);
+    }
     if (dest->mac_len != src->mac_len) {
         return false;
     }
@@ -136,6 +148,11 @@ bool address_match(BACNET_ADDRESS *dest, BACNET_ADDRESS *src)
     return true;
 }
 
+/**
+ * @brief Remove a device from the address list.
+ *
+ * @param device_id  ID of the device
+ */
 void address_remove_device(uint32_t device_id)
 {
     struct Address_Cache_Entry *pMatch;
@@ -158,13 +175,15 @@ void address_remove_device(uint32_t device_id)
     return;
 }
 
-/*****************************************************************************
+/**
- * Search the cache for the entry nearest expiry and delete it. Mark the     *
+ * @brief Search the cache for the entry nearest expiry and delete it. Mark the
- * entry as reserved with a 1 hour TTL and return a pointer to the reserved  *
+ * entry as reserved with a 1 hour TTL and return a pointer to the reserved
- * entry. Will not delete a static entry and returns NULL pointer if no      *
+ * entry. Will not delete a static entry and returns NULL pointer if no
- * entry available to free up. Does not check for free entries as it is      *
+ * 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.                     *
+ * assumed we are calling this due to the lack of those.
- *****************************************************************************/
+ *
+ * @return Candidate for deletation.
+ */
 
 static struct Address_Cache_Entry *address_remove_oldest(void)
 {
@@ -224,7 +243,8 @@ static struct Address_Cache_Entry *address_remove_oldest(void)
     return (pCandidate);
 }
 
-/** Initialize a BACNET_MAC_ADDRESS
+/**
+ * Initialize a BACNET_MAC_ADDRESS
  *
  * @param mac [out] BACNET_MAC_ADDRESS structure
  * @param adr [in] address to initialize, null if empty

src/bacnet/basic/npdu/h_npdu.c

@@ -70,6 +70,10 @@ void npdu_handler(BACNET_ADDRESS *src, /* source address */
     BACNET_ADDRESS dest = { 0 };
     BACNET_NPDU_DATA npdu_data = { 0 };
 
+    if (pdu_len < 1) {
+        return;
+    }
+
     /* only handle the version that we know how to handle */
     if (pdu[0] == BACNET_PROTOCOL_VERSION) {
         apdu_offset = npdu_decode(&pdu[0], &dest, src, &npdu_data);

src/bacnet/basic/object/av.c

@@ -63,6 +63,13 @@ static const int Analog_Value_Properties_Optional[] = { PROP_DESCRIPTION,
 
 static const int Analog_Value_Properties_Proprietary[] = { -1 };
 
+/**
+ * Initialize the pointers for the required, the optional and the properitary value properties.
+ *
+ * @param pRequired - Pointer to the pointer of required values.
+ * @param pOptional - Pointer to the pointer of optional values.
+ * @param pProprietary - Pointer to the pointer of properitary values.
+ */
 void Analog_Value_Property_Lists(
     const int **pRequired, const int **pOptional, const int **pProprietary)
 {
@@ -79,6 +86,9 @@ void Analog_Value_Property_Lists(
     return;
 }
 
+/**
+ * Initialize the analog values.
+ */
 void Analog_Value_Init(void)
 {
     unsigned i;
@@ -116,9 +126,15 @@ void Analog_Value_Init(void)
     }
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need validate that the */
+ * We simply have 0-n object instances. Yours might be
-/* given instance exists */
+ * more complex, and then you need validate that the
+ * given instance exists.
+ *
+ * @param object_instance Object instance
+ *
+ * @return true/false
+ */
 bool Analog_Value_Valid_Instance(uint32_t object_instance)
 {
     if (object_instance < MAX_ANALOG_VALUES) {
@@ -128,24 +144,39 @@ bool Analog_Value_Valid_Instance(uint32_t object_instance)
     return false;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then count how many you have */
+ * Return the count of analog values.
+ *
+ * @return Count of analog values.
+ */
 unsigned Analog_Value_Count(void)
 {
     return MAX_ANALOG_VALUES;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the instance */
+ * We simply have 0-n object instances.  Yours might be
-/* that correlates to the correct index */
+ * more complex, and then you need to return the instance
+ * that correlates to the correct index.
+ *
+ * @param index Index
+ *
+ * @return Object instance
+ */
 uint32_t Analog_Value_Index_To_Instance(unsigned index)
 {
     return index;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the index */
+ * We simply have 0-n object instances.  Yours might be
-/* that correlates to the correct instance number */
+ * more complex, and then you need to return the index
+ * that correlates to the correct instance number
+ *
+ * @param object_instance Object instance
+ *
+ * @return Index in the object table.
+ */
 unsigned Analog_Value_Instance_To_Index(uint32_t object_instance)
 {
     unsigned index = MAX_ANALOG_VALUES;
@@ -157,6 +188,16 @@ unsigned Analog_Value_Instance_To_Index(uint32_t object_instance)
     return index;
 }
 
+/**
+ * This function is used to detect a value change,
+ * using the new value compared against the prior
+ * value, using a delta as threshold.
+ *
+ * This method will update the COV-changed attribute.
+ *
+ * @param index  Object index
+ * @param value  Given present value.
+ */
 static void Analog_Value_COV_Detect(unsigned int index, float value)
 {
     float prior_value = 0.0;
@@ -203,6 +244,13 @@ bool Analog_Value_Present_Value_Set(
     return status;
 }
 
+/**
+ * For a given object instance-number, return the present value.
+ *
+ * @param  object_instance - object-instance number of the object
+ *
+ * @return  Present value
+ */
 float Analog_Value_Present_Value(uint32_t object_instance)
 {
     float value = 0;
@@ -216,7 +264,16 @@ float Analog_Value_Present_Value(uint32_t object_instance)
     return value;
 }
 
-/* note: the object name must be unique within this device */
+/**
+ * For a given object instance-number, return the name.
+ *
+ * Note: the object name must be unique within this device
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  object_name - object name/string pointer
+ *
+ * @return  true/false
+ */
 bool Analog_Value_Object_Name(
     uint32_t object_instance, BACNET_CHARACTER_STRING *object_name)
 {
@@ -370,7 +427,13 @@ void Analog_Value_Out_Of_Service_Set(uint32_t object_instance, bool value)
     }
 }
 
-/* return apdu len, or BACNET_STATUS_ERROR on error */
+/**
+ * Return the requested property of the analog value.
+ *
+ * @param rpdata  Property requested, see for BACNET_READ_PROPERTY_DATA details.
+ *
+ * @return apdu len, or BACNET_STATUS_ERROR on error
+ */
 int Analog_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
 {
     int apdu_len = 0; /* return value */
@@ -386,7 +449,11 @@ int Analog_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     unsigned i = 0;
 #endif
 
-    if ((rpdata == NULL) || (rpdata->application_data == NULL) ||
+    /* Valid data? */
+    if (rpdata == NULL) {
+        return 0;
+    }
+    if ((rpdata->application_data == NULL) ||
         (rpdata->application_data_len == 0)) {
         return 0;
     }
@@ -394,12 +461,14 @@ int Analog_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     apdu = rpdata->application_data;
 
     object_index = Analog_Value_Instance_To_Index(rpdata->object_instance);
-    if (object_index < MAX_ANALOG_VALUES) {
+    if (object_index >= MAX_ANALOG_VALUES) {
-        CurrentAV = &AV_Descr[object_index];
+        rpdata->error_class = ERROR_CLASS_OBJECT;
-    } else {
+        rpdata->error_code = ERROR_CODE_UNKNOWN_OBJECT;
         return BACNET_STATUS_ERROR;
     }
 
+    CurrentAV = &AV_Descr[object_index];
+
     switch (rpdata->object_property) {
         case PROP_OBJECT_IDENTIFIER:
             apdu_len = encode_application_object_id(
@@ -408,9 +477,10 @@ int Analog_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
 
         case PROP_OBJECT_NAME:
         case PROP_DESCRIPTION:
-            Analog_Value_Object_Name(rpdata->object_instance, &char_string);
+            if (Analog_Value_Object_Name(rpdata->object_instance, &char_string)) {
                 apdu_len =
                     encode_application_character_string(&apdu[0], &char_string);
+            }
             break;
 
         case PROP_OBJECT_TYPE:
@@ -594,7 +664,13 @@ int Analog_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     return apdu_len;
 }
 
-/* returns true if successful */
+/**
+ * Set the requested property of the analog value.
+ *
+ * @param wp_data  Property requested, see for BACNET_WRITE_PROPERTY_DATA details.
+ *
+ * @return true if successful
+ */
 bool Analog_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
 {
     bool status = false; /* return value */
@@ -603,6 +679,15 @@ bool Analog_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
     BACNET_APPLICATION_DATA_VALUE value;
     ANALOG_VALUE_DESCR *CurrentAV;
 
+    /* Valid data? */
+    if (wp_data == NULL) {
+        return false;
+    }
+    if ((wp_data->application_data == NULL) ||
+        (wp_data->application_data_len == 0)) {
+        return false;
+    }
+
     /* decode the some of the request */
     len = bacapp_decode_application_data(
         wp_data->application_data, wp_data->application_data_len, &value);
@@ -621,13 +706,17 @@ bool Analog_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
         wp_data->error_code = ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY;
         return false;
     }
+
+    /* Valid object? */
     object_index = Analog_Value_Instance_To_Index(wp_data->object_instance);
-    if (object_index < MAX_ANALOG_VALUES) {
+    if (object_index >= MAX_ANALOG_VALUES) {
-        CurrentAV = &AV_Descr[object_index];
+        wp_data->error_class = ERROR_CLASS_OBJECT;
-    } else {
+        wp_data->error_code = ERROR_CODE_UNKNOWN_OBJECT;
         return false;
     }
 
+    CurrentAV = &AV_Descr[object_index];
+
     switch (wp_data->object_property) {
         case PROP_PRESENT_VALUE:
             if (value.tag == BACNET_APPLICATION_TAG_REAL) {

src/bacnet/basic/object/bv.c

@@ -62,6 +62,13 @@ static const int Binary_Value_Properties_Optional[] = { PROP_DESCRIPTION,
 
 static const int Binary_Value_Properties_Proprietary[] = { -1 };
 
+/**
+ * Initialize the pointers for the required, the optional and the properitary value properties.
+ *
+ * @param pRequired - Pointer to the pointer of required values.
+ * @param pOptional - Pointer to the pointer of optional values.
+ * @param pProprietary - Pointer to the pointer of properitary values.
+ */
 void Binary_Value_Property_Lists(
     const int **pRequired, const int **pOptional, const int **pProprietary)
 {
@@ -78,6 +85,9 @@ void Binary_Value_Property_Lists(
     return;
 }
 
+/**
+ * Initialize the binary values.
+ */
 void Binary_Value_Init(void)
 {
     unsigned i, j;
@@ -97,9 +107,15 @@ void Binary_Value_Init(void)
     return;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need validate that the */
+ * We simply have 0-n object instances. Yours might be
-/* given instance exists */
+ * more complex, and then you need validate that the
+ * given instance exists.
+ *
+ * @param object_instance Object instance
+ *
+ * @return true/false
+ */
 bool Binary_Value_Valid_Instance(uint32_t object_instance)
 {
     if (object_instance < MAX_BINARY_VALUES) {
@@ -109,24 +125,39 @@ bool Binary_Value_Valid_Instance(uint32_t object_instance)
     return false;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then count how many you have */
+ * Return the count of analog values.
+ *
+ * @return Count of binary values.
+ */
 unsigned Binary_Value_Count(void)
 {
     return MAX_BINARY_VALUES;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the instance */
+ * We simply have 0-n object instances.  Yours might be
-/* that correlates to the correct index */
+ * more complex, and then you need to return the instance
+ * that correlates to the correct index.
+ *
+ * @param index Index
+ *
+ * @return Object instance
+ */
 uint32_t Binary_Value_Index_To_Instance(unsigned index)
 {
     return index;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the index */
+ * We simply have 0-n object instances. Yours might be
-/* that correlates to the correct instance number */
+ * more complex, and then you need to return the index
+ * that correlates to the correct instance number
+ *
+ * @param object_instance Object instance
+ *
+ * @return Index in the object table.
+ */
 unsigned Binary_Value_Instance_To_Index(uint32_t object_instance)
 {
     unsigned index = MAX_BINARY_VALUES;
@@ -138,6 +169,13 @@ unsigned Binary_Value_Instance_To_Index(uint32_t object_instance)
     return index;
 }
 
+/**
+ * For a given object instance-number, return the present value.
+ *
+ * @param  object_instance - object-instance number of the object
+ *
+ * @return  Present value
+ */
 BACNET_BINARY_PV Binary_Value_Present_Value(uint32_t object_instance)
 {
     BACNET_BINARY_PV value = RELINQUISH_DEFAULT;
@@ -157,7 +195,16 @@ BACNET_BINARY_PV Binary_Value_Present_Value(uint32_t object_instance)
     return value;
 }
 
-/* note: the object name must be unique within this device */
+/**
+ * For a given object instance-number, return the name.
+ *
+ * Note: the object name must be unique within this device
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  object_name - object name/string pointer
+ *
+ * @return  true/false
+ */
 bool Binary_Value_Object_Name(
     uint32_t object_instance, BACNET_CHARACTER_STRING *object_name)
 {
@@ -173,6 +220,13 @@ bool Binary_Value_Object_Name(
     return status;
 }
 
+/**
+ * Return the OOO value, if any.
+ *
+ * @param instance Object instance.
+ *
+ * @return true/false
+ */
 bool Binary_Value_Out_Of_Service(uint32_t instance)
 {
     unsigned index = 0;
@@ -186,6 +240,12 @@ bool Binary_Value_Out_Of_Service(uint32_t instance)
     return oos_flag;
 }
 
+/**
+ * Set the OOO value, if any.
+ *
+ * @param instance Object instance.
+ * @param oos_flag New OOO value.
+ */
 void Binary_Value_Out_Of_Service_Set(uint32_t instance, bool oos_flag)
 {
     unsigned index = 0;
@@ -196,7 +256,13 @@ void Binary_Value_Out_Of_Service_Set(uint32_t instance, bool oos_flag)
     }
 }
 
-/* return apdu len, or BACNET_STATUS_ERROR on error */
+/**
+ * Return the requested property of the binary value.
+ *
+ * @param rpdata  Property requested, see for BACNET_READ_PROPERTY_DATA details.
+ *
+ * @return apdu len, or BACNET_STATUS_ERROR on error
+ */
 int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
 {
     int len = 0;
@@ -209,10 +275,23 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     bool state = false;
     uint8_t *apdu = NULL;
 
-    if ((rpdata == NULL) || (rpdata->application_data == NULL) ||
+    /* Valid data? */
+    if (rpdata == NULL) {
+        return 0;
+    }
+    if ((rpdata->application_data == NULL) ||
         (rpdata->application_data_len == 0)) {
         return 0;
     }
+
+    /* Valid object index? */
+    object_index = Binary_Value_Instance_To_Index(rpdata->object_instance);
+    if (object_index >= MAX_BINARY_VALUES) {
+        rpdata->error_class = ERROR_CLASS_OBJECT;
+        rpdata->error_code = ERROR_CODE_UNKNOWN_OBJECT;
+        return BACNET_STATUS_ERROR;
+    }
+
     apdu = rpdata->application_data;
     switch (rpdata->object_property) {
         case PROP_OBJECT_IDENTIFIER:
@@ -223,9 +302,10 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
                You could make Description writable and different */
         case PROP_OBJECT_NAME:
         case PROP_DESCRIPTION:
-            Binary_Value_Object_Name(rpdata->object_instance, &char_string);
+            if (Binary_Value_Object_Name(rpdata->object_instance, &char_string)) {
                 apdu_len =
                     encode_application_character_string(&apdu[0], &char_string);
+            }
             break;
         case PROP_OBJECT_TYPE:
             apdu_len =
@@ -263,8 +343,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
                  */
                 /* into one packet. */
             } else if (rpdata->array_index == BACNET_ARRAY_ALL) {
-                object_index =
-                    Binary_Value_Instance_To_Index(rpdata->object_instance);
                 for (i = 0; i < BACNET_MAX_PRIORITY; i++) {
                     /* FIXME: check if we have room before adding it to APDU */
                     if (Binary_Value_Level[object_index][i] == BINARY_NULL) {
@@ -285,8 +363,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
                     }
                 }
             } else {
-                object_index =
-                    Binary_Value_Instance_To_Index(rpdata->object_instance);
                 if (rpdata->array_index <= BACNET_MAX_PRIORITY) {
                     if (Binary_Value_Level[object_index][rpdata->array_index] ==
                         BINARY_NULL) {
@@ -314,7 +390,8 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
             apdu_len = BACNET_STATUS_ERROR;
             break;
     }
-    /*  only array properties can have array options */
+
+    /* Only array properties can have array options. */
     if ((apdu_len >= 0) && (rpdata->object_property != PROP_PRIORITY_ARRAY) &&
         (rpdata->array_index != BACNET_ARRAY_ALL)) {
         rpdata->error_class = ERROR_CLASS_PROPERTY;
@@ -325,7 +402,13 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     return apdu_len;
 }
 
-/* returns true if successful */
+/**
+ * Set the requested property of the binary value.
+ *
+ * @param wp_data  Property requested, see for BACNET_WRITE_PROPERTY_DATA details.
+ *
+ * @return true if successful
+ */
 bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
 {
     bool status = false; /* return value */
@@ -335,7 +418,16 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
     int len = 0;
     BACNET_APPLICATION_DATA_VALUE value;
 
-    /* decode the some of the request */
+    /* Valid data? */
+    if (wp_data == NULL) {
+        return false;
+    }
+    if ((wp_data->application_data == NULL) || \
+        (wp_data->application_data_len == 0)) {
+        return false;
+    }
+
+    /* Decode the some of the request. */
     len = bacapp_decode_application_data(
         wp_data->application_data, wp_data->application_data_len, &value);
     /* FIXME: len < application_data_len: more data? */
@@ -345,13 +437,22 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
         wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE;
         return false;
     }
-    /*  only array properties can have array options */
+    /* Only array properties can have array options. */
     if ((wp_data->object_property != PROP_PRIORITY_ARRAY) &&
         (wp_data->array_index != BACNET_ARRAY_ALL)) {
         wp_data->error_class = ERROR_CLASS_PROPERTY;
         wp_data->error_code = ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY;
         return false;
     }
+
+    /* Valid object index? */
+    object_index = Binary_Value_Instance_To_Index(wp_data->object_instance);
+    if (object_index >= MAX_BINARY_VALUES) {
+        wp_data->error_class = ERROR_CLASS_OBJECT;
+        wp_data->error_code = ERROR_CODE_UNKNOWN_OBJECT;
+        return false;
+    }
+
     switch (wp_data->object_property) {
         case PROP_PRESENT_VALUE:
             if (value.tag == BACNET_APPLICATION_TAG_ENUMERATED) {
@@ -363,8 +464,6 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
                     (priority != 6 /* reserved */) &&
                     (value.type.Enumerated <= MAX_BINARY_PV)) {
                     level = (BACNET_BINARY_PV)value.type.Enumerated;
-                    object_index = Binary_Value_Instance_To_Index(
-                        wp_data->object_instance);
                     priority--;
                     Binary_Value_Level[object_index][priority] = level;
                     /* Note: you could set the physical output here if we
@@ -389,8 +488,6 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
                     &wp_data->error_class, &wp_data->error_code);
                 if (status) {
                     level = BINARY_NULL;
-                    object_index = Binary_Value_Instance_To_Index(
-                        wp_data->object_instance);
                     priority = wp_data->priority;
                     if (priority && (priority <= BACNET_MAX_PRIORITY)) {
                         priority--;

src/bacnet/basic/object/csv.c

@@ -28,6 +28,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <string.h>
 #include "bacnet/bacdef.h"
 #include "bacnet/bacdcode.h"
 #include "bacnet/bacenum.h"
@@ -60,6 +61,13 @@ static const int Properties_Optional[] = { PROP_EVENT_STATE,
 
 static const int Properties_Proprietary[] = { -1 };
 
+/**
+ * Initialize the pointers for the required, the optional and the properitary value properties.
+ *
+ * @param pRequired - Pointer to the pointer of required values.
+ * @param pOptional - Pointer to the pointer of optional values.
+ * @param pProprietary - Pointer to the pointer of properitary values.
+ */
 void CharacterString_Value_Property_Lists(
     const int **pRequired, const int **pOptional, const int **pProprietary)
 {
@@ -76,6 +84,9 @@ void CharacterString_Value_Property_Lists(
     return;
 }
 
+/**
+ * Initialize the character string values.
+ */
 void CharacterString_Value_Init(void)
 {
     unsigned i;
@@ -92,9 +103,15 @@ void CharacterString_Value_Init(void)
     return;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the index */
+ * We simply have 0-n object instances. Yours might be
-/* that correlates to the correct instance number */
+ * more complex, and then you need to return the instance
+ * that correlates to the correct index.
+ *
+ * @param object_instance Object instance
+ *
+ * @return Object instance
+ */
 unsigned CharacterString_Value_Instance_To_Index(uint32_t object_instance)
 {
     unsigned index = MAX_CHARACTERSTRING_VALUES;
@@ -106,21 +123,39 @@ unsigned CharacterString_Value_Instance_To_Index(uint32_t object_instance)
     return index;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then you need to return the instance */
+ * We simply have 0-n object instances. Yours might be
-/* that correlates to the correct index */
+ * more complex, and then you need to return the index
+ * that correlates to the correct instance number
+ *
+ * @param index Object index
+ *
+ * @return Index in the object table.
+ */
 uint32_t CharacterString_Value_Index_To_Instance(unsigned index)
 {
     return index;
 }
 
-/* we simply have 0-n object instances.  Yours might be */
+/**
-/* more complex, and then count how many you have */
+ * Return the count of character string values.
+ *
+ * @return Count of character string values.
+ */
 unsigned CharacterString_Value_Count(void)
 {
     return MAX_CHARACTERSTRING_VALUES;
 }
 
+/**
+ * We simply have 0-n object instances. Yours might be
+ * more complex, and then you need validate that the
+ * given instance exists.
+ *
+ * @param object_instance Object instance
+ *
+ * @return true/false
+ */
 bool CharacterString_Value_Valid_Instance(uint32_t object_instance)
 {
     unsigned index = 0; /* offset from instance lookup */
@@ -133,6 +168,16 @@ bool CharacterString_Value_Valid_Instance(uint32_t object_instance)
     return false;
 }
 
+/**
+ * For a given object instance-number, read the present-value.
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  object_name - Pointer to the new BACnet string value,
+ *                       taking the value.
+ *
+ * @return  true if values are within range and present-value
+ *          is returned.
+ */
 bool CharacterString_Value_Present_Value(
     uint32_t object_instance, BACNET_CHARACTER_STRING *object_name)
 {
@@ -147,6 +192,15 @@ bool CharacterString_Value_Present_Value(
     return status;
 }
 
+/**
+ * For a given object instance-number, sets the present-value,
+ * taken from another BACnet string.
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  object_name - Pointer to the new BACnet string value.
+ *
+ * @return  true if values are within range and present-value is set.
+ */
 bool CharacterString_Value_Present_Value_Set(
     uint32_t object_instance, BACNET_CHARACTER_STRING *object_name)
 {
@@ -161,6 +215,13 @@ bool CharacterString_Value_Present_Value_Set(
     return status;
 }
 
+/**
+ * For a given object instance-number, read the out of service value.
+ *
+ * @param  object_instance - object-instance number of the object
+ *
+ * @return  true/false
+ */
 bool CharacterString_Value_Out_Of_Service(uint32_t object_instance)
 {
     bool value = false;
@@ -174,6 +235,12 @@ bool CharacterString_Value_Out_Of_Service(uint32_t object_instance)
     return value;
 }
 
+/**
+ * For a given object instance-number, set the out of service value.
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  true/false
+ */
 static void CharacterString_Value_Out_Of_Service_Set(
     uint32_t object_instance, bool value)
 {
@@ -187,6 +254,13 @@ static void CharacterString_Value_Out_Of_Service_Set(
     return;
 }
 
+/**
+ * For a given object instance-number, return the description.
+ *
+ * @param  object_instance - object-instance number of the object
+ *
+ * @return  C-string pointer to the description.
+ */
 static char *CharacterString_Value_Description(uint32_t object_instance)
 {
     unsigned index = 0; /* offset from instance lookup */
@@ -200,8 +274,16 @@ static char *CharacterString_Value_Description(uint32_t object_instance)
     return pName;
 }
 
+/**
+ * For a given object instance-number, set the description text.
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  new_descr - C-String pointer to the string, representing the description text
+ *
+ * @return True on success, false otherwise.
+ */
 bool CharacterString_Value_Description_Set(
-    uint32_t object_instance, char *new_name)
+    uint32_t object_instance, char *new_descr)
 {
     unsigned index = 0; /* offset from instance lookup */
     size_t i = 0; /* loop counter */
@@ -210,23 +292,29 @@ bool CharacterString_Value_Description_Set(
     index = CharacterString_Value_Instance_To_Index(object_instance);
     if (index < MAX_CHARACTERSTRING_VALUES) {
         status = true;
-        if (new_name) {
+        if (new_descr) {
             for (i = 0; i < sizeof(Object_Description[index]); i++) {
-                Object_Description[index][i] = new_name[i];
+                Object_Description[index][i] = new_descr[i];
-                if (new_name[i] == 0) {
+                if (new_descr[i] == 0) {
                     break;
                 }
             }
         } else {
-            for (i = 0; i < sizeof(Object_Description[index]); i++) {
+            memset(&Object_Description[index][0], 0, sizeof(Object_Description[index]));
-                Object_Description[index][i] = 0;
-            }
         }
     }
 
     return status;
 }
 
+/**
+ * For a given object instance-number, return the object text.
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  object_name - Pointer to the BACnet string object that shall take the object name
+ *
+ * @return True on success, false otherwise.
+ */
 bool CharacterString_Value_Object_Name(
     uint32_t object_instance, BACNET_CHARACTER_STRING *object_name)
 {
@@ -241,7 +329,16 @@ bool CharacterString_Value_Object_Name(
     return status;
 }
 
-/* note: the object name must be unique within this device */
+/**
+ * For a given object instance-number, set the object text.
+ *
+ * Note: the object name must be unique within this device
+ *
+ * @param  object_instance - object-instance number of the object
+ * @param  new_name - C-String pointer to the text, representing the object name
+ *
+ * @return True on success, false otherwise.
+ */
 bool CharacterString_Value_Name_Set(uint32_t object_instance, char *new_name)
 {
     unsigned index = 0; /* offset from instance lookup */
@@ -260,16 +357,20 @@ bool CharacterString_Value_Name_Set(uint32_t object_instance, char *new_name)
                 }
             }
         } else {
-            for (i = 0; i < sizeof(Object_Name[index]); i++) {
+            memset(&Object_Name[index][0], 0, sizeof(Object_Name[index]));
-                Object_Name[index][i] = 0;
-            }
         }
     }
 
     return status;
 }
 
-/* return apdu len, or BACNET_STATUS_ERROR on error */
+/**
+ * Return the requested property of the character string value.
+ *
+ * @param rpdata  Property requested, see for BACNET_READ_PROPERTY_DATA details.
+ *
+ * @return apdu len, or BACNET_STATUS_ERROR on error
+ */
 int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
 {
     int apdu_len = 0; /* return value */
@@ -279,11 +380,25 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     bool state = false;
     uint8_t *apdu = NULL;
 
-    if ((rpdata == NULL) || (rpdata->application_data == NULL) ||
+    /* Valid data? */
+    if (rpdata == NULL) {
+        return 0;
+    }
+    if ((rpdata->application_data == NULL) || \
         (rpdata->application_data_len == 0)) {
         return 0;
     }
+
+    /* Valid object? */
+    object_index = CharacterString_Value_Instance_To_Index(rpdata->object_instance);
+    if (object_index >= MAX_CHARACTERSTRING_VALUES) {
+        rpdata->error_class = ERROR_CLASS_OBJECT;
+        rpdata->error_code = ERROR_CODE_UNKNOWN_OBJECT;
+        return BACNET_STATUS_ERROR;
+    }
+
     apdu = rpdata->application_data;
+
     switch (rpdata->object_property) {
         case PROP_OBJECT_IDENTIFIER:
             apdu_len = encode_application_object_id(&apdu[0],
@@ -292,16 +407,18 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
             /* note: Name and Description don't have to be the same.
                You could make Description writable and different */
         case PROP_OBJECT_NAME:
-            CharacterString_Value_Object_Name(
+            if (CharacterString_Value_Object_Name(
-                rpdata->object_instance, &char_string);
+                rpdata->object_instance, &char_string)) {
                 apdu_len =
                     encode_application_character_string(&apdu[0], &char_string);
+            }
             break;
         case PROP_DESCRIPTION:
-            characterstring_init_ansi(&char_string,
+            if (characterstring_init_ansi(&char_string,
-                CharacterString_Value_Description(rpdata->object_instance));
+                CharacterString_Value_Description(rpdata->object_instance))) {
                 apdu_len =
                     encode_application_character_string(&apdu[0], &char_string);
+            }
             break;
         case PROP_OBJECT_TYPE:
             apdu_len = encode_application_enumerated(
@@ -334,8 +451,6 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
                 encode_application_enumerated(&apdu[0], EVENT_STATE_NORMAL);
             break;
         case PROP_OUT_OF_SERVICE:
-            object_index = CharacterString_Value_Instance_To_Index(
-                rpdata->object_instance);
             state = Out_Of_Service[object_index];
             apdu_len = encode_application_boolean(&apdu[0], state);
             break;
@@ -356,14 +471,29 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     return apdu_len;
 }
 
-/* returns true if successful */
+/**
+ * Set the requested property of the character string value.
+ *
+ * @param wp_data  Property requested, see for BACNET_WRITE_PROPERTY_DATA details.
+ *
+ * @return true if successful
+ */
 bool CharacterString_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
 {
     bool status = false; /* return value */
     int len = 0;
+    unsigned object_index = 0;
     BACNET_APPLICATION_DATA_VALUE value;
 
-    /* decode the some of the request */
+    if (wp_data == NULL) {
+        return false;
+    }
+    if ((wp_data->application_data == NULL) || \
+        (wp_data->application_data_len == 0)) {
+        return false;
+    }
+
+    /* Decode the some of the request. */
     len = bacapp_decode_application_data(
         wp_data->application_data, wp_data->application_data_len, &value);
     /* FIXME: len < application_data_len: more data? */
@@ -373,6 +503,15 @@ bool CharacterString_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
         wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE;
         return false;
     }
+
+    /* Valid object? */
+    object_index = CharacterString_Value_Instance_To_Index(wp_data->object_instance);
+    if (object_index >= MAX_CHARACTERSTRING_VALUES) {
+        wp_data->error_class = ERROR_CLASS_OBJECT;
+        wp_data->error_code = ERROR_CODE_UNKNOWN_OBJECT;
+        return false;
+    }
+
     switch (wp_data->object_property) {
         case PROP_PRESENT_VALUE:
             status = WPValidateArgType(&value,

src/bacnet/basic/service/h_apdu.c

@@ -94,6 +94,12 @@ static BACNET_SERVICES_SUPPORTED
 /* If they are not set, they are handled by a reject message */
 static confirmed_function Confirmed_Function[MAX_BACNET_CONFIRMED_SERVICE];
 
+/**
+ * @brief Set a handler function for the given confirmed service.
+ *
+ * @param service_choice Service, see SERVICE_CONFIRMED_X enumeration.
+ * @param pFunction  Pointer to the function, being in charge of the service.
+ */
 void apdu_set_confirmed_handler(
     BACNET_CONFIRMED_SERVICE service_choice, confirmed_function pFunction)
 {
@@ -105,6 +111,12 @@ void apdu_set_confirmed_handler(
 /* Allow the APDU handler to automatically reject */
 static confirmed_function Unrecognized_Service_Handler;
 
+/**
+ * @brief Set a handler function called for an unsupported service.
+ *
+ * @param pFunction  Pointer to the function, being in charge,
+ *                   if a unsupported service has been requested.
+ */
 void apdu_set_unrecognized_service_handler_handler(confirmed_function pFunction)
 {
     Unrecognized_Service_Handler = pFunction;
@@ -115,6 +127,12 @@ void apdu_set_unrecognized_service_handler_handler(confirmed_function pFunction)
 static unconfirmed_function
     Unconfirmed_Function[MAX_BACNET_UNCONFIRMED_SERVICE];
 
+/**
+ * @brief Set a handler function for the given unconfirmed service.
+ *
+ * @param service_choice Service, see SERVICE_UNCONFIRMED_X enumeration.
+ * @param pFunction  Pointer to the function, being in charge of the service.
+ */
 void apdu_set_unconfirmed_handler(
     BACNET_UNCONFIRMED_SERVICE service_choice, unconfirmed_function pFunction)
 {
@@ -123,6 +141,13 @@ void apdu_set_unconfirmed_handler(
     }
 }
 
+/**
+ * @brief Checks if the given service is supported or not.
+ *
+ * @param service_supported  Service in question
+ *
+ * @return true/false
+ */
 bool apdu_service_supported(BACNET_SERVICES_SUPPORTED service_supported)
 {
     int i = 0;
@@ -278,6 +303,12 @@ void apdu_set_confirmed_ack_handler(
 
 static error_function Error_Function[MAX_BACNET_CONFIRMED_SERVICE];
 
+/**
+ * @brief Set a error handler function for the given confirmed service.
+ *
+ * @param service_choice  Service, see SERVICE_CONFIRMED_X enumeration.
+ * @param pFunction  Pointer to the function, being in charge of the error handling.
+ */
 void apdu_set_error_handler(
     BACNET_CONFIRMED_SERVICE service_choice, error_function pFunction)
 {
@@ -295,11 +326,35 @@ void apdu_set_abort_handler(abort_function pFunction)
 
 static reject_function Reject_Function;
 
+/**
+ * @brief Set a handler function called for a rejected service.
+ *
+ * @param pFunction  Pointer to the function, being in charge,
+ *                   if a service request will be rejected.
+ */
 void apdu_set_reject_handler(reject_function pFunction)
 {
     Reject_Function = pFunction;
 }
 
+/**
+ * @brief Decode the given confirmed service request from the received data.
+ *
+ * @param apdu  Received data buffer.
+ * @param apdu_len  Count of valid bytes in the receive buffer.
+ * @param service_data  Pointer of the structure being filled with the service
+ *                      data.
+ * @param service_choice  Pointer to a variable taking the service requested,
+ *                        see SERVICE_CONFIRMED_X
+ * @param service_request  Pointer to a pointer that will take the pointer
+ *                         were the service request data can be found in
+ *                         the receive buffer.
+ * @param service_request_len  Pointer to a variable that takes the length
+ *                             of the service request data.
+ *
+ * @return The length of the service data  and reflects the offset, where
+ *         we are in PDU after the service had been decoded.
+ */
 uint16_t apdu_decode_confirmed_service_request(uint8_t *apdu, /* APDU data */
     uint16_t apdu_len,
     BACNET_CONFIRMED_SERVICE_DATA *service_data,
@@ -382,13 +437,19 @@ static bool apdu_confirmed_dcc_disabled(uint8_t service_choice)
     return status;
 }
 
-/* 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
-/* If the request is valid and the 'Enable/Disable' parameter is
+ * shall be processed and no messages shall be initiated.
-   DISABLE_INITIATION, the responding BACnet-user shall
+ * If the request is valid and the 'Enable/Disable' parameter is
-   discontinue the initiation of messages except for I-Am
+ * DISABLE_INITIATION, the responding BACnet-user shall
-   requests issued in accordance with the Who-Is service procedure.*/
+ * discontinue the initiation of messages except for I-Am
+ * requests issued in accordance with the Who-Is service procedure.
+ *
+ * @param service_choice  Service, like SERVICE_UNCONFIRMED_WHO_IS
+ *
+ * @return true, if being disabled.
+ */
 static bool apdu_unconfirmed_dcc_disabled(uint8_t service_choice)
 {
     bool status = false;
@@ -462,6 +523,7 @@ void apdu_handler(BACNET_ADDRESS *src,
                 }
                 break;
             case PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST:
+                if (apdu_len >= 2) {
                     service_choice = apdu[1];
                     service_request = &apdu[2];
                     service_request_len = apdu_len - 2;
@@ -479,8 +541,10 @@ void apdu_handler(BACNET_ADDRESS *src,
                                 service_request, service_request_len, src);
                         }
                     }
+                }
                 break;
             case PDU_TYPE_SIMPLE_ACK:
+                if (apdu_len >= 3) {
                     invoke_id = apdu[1];
                     service_choice = apdu[2];
                     switch (service_choice) {
@@ -514,8 +578,10 @@ void apdu_handler(BACNET_ADDRESS *src,
                         default:
                             break;
                     }
+                }
                 break;
             case PDU_TYPE_COMPLEX_ACK:
+                if (apdu_len >= 3) {
                     service_ack_data.segmented_message =
                         (apdu[0] & BIT(3)) ? true : false;
                     service_ack_data.more_follows =
@@ -558,6 +624,7 @@ void apdu_handler(BACNET_ADDRESS *src,
                         default:
                             break;
                     }
+                }
                 break;
             case PDU_TYPE_SEGMENT_ACK:
                 /* FIXME: what about a denial of service attack here?
@@ -565,6 +632,7 @@ void apdu_handler(BACNET_ADDRESS *src,
                 tsm_free_invoke_id(invoke_id);
                 break;
             case PDU_TYPE_ERROR:
+                if (apdu_len >= 3) {
                     invoke_id = apdu[1];
                     service_choice = apdu[2];
                     len = 3;
@@ -583,12 +651,20 @@ void apdu_handler(BACNET_ADDRESS *src,
                                       octet */
                         }
                     }
+
+                    if (len < apdu_len) {
                         len += decode_tag_number_and_value(
                             &apdu[len], &tag_number, &len_value);
+
+                        if (len < apdu_len) {
                             /* FIXME: we could validate that the tag is enumerated... */
                             len += decode_enumerated(&apdu[len], len_value, &error_class);
+
+                            if (len < apdu_len) {
                                 len += decode_tag_number_and_value(
                                     &apdu[len], &tag_number, &len_value);
+
+                                if (len < apdu_len) {
                                     /* FIXME: we could validate that the tag is enumerated... */
                                     len += decode_enumerated(&apdu[len], len_value, &error_code);
 
@@ -600,6 +676,11 @@ void apdu_handler(BACNET_ADDRESS *src,
                                                       octet */
                                         }
                                     }
+                                }
+                            }
+                        }
+                    }
+
                     if (service_choice < MAX_BACNET_CONFIRMED_SERVICE) {
                         if (Error_Function[service_choice]) {
                             Error_Function[service_choice](src, invoke_id,
@@ -608,16 +689,20 @@ void apdu_handler(BACNET_ADDRESS *src,
                         }
                     }
                     tsm_free_invoke_id(invoke_id);
+                }
                 break;
             case PDU_TYPE_REJECT:
+                if (apdu_len >= 3) {
                     invoke_id = apdu[1];
                     reason = apdu[2];
                     if (Reject_Function) {
                         Reject_Function(src, invoke_id, reason);
                     }
                     tsm_free_invoke_id(invoke_id);
+                }
                 break;
             case PDU_TYPE_ABORT:
+                if (apdu_len >= 3) {
                     server = apdu[0] & 0x01;
                     invoke_id = apdu[1];
                     reason = apdu[2];
@@ -625,6 +710,7 @@ void apdu_handler(BACNET_ADDRESS *src,
                         Abort_Function(src, invoke_id, reason, server);
                     }
                     tsm_free_invoke_id(invoke_id);
+                }
                 break;
             default:
                 break;

src/bacnet/basic/service/h_cov.c

@@ -802,8 +802,7 @@ void handler_cov_subscribe(uint8_t *service_request,
         fprintf(stderr, "SubscribeCOV: Segmented message.  Sending Abort!\n");
 #endif
         error = true;
-        goto COV_ABORT;
+    } else {
-    }
         len = cov_subscribe_decode_service_request(
             service_request, service_len, &cov_data);
 #if PRINT_ENABLED
@@ -812,8 +811,7 @@ void handler_cov_subscribe(uint8_t *service_request,
 #endif
         if (len < 0) {
             error = true;
-        goto COV_ABORT;
+        } else {
-    }
             cov_data.error_class = ERROR_CLASS_OBJECT;
             cov_data.error_code = ERROR_CODE_UNKNOWN_OBJECT;
             success = cov_subscribe(
@@ -831,7 +829,10 @@ void handler_cov_subscribe(uint8_t *service_request,
                 fprintf(stderr, "SubscribeCOV: Sending Error!\n");
 #endif
             }
-COV_ABORT:
+        }
+    }
+
+    /* Error? */
     if (error) {
         if (len == BACNET_STATUS_ABORT) {
             apdu_len = abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len],

src/bacnet/basic/service/h_wp.c

@@ -68,6 +68,7 @@ void handler_write_property(uint8_t *service_request,
 {
     BACNET_WRITE_PROPERTY_DATA wp_data;
     int len = 0;
+    bool bcontinue = true;
     int pdu_len = 0;
     BACNET_NPDU_DATA npdu_data;
     int bytes_sent = 0;
@@ -88,8 +89,11 @@ void handler_write_property(uint8_t *service_request,
 #if PRINT_ENABLED
         fprintf(stderr, "WP: Segmented message.  Sending Abort!\n");
 #endif
-        goto WP_ABORT;
+        bcontinue = false;
-    } /* decode the service request only */
+    }
+
+    if (bcontinue) {
+        /* decode the service request only */
         len = wp_decode_service_request(service_request, service_len, &wp_data);
 #if PRINT_ENABLED
         if (len > 0)
@@ -109,8 +113,10 @@ void handler_write_property(uint8_t *service_request,
 #if PRINT_ENABLED
             fprintf(stderr, "WP: Bad Encoding. Sending Abort!\n");
 #endif
-        goto WP_ABORT;
+            bcontinue = false;
         }
+
+        if (bcontinue) {
             if (Device_Write_Property(&wp_data)) {
                 len = encode_simple_ack(&Handler_Transmit_Buffer[pdu_len],
                     service_data->invoke_id, SERVICE_CONFIRMED_WRITE_PROPERTY);
@@ -125,9 +131,12 @@ void handler_write_property(uint8_t *service_request,
                 fprintf(stderr, "WP: Sending Error!\n");
 #endif
             }
-WP_ABORT:
+        }
+    }
+
+    /* 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
@@ -138,12 +147,20 @@ WP_ABORT:
     return;
 }
 
-/** Perform basic validation of Write Property argument based on
+/**
+ * @brief Perform basic validation of Write Property argument based on
  * the assumption that it is a string. Check for correct data type,
  * correct encoding (fixed here as ANSI X34),correct length, and
  * finally if it is allowed to be empty.
+ *
+ * @param pValue  Pointer to the application data value representing the string.
+ * @param iMaxLen  Maximum string length allowed.
+ * @param bEmptyAllowed  true, if empty strings shall be allowed.
+ * @param pErrorClass  Pointer to a variable taking the error class.
+ * @param pErrorCode  Pointer to a variable taking the error code.
+ *
+ * @return True on success, false otherwise.
  */
-
 bool WPValidateString(BACNET_APPLICATION_DATA_VALUE *pValue,
     int iMaxLen,
     bool bEmptyAllowed,

src/bacnet/iam.c

@@ -41,7 +41,17 @@
 
 /** @file iam.c  Encode/Decode I-Am service */
 
-/* encode I-Am service */
+/**
+ * @brief Encode the I-Am service.
+ *
+ * @param apdu  Transmit buffer
+ * @param device_id  Device Id
+ * @param max_apdu  Transmit buffer size.
+ * @param segmentation  True, if segmentation shall be featured.
+ * @param vendor_id  Vendor Id
+ *
+ * @return Total length of the apdu, zero otherwise.
+ */
 int iam_encode_apdu(uint8_t *apdu,
     uint32_t device_id,
     unsigned max_apdu,
@@ -70,6 +80,17 @@ int iam_encode_apdu(uint8_t *apdu,
     return apdu_len;
 }
 
+/**
+ * @brief Decode the I-Am service.
+ *
+ * @param apdu  Receive buffer
+ * @param pDevice_id  Pointer to the variable that shal ltake the device Id.
+ * @param pMax_apdu  Pointer to a variable that shall take the decoded length.
+ * @param pSegmentation  Pointer to a variable taking if segmentation is used.
+ * @param pVendor_id  Pointer to a variable taking the vendor id.
+ *
+ * @return Total length of the apdu, zero otherwise.
+ */
 int iam_decode_service_request(uint8_t *apdu,
     uint32_t *pDevice_id,
     unsigned *pMax_apdu,