Refactor Who-Am-I and You-Are JSON handlers to eliminate dynamic memory allocation for model and serial number strings, improving memory management and simplifying code. (#1089)

src/bacnet/basic/service/h_whoami.c

@@ -7,7 +7,6 @@
  */
 #include <stddef.h>
 #include <stdint.h>
-#include <stdlib.h>
 #include <stdio.h>
 /* BACnet Stack defines - first */
 #include "bacnet/bacdef.h"
@@ -31,8 +30,7 @@ void handler_who_am_i_json_print(
     uint16_t vendor_id = 0;
     BACNET_CHARACTER_STRING model_name = { 0 };
     BACNET_CHARACTER_STRING serial_number = { 0 };
-    char *model_name_string = NULL;
+    char name[MAX_CHARACTER_STRING_BYTES + 1] = { 0 };
-    char *serial_number_string = NULL;
 
     (void)src;
     len = who_am_i_request_decode(
@@ -40,31 +38,11 @@ void handler_who_am_i_json_print(
     if (len > 0) {
         debug_printf_stdout("{\n\"Who-Am-I-Request\": {\n");
         debug_printf_stdout(" \"vendor-id\" : %u,\n", (unsigned)vendor_id);
-        len = bacapp_snprintf_character_string(NULL, 0, &model_name);
+        bacapp_snprintf_character_string(name, sizeof(name), &model_name);
-        if (len > 0) {
+        debug_printf_stdout(" \"model-name\" : %s,\n", name);
-            model_name_string = calloc(sizeof(char), len + 1);
+        bacapp_snprintf_character_string(name, sizeof(name), &serial_number);
-            if (model_name_string) {
+        debug_printf_stdout(" \"serial-number\" : %s", name);
-                bacapp_snprintf_character_string(
-                    model_name_string, len + 1, &model_name);
-            }
-        }
-        debug_printf_stdout(
-            " \"model-name\" : %s,\n",
-            model_name_string ? model_name_string : "");
-        len = bacapp_snprintf_character_string(NULL, 0, &serial_number);
-        if (len > 0) {
-            serial_number_string = calloc(sizeof(char), len + 1);
-            if (serial_number_string) {
-                bacapp_snprintf_character_string(
-                    serial_number_string, len + 1, &serial_number);
-            }
-        }
-        debug_printf_stdout(
-            " \"serial-number\" : %s",
-            serial_number_string ? serial_number_string : "");
         debug_printf_stdout("\n }\n}\n");
-        free(model_name_string);
-        free(serial_number_string);
     }
 
     return;

src/bacnet/basic/service/h_youare.c

@@ -7,7 +7,6 @@
  */
 #include <stddef.h>
 #include <stdint.h>
-#include <stdlib.h>
 #include <stdio.h>
 /* BACnet Stack defines - first */
 #include "bacnet/bacdef.h"
@@ -34,9 +33,7 @@ void handler_you_are_json_print(
     BACNET_CHARACTER_STRING model_name = { 0 };
     BACNET_CHARACTER_STRING serial_number = { 0 };
     BACNET_OCTET_STRING mac_address = { 0 };
-    char *model_name_string = NULL;
+    char name[MAX_CHARACTER_STRING_BYTES + 1] = { 0 };
-    char *serial_number_string = NULL;
-    char *mac_address_string = NULL;
 
     (void)src;
     len = you_are_request_decode(
@@ -45,28 +42,10 @@ void handler_you_are_json_print(
     if (len > 0) {
         debug_printf_stdout("{\n\"You-Are-Request\": {\n");
         debug_printf_stdout(" \"vendor-id\" : %u,\n", (unsigned)vendor_id);
-        len = bacapp_snprintf_character_string(NULL, 0, &model_name);
+        bacapp_snprintf_character_string(name, sizeof(name), &model_name);
-        if (len > 0) {
+        debug_printf_stdout(" \"model-name\" : %s,\n", name);
-            model_name_string = calloc(sizeof(char), len + 1);
+        bacapp_snprintf_character_string(name, sizeof(name), &serial_number);
-            if (model_name_string) {
+        debug_printf_stdout(" \"serial-number\" : %s", name);
-                bacapp_snprintf_character_string(
-                    model_name_string, len + 1, &model_name);
-            }
-        }
-        debug_printf_stdout(
-            " \"model-name\" : %s,\n",
-            model_name_string ? model_name_string : "");
-        len = bacapp_snprintf_character_string(NULL, 0, &serial_number);
-        if (len > 0) {
-            serial_number_string = calloc(sizeof(char), len + 1);
-            if (serial_number_string) {
-                bacapp_snprintf_character_string(
-                    serial_number_string, len + 1, &serial_number);
-            }
-        }
-        debug_printf_stdout(
-            " \"serial-number\" : %s",
-            serial_number_string ? serial_number_string : "");
         if (device_id <= BACNET_MAX_INSTANCE) {
             debug_printf_stdout(",\n");
             debug_printf_stdout(
@@ -74,22 +53,10 @@ void handler_you_are_json_print(
         }
         if (mac_address.length > 0) {
             debug_printf_stdout(",\n");
-            len = bacapp_snprintf_octet_string(NULL, 0, &mac_address);
+            bacapp_snprintf_octet_string(name, sizeof(name), &mac_address);
-            if (len > 0) {
+            debug_printf_stdout(" \"device-mac-address\" : \"%s\"", name);
-                mac_address_string = calloc(sizeof(char), len + 1);
-                if (mac_address_string) {
-                    bacapp_snprintf_octet_string(
-                        mac_address_string, len + 1, &mac_address);
-                }
-            }
-            debug_printf_stdout(
-                " \"device-mac-address\" : \"%s\"",
-                mac_address_string ? mac_address_string : "");
         }
         debug_printf_stdout("\n }\n}\n");
-        free(model_name_string);
-        free(serial_number_string);
-        free(mac_address_string);
     }
 
     return;

test/bacnet/bacapp/src/main.c

@@ -1297,36 +1297,134 @@ static void test_bacapp_data(void)
 }
 
 /**
- * @brief Test
+ * @brief Helper function to test bacapp_snprintf_value()
+ * @param tag_number [in] The BACnet application tag to test
+ * @param argv [in] The string to parse into a BACNET_APPLICATION_DATA_VALUE
+ * @param expected [in] The expected string output from bacapp_snprintf_value()
  */
-#if defined(CONFIG_ZTEST_NEW_API)
+void test_bacapp_snprintf(
-ZTEST(bacapp_tests, test_bacapp_sprintf_data)
+    BACNET_APPLICATION_TAG tag_number, char *argv, const char *expected)
-#else
-static void test_bacapp_sprintf_data(void)
-#endif
 {
     BACNET_APPLICATION_DATA_VALUE value = { 0 };
     BACNET_OBJECT_PROPERTY_VALUE object_value = { 0 };
     bool status = false;
-    int str_len = 0;
+    int str_len = 0, len, test_len;
 
+    /* object property value is only needed to snprintf ENUMERATED type */
     object_value.object_type = OBJECT_DEVICE;
     object_value.object_instance = 0;
-    object_value.object_property = PROP_DAYLIGHT_SAVINGS_STATUS;
+    object_value.object_property = PROP_SEGMENTATION_SUPPORTED;
     object_value.array_index = BACNET_ARRAY_ALL;
     object_value.value = &value;
 
-    status = bacapp_parse_application_data(
+    status = bacapp_parse_application_data(tag_number, argv, &value);
-        BACNET_APPLICATION_TAG_NULL, NULL, &value);
     zassert_true(status, NULL);
     str_len = bacapp_snprintf_value(NULL, 0, &object_value);
-    if (str_len > 0) {
+    if (str_len >= 0) {
         char str[str_len + 1];
-        bacapp_snprintf_value(str, str_len + 1, &object_value);
+        str[0] = '\0';
-        zassert_mem_equal(str, "Null", str_len, NULL);
+        /* normal case */
+        len = bacapp_snprintf_value(str, str_len + 1, &object_value);
+        zassert_mem_equal(
+            str, expected, str_len, "str='%s' expected='%s'", str, expected);
+        zassert_equal(len, str_len, NULL);
+        /* test when buffer is too small the behavior matches snprintf(). */
+        test_len = len;
+        while (test_len >= 0) {
+            len = bacapp_snprintf_value(str, test_len, &object_value);
+            zassert_equal(len, str_len, "len=%d str_len=%d", len, str_len);
+            zassert_equal(
+                str[test_len], '\0', "tag=%u '%s':str[%d]=%02X not NULL",
+                tag_number, argv, test_len, str[test_len]);
+            test_len--;
+        }
     }
 }
 
+/**
+ * @brief Test
+ */
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST(bacapp_tests, test_bacapp_sprintf_epics)
+#else
+static void test_bacapp_sprintf_epics(void)
+#endif
+{
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_NULL, NULL, "Null");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_BOOLEAN, "true", "TRUE");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_BOOLEAN, "false", "FALSE");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_UNSIGNED_INT, "0", "0");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_UNSIGNED_INT, "42", "42");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_UNSIGNED_INT, "4294967295", "4294967295");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_SIGNED_INT, "0", "0");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_SIGNED_INT, "-42", "-42");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_SIGNED_INT, "2147483647", "2147483647");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_SIGNED_INT, "-2147483648", "-2147483648");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_REAL, "0.0", "0.000000");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_REAL, "3.14159", "3.141590");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_REAL, "-3.14159", "-3.141590");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_DOUBLE, "0.0", "0.000000");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_DOUBLE, "3.14159", "3.141590");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_DOUBLE, "-3.14159", "-3.141590");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OCTET_STRING, "1234567890ABCDEF",
+        "1234567890ABCDEF");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OCTET_STRING, "12-34-56-78-90-AB-CD-EF",
+        "1234567890ABCDEF");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OCTET_STRING, "12 34 56 78 90 AB CD EF",
+        "1234567890ABCDEF");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_OCTET_STRING, "", "");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_CHARACTER_STRING, "Karg!", "\"Karg!\"");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_CHARACTER_STRING, "", "\"\"");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_BIT_STRING, "1011010010011111",
+        "{true,false,true,true,false,true,false,false,true,false,false,true,"
+        "true,true,true,true}");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_BIT_STRING, "111100001111",
+        "{true,true,true,true,false,false,false,false,true,true,true,true}");
+    /* note to tester: enumerated test relies on BACNET_OBJECT_PROPERTY_VALUE
+       initialized in test_bacapp_snprintf() as PROP_SEGMENTATION_SUPPORTED */
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_ENUMERATED, "0", "segmented-both");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_ENUMERATED, "1", "segmented-transmit");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_ENUMERATED, "2", "segmented-receive");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_ENUMERATED, "3", "no-segmentation");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_ENUMERATED, "42", "Reserved for Use by ASHRAE");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_DATE, "2005/5/22:1", "Monday, May 22, 2005");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_DATE, "2007/2/14",
+        "Wednesday, February 14, 2007");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_DATE, "2155/255/255:255",
+        "any day of week, Any Month (unspecified), (unspecified)");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_TIME, "23:59:59.12", "23:59:59.12");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_TIME, "23:59:59", "23:59:59.00");
+    test_bacapp_snprintf(BACNET_APPLICATION_TAG_TIME, "23:59", "23:59:00.00");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_TIME, "255:255:255.255", "**:**:**.**");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OBJECT_ID, "0:100", "(analog-input, 100)");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OBJECT_ID, "8:4194303", "(device, 4194303)");
+    test_bacapp_snprintf(
+        BACNET_APPLICATION_TAG_OBJECT_ID, "0:0", "(analog-input, 0)");
+}
+
 /**
  * @}
  */
@@ -1347,7 +1445,7 @@ void test_main(void)
         ztest_unit_test(testBACnetApplicationDataLength),
         ztest_unit_test(testBACnetApplicationData_Safe),
         ztest_unit_test(test_bacapp_data),
-        ztest_unit_test(test_bacapp_sprintf_data));
+        ztest_unit_test(test_bacapp_sprintf_epics));
 
     ztest_run_test_suite(bacapp_tests);
 }