don't gc_free() ringbuf if no heap; cleanup ringbuf

ports/broadcom/common-hal/busio/UART.c

@@ -213,16 +213,18 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
     self->sigint_enabled = sigint_enabled;
 
     if (rx != NULL) {
+        self->allocated_ringbuf = true;
+        // Use the provided buffer when given.
         if (receiver_buffer != NULL) {
-            self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
+            ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
+            self->allocated_ringbuf = false;
         } else {
             // Initially allocate the UART's buffer in the long-lived part of the
             // heap. UARTs are generally long-lived objects, but the "make long-
             // lived" machinery is incapable of moving internal pointers like
             // self->buffer, so do it manually.  (However, as long as internal
             // pointers like this are NOT moved, allocating the buffer
             // in the long-lived pool is not strictly necessary)
-            // (This is a macro.)
             if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
                 mp_raise_msg(&mp_type_MemoryError, translate("Failed to allocate RX buffer"));
             }
@@ -342,7 +344,9 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
         pl011->CR = 0;
     }
     active_uart[self->uart_id] = NULL;
-    ringbuf_free(&self->ringbuf);
+    if (self->allocated_ringbuf) {
+        ringbuf_free(&self->ringbuf);
+    }
     uart_status[self->uart_id] = STATUS_FREE;
     common_hal_reset_pin(self->tx_pin);
     common_hal_reset_pin(self->rx_pin);

ports/broadcom/common-hal/busio/UART.h

@@ -36,10 +36,11 @@ typedef struct {
     const mcu_pin_obj_t *rx_pin;
     const mcu_pin_obj_t *cts_pin;
     const mcu_pin_obj_t *rts_pin;
-    uint8_t uart_id;
     uint32_t baudrate;
     uint32_t timeout_ms;
+    uint8_t uart_id;
     bool sigint_enabled;
+    bool allocated_ringbuf;
     ringbuf_t ringbuf;
 } busio_uart_obj_t;
 

ports/espressif/common-hal/_bleio/CharacteristicBuffer.c

@@ -73,10 +73,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
     self->characteristic = characteristic;
     self->timeout_ms = timeout * 1000;
 
