Memory Leak?

[us292231]

Thank you for the pointer to this repo and for the code. Working much better than the other library :)

Question: I am looking at the uart_event_task method. Inside the for(;;) loop I don't see any way to exit the loop and am wondering if the free(dtmp), etc are ever executed or if there is a memory leak here. Please correct me if I am wrong, but the breaks only drop out of the case statements.

Jim

void DMX::uart_event_task(void *pvParameters) {
    uart_event_t event;
    uint8_t* dtmp = (uint8_t*) malloc(BUF_SIZE);
    for(;;) {
        // wait for data in the dmx_queue
        if(xQueueReceive(dmx_rx_queue, (void * )&event, (portTickType)portMAX_DELAY)) {
            bzero(dtmp, BUF_SIZE);
            switch(event.type) {
                case UART_DATA:
                    // read the received data
                    uart_read_bytes(DMX_UART_NUM, dtmp, event.size, portMAX_DELAY);
                    // check if break detected
                    if(dmx_state == DMX_BREAK) {
                        // if not 0, then RDM or custom protocol
                        if(dtmp[0] == 0) {
                            dmx_state = DMX_DATA;
                            // reset dmx adress to 0
                            current_rx_addr = 0;
                            xSemaphoreTake(sync_dmx, portMAX_DELAY);
                            // store received timestamp
                            last_dmx_packet = millis();
                            xSemaphoreGive(sync_dmx);
                        }
                    }

                    // check if in data receive mode
                    if(dmx_state == DMX_DATA) {
                        xSemaphoreTake(sync_dmx, portMAX_DELAY);
                        // copy received bytes to dmx data array
                        for(int i = 0; i < event.size; i++) {
                            dmx_data[current_rx_addr++] = dtmp[i];
                        }
                        xSemaphoreGive(sync_dmx);
                    }
                    break;

                case UART_BREAK:
                    // break detected, clear queue und flush received bytes
                    // results in missing the last few bytes... needs fix
                    uart_flush_input(DMX_UART_NUM);
                    xQueueReset(dmx_rx_queue);
                    dmx_state = DMX_BREAK;
                    break;
                    
                case UART_FRAME_ERR:
                case UART_PARITY_ERR:
                case UART_BUFFER_FULL:
                case UART_FIFO_OVF:
                default:
                    // error recevied, going to idle mode
                    uart_flush_input(DMX_UART_NUM);
                    xQueueReset(dmx_rx_queue);
                    dmx_state = DMX_IDLE;
                    break;
            }
        }
    }
    free(dtmp);
    dtmp = NULL;
    vTaskDelete(NULL);
}

[luksal]

Hey @us292231
It is correct that the loop is never actually left. I have to admit that I copied this part blindly from an IDF example.
So yes, the part after the loop can actually be omitted in my opinion.
Thanks a lot for the hint.

[us292231]

Ok, one more thought. I don't think there is garbage collection, so is there still a memory leak or does the malloc on the stack? It's been a long time since I have done straight c...

[luksal]

Because malloc is outside of the loop, the memory will be allocated just once. So in my eyes there is no need for garbage collection or a possible memory leak.

[us292231]

Got it -- thanks for the clarification. I didn't understand the example code and thought that the task got called every time there was a set of frames to be received. The task just runs forever, so all is good.

Thanks again -- cool stuff.
Jim