From 3517ae6387e10ebb01c34134afcae6873d3500d9 Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 14 Mar 2022 10:47:33 +0800 Subject: [PATCH] lcd: check return value of xQueueReceive Fix warnnings reported by Coverity Scan Test --- components/esp_lcd/src/esp_lcd_panel_io_i2s.c | 27 ++++++++++++------- components/esp_lcd/src/esp_lcd_panel_io_i80.c | 20 +++++++++----- components/esp_lcd/src/esp_lcd_panel_io_spi.c | 14 ++++++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c index c8ccdf308f..e1f805500f 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c @@ -323,9 +323,12 @@ static esp_err_t panel_io_i80_del(esp_lcd_panel_io_t *io) lcd_panel_io_i80_t *i80_device = __containerof(io, lcd_panel_io_i80_t, base); esp_lcd_i80_bus_t *bus = i80_device->bus; lcd_i80_trans_descriptor_t *trans_desc = NULL; + size_t num_trans_inflight = i80_device->num_trans_inflight; // wait all pending transaction to finish - for (size_t i = 0; i < i80_device->num_trans_inflight; i++) { - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + i80_device->num_trans_inflight--; } // remove from device list portENTER_CRITICAL(&bus->spinlock); @@ -466,12 +469,13 @@ static esp_err_t panel_io_i80_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons lcd_i80_trans_descriptor_t *trans_desc = NULL; assert(param_size <= (bus->num_dma_nodes * DMA_DESCRIPTOR_BUFFER_MAX_SIZE) && "parameter bytes too long, enlarge max_transfer_bytes"); assert(param_size <= CONFIG_LCD_PANEL_IO_FORMAT_BUF_SIZE && "format buffer too small, increase CONFIG_LCD_PANEL_IO_FORMAT_BUF_SIZE"); - + size_t num_trans_inflight = next_device->num_trans_inflight; // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary @@ -530,12 +534,13 @@ static esp_err_t panel_io_i80_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons lcd_panel_io_i80_t *cur_device = bus->cur_device; lcd_i80_trans_descriptor_t *trans_desc = NULL; assert(color_size <= (bus->num_dma_nodes * DMA_DESCRIPTOR_BUFFER_MAX_SIZE) && "color bytes too long, enlarge max_transfer_bytes"); - + size_t num_trans_inflight = next_device->num_trans_inflight; // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary @@ -730,6 +735,8 @@ static IRAM_ATTR void lcd_default_isr_handler(void *args) if (high_task_woken == pdTRUE) { need_yield = true; } + // sanity check + assert(trans_desc); // only clear the interrupt status when we're sure there still remains transaction to handle i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i80.c b/components/esp_lcd/src/esp_lcd_panel_io_i80.c index b5e53055a0..351c0a2ee5 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i80.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i80.c @@ -315,8 +315,11 @@ static esp_err_t panel_io_i80_del(esp_lcd_panel_io_t *io) esp_lcd_i80_bus_t *bus = i80_device->bus; lcd_i80_trans_descriptor_t *trans_desc = NULL; // wait all pending transaction to finish - for (size_t i = 0; i < i80_device->num_trans_inflight; i++) { - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + size_t num_trans_inflight = i80_device->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + i80_device->num_trans_inflight--; } // remove from device list portENTER_CRITICAL(&bus->spinlock); @@ -383,10 +386,12 @@ static esp_err_t panel_io_i80_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons i80_lcd_prepare_cmd_buffer(bus, next_device, &lcd_cmd); uint32_t param_len = i80_lcd_prepare_param_buffer(bus, next_device, param, param_size); // wait all pending transaction in the queue to finish - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + size_t num_trans_inflight = next_device->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE( xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; uint32_t intr_status = lcd_ll_get_interrupt_status(bus->hal.dev); lcd_ll_clear_interrupt_status(bus->hal.dev, intr_status); @@ -437,7 +442,8 @@ static esp_err_t panel_io_i80_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons trans_desc = &i80_device->trans_pool[i80_device->num_trans_inflight]; } else { // transaction pool has used up, recycle one from done_queue - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); i80_device->num_trans_inflight--; } trans_desc->i80_device = i80_device; @@ -640,6 +646,8 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args) if (high_task_woken == pdTRUE) { need_yield = true; } + // sanity check + assert(trans_desc); // only clear the interrupt status when we're sure there still remains transaction to handle lcd_ll_clear_interrupt_status(bus->hal.dev, intr_status); // switch devices if necessary diff --git a/components/esp_lcd/src/esp_lcd_panel_io_spi.c b/components/esp_lcd/src/esp_lcd_panel_io_spi.c index b5395de43a..6a355e08e1 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_spi.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_spi.c @@ -124,9 +124,11 @@ static esp_err_t panel_io_spi_del(esp_lcd_panel_io_t *io) esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // wait all pending transaction to finish - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } spi_bus_remove_device(spi_panel_io->spi_dev); if (spi_panel_io->dc_gpio_num >= 0) { @@ -175,11 +177,12 @@ static esp_err_t panel_io_spi_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } - spi_panel_io->num_trans_inflight = 0; lcd_trans = &spi_panel_io->trans_pool[0]; memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t)); spi_lcd_prepare_cmd_buffer(spi_panel_io, &lcd_cmd); @@ -223,11 +226,12 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } - spi_panel_io->num_trans_inflight = 0; lcd_trans = &spi_panel_io->trans_pool[0]; memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t)); spi_lcd_prepare_cmd_buffer(spi_panel_io, &lcd_cmd);