Improved AtomicReadFile and AtomicWriteFile error detection. Eliminated redundancy in the octetstring_init function.

bacnet-stack/demo/readfile/main.c

@@ -59,9 +59,11 @@ static uint32_t Target_File_Object_Instance = BACNET_MAX_INSTANCE;
 static uint32_t Target_Device_Object_Instance = BACNET_MAX_INSTANCE;
 static BACNET_ADDRESS Target_Address;
 static char *Local_File_Name = NULL;
+static int Target_File_Start_Position;
+static unsigned int Target_File_Requested_Octet_Count;
 static bool End_Of_File_Detected = false;
 static bool Error_Detected = false;
-static uint8_t Current_Invoke_ID = 0;
+static uint8_t Request_Invoke_ID = 0;
 
 static void Atomic_Read_File_Error_Handler(
     BACNET_ADDRESS * src,
@@ -69,13 +71,13 @@ static void Atomic_Read_File_Error_Handler(
     BACNET_ERROR_CLASS error_class,
     BACNET_ERROR_CODE error_code)
 {
-    /* FIXME: verify src and invoke id */
+    if (address_match(&Target_Address, src) &&
-    (void) src;
+        (invoke_id == Request_Invoke_ID)) {
-    (void) invoke_id;
+        printf("BACnet Error: %s: %s\n",
-    printf("\nBACnet Error!\n");
+            bactext_error_class_name((int) error_class),
-    printf("Error Class: %s\n", bactext_error_class_name(error_class));
+            bactext_error_code_name((int) error_code));
-    printf("Error Code: %s\n", bactext_error_code_name(error_code));
+        Error_Detected = true;
-    Error_Detected = true;
+    }
 }
 
 void MyAbortHandler(
@@ -84,13 +86,13 @@ void MyAbortHandler(
     uint8_t abort_reason,
     bool server)
 {
-    /* FIXME: verify src and invoke id */
-    (void) src;
-    (void) invoke_id;
     (void) server;
-    printf("\nBACnet Abort!\n");
+    if (address_match(&Target_Address, src) &&
-    printf("Abort Reason: %s\n", bactext_abort_reason_name(abort_reason));
+        (invoke_id == Request_Invoke_ID)) {
-    Error_Detected = true;
+        printf("BACnet Abort: %s\n",
+            bactext_abort_reason_name((int) abort_reason));
+        Error_Detected = true;
+    }
 }
 
 void MyRejectHandler(
@@ -98,12 +100,12 @@ void MyRejectHandler(
     uint8_t invoke_id,
     uint8_t reject_reason)
 {
-    /* FIXME: verify src and invoke id */
+    if (address_match(&Target_Address, src) &&
-    (void) src;
+        (invoke_id == Request_Invoke_ID)) {
-    (void) invoke_id;
+        printf("BACnet Reject: %s\n",
-    printf("\nBACnet Reject!\n");
+            bactext_reject_reason_name((int) reject_reason));
-    printf("Reject Reason: %s\n", bactext_reject_reason_name(reject_reason));
+        Error_Detected = true;
-    Error_Detected = true;
+    }
 }
 
 static void AtomicReadFileAckHandler(
@@ -113,44 +115,59 @@ static void AtomicReadFileAckHandler(
     BACNET_CONFIRMED_SERVICE_ACK_DATA * service_data)
 {
     int len = 0;
+    int result = 0;
     BACNET_ATOMIC_READ_FILE_DATA data;
     FILE *pFile = NULL; /* stream pointer */
     size_t octets_written = 0;
 
-    (void) src; /* FIXME: validate the source address matches */
+    if (address_match(&Target_Address, src) &&
-    len = arf_ack_decode_service_request(service_request, service_len, &data);
+        (service_data->invoke_id == Request_Invoke_ID)) {
-    if (len > 0) {
+        len = arf_ack_decode_service_request(service_request, service_len, &data);
-        /* validate the parameters before storing data */
+        if ((len > 0) && (data.access == FILE_STREAM_ACCESS)) {
-        if ((data.access == FILE_STREAM_ACCESS) &&
+            if (data.type.stream.fileStartPosition == 0) {
-            (service_data->invoke_id == Current_Invoke_ID)) {
-            if (data.type.stream.fileStartPosition == 0)
                 pFile = fopen(Local_File_Name, "wb");
-            else
+            } else {
                 pFile = fopen(Local_File_Name, "rb+");
+            }
             if (pFile) {
-                /* is there anything to do with this? data.stream.requestedOctetCount */
+                result = fseek(pFile, data.type.stream.fileStartPosition,
-                (void) fseek(pFile, data.type.stream.fileStartPosition,
                     SEEK_SET);
-                octets_written = fwrite(octetstring_value(&data.fileData[0]), 1,        /* unit to write in bytes - in our case, an octet is one byte */
+                if (result == 0) {
-                    octetstring_length(&data.fileData[0]), pFile);
+                    /* unit to write in bytes -
-                if (octets_written != octetstring_length(&data.fileData[0])) {
+                       in our case, an octet is one byte */
-                    fprintf(stderr, "Unable to write data to file \"%s\".\n",
+                    octets_written = fwrite(
-                        Local_File_Name);
+                        octetstring_value(&data.fileData[0]), 1,
-                } else if (octets_written == 0) {
+                        octetstring_length(&data.fileData[0]), pFile);
-                    fprintf(stderr, "Received 0 byte octet string!.\n");
+                    if (octets_written !=
+                        octetstring_length(&data.fileData[0])) {
+                        fprintf(stderr,
+                            "Unable to write data to file \"%s\".\n",
+                            Local_File_Name);
+                    } else if (octets_written == 0) {
+                        fprintf(stderr, "Received 0 byte octet string!.\n");
+                    } else {
+                        Target_File_Start_Position =
+                            data.type.stream.fileStartPosition +
+                            octets_written;
+                        printf("\r%d bytes", (int)Target_File_Start_Position);
+                    }
+                    fflush(pFile);
                 } else {
-                    printf("\r%d bytes",
+                    fprintf(stderr, "Unable to seek to %d!\n",
-                        (int)(data.type.stream.fileStartPosition +
+                        data.type.stream.fileStartPosition);
-                        octets_written));
                 }
-                fflush(pFile);
                 fclose(pFile);
             }
             if (data.endOfFile) {
                 End_Of_File_Detected = true;
                 printf("\n");
             }
+        } else {
+            fprintf(stderr, "Decode error! %d bytes decoded.\n", len);
         }
+    } else {
+        fprintf(stderr, "Address & Invoke ID mismatch! Invoke ID=%d\n",
+            Request_Invoke_ID);
     }
 }
 
@@ -249,8 +266,6 @@ int main(
     time_t last_seconds = 0;
     time_t current_seconds = 0;
     time_t timeout_seconds = 0;
-    int fileStartPosition = 0;
-    unsigned requestedOctetCount = 0;
     uint8_t invoke_id = 0;
     bool found = false;
     uint16_t my_max_apdu = 0;
@@ -343,13 +358,13 @@ int main(
             }
             /* Typical sizes are 50, 128, 206, 480, 1024, and 1476 octets */
             if (my_max_apdu <= 50) {
-                requestedOctetCount = my_max_apdu - 20;
+                Target_File_Requested_Octet_Count = my_max_apdu - 20;
             } else if (my_max_apdu <= 480) {
-                requestedOctetCount = my_max_apdu - 32;
+                Target_File_Requested_Octet_Count = my_max_apdu - 32;
             } else if (my_max_apdu <= 1476) {
-                requestedOctetCount = my_max_apdu - 64;
+                Target_File_Requested_Octet_Count = my_max_apdu - 64;
             } else {
-                requestedOctetCount = my_max_apdu / 2;
+                Target_File_Requested_Octet_Count = my_max_apdu / 2;
             }
             /* has the previous invoke id expired or returned?
                note: invoke ID = 0 is invalid, so it will be idle */
@@ -357,16 +372,14 @@ int main(
                 if (End_Of_File_Detected || Error_Detected) {
                     break;
                 }
-                if (invoke_id != 0) {
+                /* the ACK will increment the start position if OK */
-                    fileStartPosition += requestedOctetCount;
-                }
                 /* we'll read the file in chunks
                    less than max_apdu to keep unsegmented */
                 invoke_id =
                     Send_Atomic_Read_File_Stream(Target_Device_Object_Instance,
-                    Target_File_Object_Instance, fileStartPosition,
+                    Target_File_Object_Instance, Target_File_Start_Position,
-                    requestedOctetCount);
+                    Target_File_Requested_Octet_Count);
-                Current_Invoke_ID = invoke_id;
+                Request_Invoke_ID = invoke_id;
             } else if (tsm_invoke_id_failed(invoke_id)) {
                 fprintf(stderr, "\rError: TSM Timeout!\n");
                 tsm_free_invoke_id(invoke_id);

bacnet-stack/src/arf.c

@@ -256,6 +256,7 @@ int arf_ack_decode_service_request(
 {
     int len = 0;
     int tag_len = 0;
+    int decoded_len = 0;
     uint8_t tag_number = 0;
     uint32_t len_value_type = 0;
     uint32_t i = 0;
@@ -265,8 +266,9 @@ int arf_ack_decode_service_request(
         len =
             decode_tag_number_and_value(&apdu[0], &tag_number,
             &len_value_type);
-        if (tag_number != BACNET_APPLICATION_TAG_BOOLEAN)
+        if (tag_number != BACNET_APPLICATION_TAG_BOOLEAN) {
             return -1;
+        }
         data->endOfFile = decode_boolean(len_value_type);
         if (decode_is_opening_tag_number(&apdu[len], 0)) {
             data->access = FILE_STREAM_ACCESS;
@@ -277,8 +279,9 @@ int arf_ack_decode_service_request(
                 decode_tag_number_and_value(&apdu[len], &tag_number,
                 &len_value_type);
             len += tag_len;
-            if (tag_number != BACNET_APPLICATION_TAG_SIGNED_INT)
+            if (tag_number != BACNET_APPLICATION_TAG_SIGNED_INT) {
                 return -1;
+            }
             len +=
                 decode_signed(&apdu[len], len_value_type,
                 &data->type.stream.fileStartPosition);
@@ -287,13 +290,19 @@ int arf_ack_decode_service_request(
                 decode_tag_number_and_value(&apdu[len], &tag_number,
                 &len_value_type);
             len += tag_len;
-            if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING)
+            if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING) {
                 return -1;
-            len +=
+            }
+            decoded_len =
                 decode_octet_string(&apdu[len], len_value_type,
                 &data->fileData[0]);
-            if (!decode_is_closing_tag_number(&apdu[len], 0))
+            if (decoded_len != len_value_type) {
                 return -1;
+            }
+            len += decoded_len;
+            if (!decode_is_closing_tag_number(&apdu[len], 0)) {
+                return -1;
+            }
             /* a tag number is not extended so only one octet */
             len++;
         } else if (decode_is_opening_tag_number(&apdu[len], 1)) {
@@ -305,8 +314,9 @@ int arf_ack_decode_service_request(
                 decode_tag_number_and_value(&apdu[len], &tag_number,
                 &len_value_type);
             len += tag_len;
-            if (tag_number != BACNET_APPLICATION_TAG_SIGNED_INT)
+            if (tag_number != BACNET_APPLICATION_TAG_SIGNED_INT) {
                 return -1;
+            }
             len +=
                 decode_signed(&apdu[len], len_value_type,
                 &data->type.record.fileStartRecord);
@@ -315,8 +325,9 @@ int arf_ack_decode_service_request(
                 decode_tag_number_and_value(&apdu[len], &tag_number,
                 &len_value_type);
             len += tag_len;
-            if (tag_number != BACNET_APPLICATION_TAG_UNSIGNED_INT)
+            if (tag_number != BACNET_APPLICATION_TAG_UNSIGNED_INT) {
                 return -1;
+            }
             len +=
                 decode_unsigned(&apdu[len], len_value_type,
                 &data->type.record.RecordCount);
@@ -326,18 +337,25 @@ int arf_ack_decode_service_request(
                     decode_tag_number_and_value(&apdu[len], &tag_number,
                     &len_value_type);
                 len += tag_len;
-                if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING)
+                if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING) {
                     return -1;
-                len +=
+                }
-                    decode_octet_string(&apdu[len], len_value_type,
+                decoded_len =
-                    &data->fileData[i]);
+                        decode_octet_string(&apdu[len], len_value_type,
+                        &data->fileData[i]);
+                if (decoded_len != len_value_type) {
+                    return -1;
+                }
+                len += decoded_len;
             }
-            if (!decode_is_closing_tag_number(&apdu[len], 1))
+            if (!decode_is_closing_tag_number(&apdu[len], 1)) {
                 return -1;
+            }
             /* a tag number is not extended so only one octet */
             len++;
-        } else
+        } else {
             return -1;
+        }
     }
 
     return len;

bacnet-stack/src/awf.c

@@ -99,6 +99,7 @@ int awf_decode_service_request(
 {
     int len = 0;
     int tag_len = 0;
+    int decoded_len = 0;
     uint8_t tag_number = 0;
     uint32_t len_value_type = 0;
     int32_t signed_value = 0;
@@ -135,9 +136,13 @@ int awf_decode_service_request(
             len += tag_len;
             if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING)
                 return -1;
-            len +=
+            decoded_len =
                 decode_octet_string(&apdu[len], len_value_type,
                 &data->fileData[0]);
+            if (decoded_len != len_value_type) {
+                return -1;
+            }
+            len += decoded_len;
             if (!decode_is_closing_tag_number(&apdu[len], 0))
                 return -1;
             /* a tag number is not extended so only one octet */
@@ -173,9 +178,13 @@ int awf_decode_service_request(
                 len += tag_len;
                 if (tag_number != BACNET_APPLICATION_TAG_OCTET_STRING)
                     return -1;
-                len +=
+                decoded_len =
                     decode_octet_string(&apdu[len], len_value_type,
                     &data->fileData[i]);
+                if (decoded_len != len_value_type) {
+                    return -1;
+                }
+                len += decoded_len;
             }
             if (!decode_is_closing_tag_number(&apdu[len], 1))
                 return -1;

bacnet-stack/src/bacstr.c

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