fix: memory leak

Author: elagil

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
 - repo: https://github.com/pre-commit/mirrors-clang-format
-  rev: v17.0.2
+  rev: v17.0.3
   hooks:
   - id: clang-format
     types_or: [c++, c]
@@ -19,7 +19,7 @@ repos:
   - id: cpplint
 
 - repo: https://github.com/crate-ci/typos
-  rev: v1.16.20
+  rev: typos-dict-v0.11.2
   hooks:
   - id: typos
     args:

.vscode/launch.json

@@ -15,6 +15,9 @@
             "servertype": "external",
             "preLaunchTask": "Launch OpenOCD",
             "gdbTarget": "${env:WSL2_HOST_IP}:3333",
+            "preLaunchCommands": [
+                "monitor arm semihosting enable",
+            ],
         }
     ]
 }

.vscode/settings.json

@@ -37,5 +37,5 @@
                 "$gcc"
             ],
         }
-    ]
+    ],
 }

Makefile

@@ -2,14 +2,15 @@
 # Build global options
 # NOTE: Can be overridden externally.
 #
+SEMIHOSTING_OPT = --specs=rdimon.specs -lc -lrdimon
 
 ifeq ($(APP),)
   APP = default
 endif
 
 # Compiler options here.
 ifeq ($(USE_OPT),)
-  USE_OPT = -Og -ggdb -fomit-frame-pointer -falign-functions=16
+  USE_OPT = -Og -ggdb -fomit-frame-pointer -falign-functions=16 $(SEMIHOSTING_OPT)
 endif
 
 # C specific options here (added to USE_OPT).

apps/blus_mini/blus_mini.c

@@ -16,11 +16,6 @@
 #include "chprintf.h"
 #include "tas2780.h"
 
-/**
- * @brief Global stream pointer for print messages.
- */
-static BaseSequentialStream *gp_stream = (BaseSequentialStream *)&SD2;
-
 /**
  * @brief Settings structure for the TAS2780 I2C driver.
  */