-    self->ringbuf.buf = (uint8_t *)buffer;
-    self->ringbuf.size = buffer_size;
-    self->ringbuf.iget = 0;
-    self->ringbuf.iput = 0;
+    ringbuf_init(&self->ringbuf, buffer, buffer_size);
 
     if (static_handler_entry != NULL) {
         ble_event_add_handler_entry((ble_event_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);

ports/espressif/common-hal/_bleio/PacketBuffer.c

@@ -164,10 +164,7 @@ void _common_hal_bleio_packet_buffer_construct(
     }
 
     if (incoming) {
-        self->ringbuf.buf = (uint8_t *)incoming_buffer;
-        self->ringbuf.size = incoming_buffer_size;
-        self->ringbuf.iget = 0;
-        self->ringbuf.iput = 0;
+        ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
     }
 
     self->packet_queued = false;

ports/nrf/common-hal/_bleio/CharacteristicBuffer.c

@@ -90,10 +90,7 @@ void _common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buff
     self->characteristic = characteristic;
     self->timeout_ms = timeout * 1000;
 
-    self->ringbuf.buf = (uint8_t *)buffer;
-    self->ringbuf.size = buffer_size;
-    self->ringbuf.iget = 0;
-    self->ringbuf.iput = 0;
+    ringbuf_init(&self->ringbuf, buffer, buffer_size);
 
     if (static_handler_entry != NULL) {
         ble_drv_add_event_handler_entry((ble_drv_evt_handler_entry_t *)static_handler_entry, characteristic_buffer_on_ble_evt, self);

ports/nrf/common-hal/_bleio/PacketBuffer.c

@@ -233,10 +233,7 @@ void _common_hal_bleio_packet_buffer_construct(
     }
 
     if (incoming) {
-        self->ringbuf.buf = (uint8_t *)incoming_buffer;
-        self->ringbuf.size = incoming_buffer_size;
-        self->ringbuf.iget = 0;
-        self->ringbuf.iput = 0;
+        ringbuf_init(&self->ringbuf, (uint8_t *)incoming_buffer, incoming_buffer_size);
     }
 
     self->packet_queued = false;

ports/nrf/common-hal/busio/UART.c

@@ -220,21 +220,19 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
         self->allocated_ringbuf = true;
         // Use the provided buffer when given.
         if (receiver_buffer != NULL) {
-            self->ringbuf.buf = receiver_buffer;
-            self->ringbuf.size = receiver_buffer_size - 1;
-            self->ringbuf.iput = 0;
-            self->ringbuf.iget = 0;
+            ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
             self->allocated_ringbuf = false;
+        } else {
             // Initially allocate the UART's buffer in the long-lived part of the
             // heap.  UARTs are generally long-lived objects, but the "make long-
             // lived" machinery is incapable of moving internal pointers like
             // self->buffer, so do it manually.  (However, as long as internal
             // pointers like this are NOT moved, allocating the buffer
             // in the long-lived pool is not strictly necessary)
-            // (This is a macro.)
-        } else if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
-            nrfx_uarte_uninit(self->uarte);
-            mp_raise_msg(&mp_type_MemoryError, translate("Failed to allocate RX buffer"));
+            if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
+                nrfx_uarte_uninit(self->uarte);
+                mp_raise_msg(&mp_type_MemoryError, translate("Failed to allocate RX buffer"));
+            }
         }
 
         self->rx_pin_number = rx->number;

ports/raspberrypi/common-hal/busio/UART.c

@@ -140,16 +140,23 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
     uart_set_hw_flow(self->uart, (cts != NULL), (rts != NULL));
 
     if (rx != NULL) {
-        // Initially allocate the UART's buffer in the long-lived part of the
-        // heap. UARTs are generally long-lived objects, but the "make long-
-        // lived" machinery is incapable of moving internal pointers like
-        // self->buffer, so do it manually.  (However, as long as internal
-        // pointers like this are NOT moved, allocating the buffer
-        // in the long-lived pool is not strictly necessary)
-        // (This is a macro.)
-        if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
-            mp_raise_msg(&mp_type_MemoryError, translate("Failed to allocate RX buffer"));
+        self->allocated_ringbuf = true;
+        // Use the provided buffer when given.
+        if (receiver_buffer != NULL) {
+            ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
+            self->allocated_ringbuf = false;
+        } else {
+            // Initially allocate the UART's buffer in the long-lived part of the
+            // heap. UARTs are generally long-lived objects, but the "make long-
+            // lived" machinery is incapable of moving internal pointers like
+            // self->buffer, so do it manually.  (However, as long as internal
+            // pointers like this are NOT moved, allocating the buffer
+            // in the long-lived pool is not strictly necessary)
+            if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
+                mp_raise_msg(&mp_type_MemoryError, translate("Failed to allocate RX buffer"));
+            }
         }
+
         active_uarts[uart_id] = self;
         if (uart_id == 1) {
             self->uart_irq_id = UART1_IRQ;
@@ -172,7 +179,9 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
         return;
     }
     uart_deinit(self->uart);
-    ringbuf_free(&self->ringbuf);
+    if (self->allocated_ringbuf) {
+        ringbuf_free(&self->ringbuf);
+    }
     active_uarts[self->uart_id] = NULL;
     uart_status[self->uart_id] = STATUS_FREE;
     reset_pin_number(self->tx_pin);

ports/raspberrypi/common-hal/busio/UART.h

@@ -40,6 +40,7 @@ typedef struct {
     uint8_t rts_pin;
     uint8_t uart_id;
     uint8_t uart_irq_id;
+    bool allocated_ringbuf;
     uint32_t baudrate;
     uint32_t timeout_ms;
     uart_inst_t *uart;

ports/stm/common-hal/busio/UART.c

@@ -217,9 +217,17 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
 
     // Init buffer for rx and claim pins
     if (self->rx != NULL) {
+        self->allocated_ringbuf = true;
         if (receiver_buffer != NULL) {
-            self->ringbuf = (ringbuf_t) { receiver_buffer, receiver_buffer_size };
+            ringbuf_init(&self->ringbuf, receiver_buffer, receiver_buffer_size);
+            self->allocated_ringbuf = false;
         } else {
+            // Initially allocate the UART's buffer in the long-lived part of the
+            // heap. UARTs are generally long-lived objects, but the "make long-
+            // lived" machinery is incapable of moving internal pointers like
+            // self->buffer, so do it manually.  (However, as long as internal
+            // pointers like this are NOT moved, allocating the buffer
+            // in the long-lived pool is not strictly necessary)
             if (!ringbuf_alloc(&self->ringbuf, receiver_buffer_size, true)) {
                 mp_raise_ValueError(translate("UART Buffer allocation error"));
             }
@@ -284,7 +292,9 @@ void common_hal_busio_uart_deinit(busio_uart_obj_t *self) {
         self->rx = NULL;
     }
 
-    ringbuf_free(&self->ringbuf);
+    if (self->allocated_ringbuf) {
+        ringbuf_free(&self->ringbuf);
+    }
 }
 
 size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t len, int *errcode) {

ports/stm/common-hal/busio/UART.h

@@ -48,12 +48,12 @@ typedef struct {
     const mcu_periph_obj_t *rx;
 
     ringbuf_t ringbuf;
-    uint8_t rx_char;
-
     uint32_t baudrate;
     uint32_t timeout_ms;
 
+    uint8_t rx_char;
     bool sigint_enabled;
+    bool allocated_ringbuf;
 } busio_uart_obj_t;
 
 void uart_reset(void);

ports/unix/coverage.c

@@ -526,7 +526,9 @@ STATIC mp_obj_t extra_coverage(void) {
 
     // ringbuf
     {
-        byte buf[100];
+        #define RINGBUF_CAPACITY 99
+
+        byte buf[RINGBUF_CAPACITY];
         ringbuf_t ringbuf;
         ringbuf_init(&ringbuf, &buf[0], sizeof(buf));
 
@@ -546,7 +548,7 @@ STATIC mp_obj_t extra_coverage(void) {
         mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
 
         // Two-byte put with full ringbuf.
-        for (int i = 0; i < 99; ++i) {
+        for (int i = 0; i < RINGBUF_CAPACITY; ++i) {
             ringbuf_put(&ringbuf, i);
         }
         mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
@@ -558,40 +560,36 @@ STATIC mp_obj_t extra_coverage(void) {
         ringbuf_get(&ringbuf);
         mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
         mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0xcc99));
-        for (int i = 0; i < 97; ++i) {
+        for (int i = 0; i < RINGBUF_CAPACITY - 2; ++i) {
             ringbuf_get(&ringbuf);
         }
         mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
         mp_printf(&mp_plat_print, "%d %d\n", ringbuf_num_empty(&ringbuf), ringbuf_num_filled(&ringbuf));
 
         // Two-byte put with wrap around on first byte:
-        ringbuf.iput = 0;
-        ringbuf.iget = 0;
-        for (int i = 0; i < 99; ++i) {
+        ringbuf_clear(&ringbuf);
+        for (int i = 0; i < RINGBUF_CAPACITY; ++i) {
             ringbuf_put(&ringbuf, i);
             ringbuf_get(&ringbuf);
         }
         mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0x11bb));
         mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
 
         // Two-byte put with wrap around on second byte:
-        ringbuf.iput = 0;
-        ringbuf.iget = 0;
-        for (int i = 0; i < 98; ++i) {
+        ringbuf_clear(&ringbuf);
+        for (int i = 0; i < RINGBUF_CAPACITY - 1; ++i) {
             ringbuf_put(&ringbuf, i);
             ringbuf_get(&ringbuf);
         }
         mp_printf(&mp_plat_print, "%d\n", ringbuf_put16(&ringbuf, 0x22ff));
         mp_printf(&mp_plat_print, "%04x\n", ringbuf_get16(&ringbuf));
 
         // Two-byte get from empty ringbuf.
-        ringbuf.iput = 0;
-        ringbuf.iget = 0;
+        ringbuf_clear(&ringbuf);
         mp_printf(&mp_plat_print, "%d\n", ringbuf_get16(&ringbuf));
 
         // Two-byte get from ringbuf with one byte available.
-        ringbuf.iput = 0;
-        ringbuf.iget = 0;
+        ringbuf_clear(&ringbuf);
         ringbuf_put(&ringbuf, 0xaa);
         mp_printf(&mp_plat_print, "%d\n", ringbuf_get16(&ringbuf));
     }