@@ -70,32 +65,31 @@ static THD_FUNCTION(housekeeping_thread, arg) {
     chRegSetThreadName("reporting");
 
     while (true) {
+        tas2780_acquire_lock();
         tas2780_ensure_active_all();
+        tas2780_release_lock();
 
-        chBSemWait(&g_stream_lock);
         chSysLock();
-        size_t buffer_fill_size = audio_playback_get_buffer_fill_size();
-        size_t feedback_value   = audio_feedback_get_value();
-        size_t playback_state   = audio_playback_get_state();
+        // size_t buffer_fill_size = audio_playback_get_buffer_fill_size();
+        // size_t feedback_value   = audio_feedback_get_value();
+        // size_t playback_state   = audio_playback_get_state();
         chSysUnlock();
 
 // Disable extended reporting statistics.
 #if 0
         uint8_t noise_gate_mask = tas2780_get_noise_gate_mask_all();
-        chprintf(gp_stream, "Noise gate: %u\n", noise_gate_mask);
+        PRINTF("Noise gate: %u\n", noise_gate_mask);
 
-        chprintf(gp_stream, "Potentiometer: %u\n",
+        PRINTF("Potentiometer: %u\n",
                  g_adc_sample >> 4);  // Convert to an 8 bit number.
 
-        chprintf(gp_stream, "Volume: %li / %li dB\n",
+        PRINTF("Volume: %li / %li dB\n",
                  (audio_request_get_channel_volume(AUDIO_COMMON_CHANNEL_LEFT) >> 8),
                  (audio_request_get_channel_volume(AUDIO_COMMON_CHANNEL_RIGHT) >> 8));
 #endif
 
-        chprintf(gp_stream, "Buffer: %lu / %lu (fb %lu) @ state %lu\n", buffer_fill_size, AUDIO_MAX_BUFFER_SIZE,
-                 feedback_value, playback_state);
-
-        chBSemSignal(&g_stream_lock);
+        // PRINTF("Buffer: %u / %u (fb %u) @ state %u\n", buffer_fill_size, AUDIO_MAX_BUFFER_SIZE, feedback_value,
+        //        playback_state);
 
         chThdSleepMilliseconds(500);
     }
@@ -107,7 +101,12 @@ static THD_FUNCTION(housekeeping_thread, arg) {
 void app_setup(void) {
     // Setup amplifiers.
     i2cStart(&I2CD1, &g_tas2780_i2c_config);
+
+    tas2780_init();
+
+    tas2780_acquire_lock();
     tas2780_setup_all();
+    tas2780_release_lock();
 
     // Begin reading volume potentiometer ADC.
     app_start_volume_adc();
@@ -128,6 +127,7 @@ static void app_set_volume_and_mute_state(void) {
     int16_t right_channel_volume = audio_request_get_channel_volume(AUDIO_COMMON_CHANNEL_RIGHT);
     chSysUnlock();
 
+    tas2780_acquire_lock();
     if (b_left_channel_is_muted) {
         tas2780_set_volume_all(TAS2780_VOLUME_MUTE, TAS2780_CHANNEL_LEFT);
     } else {
@@ -139,6 +139,7 @@ static void app_set_volume_and_mute_state(void) {
     } else {
         tas2780_set_volume_all(right_channel_volume, TAS2780_CHANNEL_RIGHT);
     }
+    tas2780_release_lock();
 }
 
 /**

source/audio/audio.c

@@ -14,13 +14,8 @@
 
 #include <string.h>
 
-#include "chprintf.h"
 #include "common.h"
-
-/**
- * @brief Global stream pointer for print messages.
- */
-static BaseSequentialStream *gp_stream = (BaseSequentialStream *)&SD2;
+#include "print.h"
 
 /**
  * @brief The number of audio messages that can be held.
@@ -165,7 +160,7 @@ static void audio_update_sample_rate(void) {
     g_i2s_config.i2spr = AUDIO_SPI_GET_I2SPR(sample_rate_hz);
 }
 
-static THD_WORKING_AREA(wa_audio_thread, 128);
+static THD_WORKING_AREA(wa_audio_thread, 256u);
 
 /**
  * @brief A thread that handles audio-related tasks.
@@ -192,9 +187,7 @@ static THD_FUNCTION(audio_thread, arg) {
 
         switch (message) {
             case AUDIO_COMMON_MSG_START_PLAYBACK:
-                chBSemWait(&g_stream_lock);
-                chprintf(gp_stream, "### Start playback.\n");
-                chBSemSignal(&g_stream_lock);
+                PRINTF("### Start playback.\n");
 
                 // Set volumes to the values configured via USB audio.
                 audio_send_app_message(AUDIO_COMMON_MSG_SET_VOLUME);
@@ -205,9 +198,7 @@ static THD_FUNCTION(audio_thread, arg) {
                 break;
 
             case AUDIO_COMMON_MSG_STOP_PLAYBACK:
-                chBSemWait(&g_stream_lock);
-                chprintf(gp_stream, "### Stop playback.\n");
-                chBSemSignal(&g_stream_lock);
+                PRINTF("### Stop playback.\n");
 
                 audio_feedback_stop_sof_capture();
                 i2sStopExchange(&I2S_DRIVER);
@@ -217,9 +208,7 @@ static THD_FUNCTION(audio_thread, arg) {
                 break;
 
             case AUDIO_COMMON_MSG_SET_SAMPLE_RATE:
-                chBSemWait(&g_stream_lock);
-                chprintf(gp_stream, "### Set sample rate.\n");
-                chBSemSignal(&g_stream_lock);
+                PRINTF("### Set sample rate.\n");
 
                 chSysLock();
                 audio_update_sample_rate();

source/audio/audio_request.c

@@ -43,7 +43,7 @@ static volatile struct audio_request_message {
     uint16_t value;         ///< The wValue field of the request.
     uint16_t index;         ///< The wIndex field of the request.
     uint16_t length;        ///< The valid data length.
-    uint8_t  data[8u];      ///< The data buffer.
+    uint8_t  data[1024u];   ///< The data buffer.
 } g_request;
 
 /**
@@ -398,6 +398,8 @@ bool audio_request_hook_cb(USBDriver *p_usb) {
     g_request.index        = ((p_usb->setup[5] << 8) | p_usb->setup[4]);
     g_request.length       = ((p_usb->setup[7] << 8) | p_usb->setup[6]);
 
+    chDbgAssert(g_request.length <= ARRAY_LENGTH(g_request.data), "Request data buffer exceeded.");
+
     switch (g_request.request_type & USB_RTYPE_TYPE_MASK) {
         case USB_RTYPE_TYPE_STD:
             return audio_request_handle_standard(p_usb);

source/common.h

@@ -127,11 +127,6 @@ __STATIC_INLINE size_t add_circular_unsigned(size_t summand_a, size_t summand_b,
     return wrap_unsigned(summand_a + summand_b, max_value);
 }
 
-/**
- * @brief A lock for the stream that is used for printing messages, defined in the \a main module.
- */
-extern binary_semaphore_t g_stream_lock;
-
 #endif  // SOURCE_COMMON_H_
 
 /**

source/drivers/tas2780/tas2780.c

@@ -14,6 +14,26 @@
 #include "tas2780_registers.h"
 #include "tas2780_settings.h"
 
+/**
+ * @brief The locking semaphore for the TAS2780 module.
+ */
+static binary_semaphore_t g_bsem;
+
+/**
+ * @brief Lock the TAS2780 module.
+ */
+void tas2780_acquire_lock(void) { chBSemWait(&g_bsem); }
+
+/**
+ * @brief Release the TAS2780 module.
+ */
+void tas2780_release_lock(void) { chBSemSignal(&g_bsem); }
+
+/**
+ * @brief Initialize the TAS2780 module.
+ */
+void tas2780_init(void) { chBSemObjectInit(&g_bsem, false); }
+
 /**
  * @brief All amplifier contexts.
  * @details Will be set up with settings from \a tas2780_settings.h .

source/drivers/tas2780/tas2780.h

@@ -55,9 +55,12 @@ struct tas2780_context {
                                    ///< is loudest. The range of gains is 10 dB.
 };
 
+void tas2780_acquire_lock(void);
+void tas2780_release_lock(void);
+void tas2780_init(void);
+
 void    tas2780_setup_all(void);
 void    tas2780_set_volume_all(int16_t volume_8q8_db, enum tas2780_channel channel);
-void    tas2780_init(struct tas2780_context *p_context, uint16_t device_address);
 void    tas2780_ensure_active_all(void);
 uint8_t tas2780_get_noise_gate_mask_all(void);
 

source/main.c

@@ -17,8 +17,8 @@
 
 #include "audio.h"
 #include "ch.h"
-#include "chprintf.h"
 #include "hal.h"
+#include "print.h"
 #include "usb.h"
 
 /**
@@ -36,16 +36,6 @@ static mailbox_t g_mailbox;
  */
 static msg_t g_mailbox_buffer[MESSAGE_BUFFER_LENGTH];
 
-/**
- * @brief Global stream pointer for print messages.
- */
-static BaseSequentialStream *gp_stream = (BaseSequentialStream *)&SD2;
-
-/**
- * @brief A lock for the stream that is used for printing messages.
- */
-binary_semaphore_t g_stream_lock;
-
 // Weak definitions that shall be overridden by the user application (if required).
 
 /**
@@ -68,6 +58,11 @@ __WEAK void app_set_volume(void) {}
  */
 __WEAK void app_set_mute_state(void) {}
 
+/**
+ * @brief Semihosting initialization.
+ */
+extern void initialise_monitor_handles(void);
+
 /**
  * @brief Application entry point.
  *
@@ -77,6 +72,9 @@ int main(void) {
     halInit();
     chSysInit();
 
+    // Enable semihosting interface.
+    initialise_monitor_handles();
+
     // Initialize the main thread mailbox that receives audio messages (volume, mute).
     chMBObjectInit(&g_mailbox, g_mailbox_buffer, ARRAY_LENGTH(g_mailbox_buffer));
 
@@ -86,14 +84,10 @@ int main(void) {
     // Initialize the USB module.
     usb_setup();
 
-    chBSemObjectInit(&g_stream_lock, false);
-
     // Initialize a stream for print messages.
     sdStart(&SD2, NULL);
 
-    chBSemWait(&g_stream_lock);
-    chprintf(gp_stream, "### Starting USB-I2S bridge.\n");
-    chBSemSignal(&g_stream_lock);
+    PRINTF("### Starting USB-I2S bridge.\n");
 
     // Set up the user application.
     app_setup();
@@ -107,30 +101,26 @@ int main(void) {
             chSysHalt("Failed to receive message.");
         }
 
-        chBSemWait(&g_stream_lock);
-
         switch (message) {
             case AUDIO_COMMON_MSG_RESET_VOLUME:
-                chprintf(gp_stream, "### Reset volume.\n");
+                PRINTF("### Reset volume.\n");
                 app_reset_volume();
                 break;
 
             case AUDIO_COMMON_MSG_SET_MUTE_STATE:
-                chprintf(gp_stream, "### Set mute state.\n");
+                PRINTF("### Set mute state.\n");
                 app_set_mute_state();
                 break;
 
             case AUDIO_COMMON_MSG_SET_VOLUME:
-                chprintf(gp_stream, "### Set volume.\n");
+                PRINTF("### Set volume.\n");
                 app_set_volume();
                 break;
 
             default:
                 chSysHalt("Unknown message type.");
                 break;
         }
-
-        chBSemSignal(&g_stream_lock);
     }
 }
 

source/print.h

@@ -0,0 +1,32 @@
+// Copyright 2023 elagil
+
+/**
+ * @file
+ * @brief   Printing functionality.
+ *
+ * @addtogroup common
+ * @{
+ */
+
+#ifndef SOURCE_PRINT_H_
+#define SOURCE_PRINT_H_
+
+#include <stdarg.h>
+
+#include "chprintf.h"
+#include "common.h"
+
+/**
+ * @brief Generic print function, which can perform different functions.
+ *
+ * @param fmt Format specifier.
+ * @param ... Additional arguments.
+ */
+static inline void PRINTF(const char* fmt, ...) {
+    va_list args;
+    va_start(args, fmt);
+    chvprintf((BaseSequentialStream*)&SD2, fmt, args);
+    va_end(args);
+}
+
+#endif  // SOURCE_PRINT_H_