From 409c7e56108757a6864a3c70844712749b1ac6e1 Mon Sep 17 00:00:00 2001 From: Ondrej Date: Thu, 24 Nov 2022 12:37:09 +0000 Subject: [PATCH] Improved emac rx task code to suppress Coverity false positive memory leak indication --- components/esp_eth/src/esp_eth_mac_dm9051.c | 38 +++++++++---------- components/esp_eth/src/esp_eth_mac_esp.c | 36 +++++++++--------- .../esp_eth/src/esp_eth_mac_ksz8851snl.c | 38 +++++++++---------- components/esp_eth/src/esp_eth_mac_w5500.c | 38 +++++++++---------- 4 files changed, 71 insertions(+), 79 deletions(-) diff --git a/components/esp_eth/src/esp_eth_mac_dm9051.c b/components/esp_eth/src/esp_eth_mac_dm9051.c index e7dc2c5c3a..5d4d284aa5 100644 --- a/components/esp_eth/src/esp_eth_mac_dm9051.c +++ b/components/esp_eth/src/esp_eth_mac_dm9051.c @@ -805,32 +805,30 @@ static void emac_dm9051_task(void *arg) uint32_t frame_len = ETH_MAX_PACKET_SIZE; uint8_t *buffer; dm9051_alloc_recv_buf(emac, &buffer, &frame_len); - /* there is a waiting frame */ - if (frame_len) { - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { - dm9051_flush_recv_frame(emac); - free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); - } - } else { - ESP_LOGE(TAG, "frame read from module failed"); + /* we have memory to receive the frame of maximal size previously defined */ + if (buffer != NULL) { + uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { dm9051_flush_recv_frame(emac); free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); } } else { - ESP_LOGE(TAG, "no mem for receive buffer"); + ESP_LOGE(TAG, "frame read from module failed"); dm9051_flush_recv_frame(emac); + free(buffer); } + /* if allocation failed and there is a waiting frame */ + } else if (frame_len) { + ESP_LOGE(TAG, "no mem for receive buffer"); + dm9051_flush_recv_frame(emac); } } while (emac->packets_remain); } diff --git a/components/esp_eth/src/esp_eth_mac_esp.c b/components/esp_eth/src/esp_eth_mac_esp.c index 9894420703..9d43093313 100644 --- a/components/esp_eth/src/esp_eth_mac_esp.c +++ b/components/esp_eth/src/esp_eth_mac_esp.c @@ -259,28 +259,26 @@ static void emac_esp32_rx_task(void *arg) /* set max expected frame len */ uint32_t frame_len = ETH_MAX_PACKET_SIZE; buffer = emac_hal_alloc_recv_buf(&emac->hal, &frame_len); - /* there is a waiting frame */ - if (frame_len) { - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - uint32_t recv_len = emac_hal_receive_frame(&emac->hal, buffer, EMAC_HAL_BUF_SIZE_AUTO, &emac->frames_remain, &emac->free_rx_descriptor); - if (recv_len == 0) { - ESP_LOGE(TAG, "frame copy error"); - free(buffer); - /* ensure that interface to EMAC does not get stuck with unprocessed frames */ - emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor); - } else if (frame_len > recv_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len= %d", recv_len); - emac->eth->stack_input(emac->eth, buffer, recv_len); - } - } else { - ESP_LOGE(TAG, "no mem for receive buffer"); + /* we have memory to receive the frame of maximal size previously defined */ + if (buffer != NULL) { + uint32_t recv_len = emac_hal_receive_frame(&emac->hal, buffer, EMAC_HAL_BUF_SIZE_AUTO, &emac->frames_remain, &emac->free_rx_descriptor); + if (recv_len == 0) { + ESP_LOGE(TAG, "frame copy error"); + free(buffer); /* ensure that interface to EMAC does not get stuck with unprocessed frames */ emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor); + } else if (frame_len > recv_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len= %d", recv_len); + emac->eth->stack_input(emac->eth, buffer, recv_len); } + /* if allocation failed and there is a waiting frame */ + } else if (frame_len) { + ESP_LOGE(TAG, "no mem for receive buffer"); + /* ensure that interface to EMAC does not get stuck with unprocessed frames */ + emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor); } #if CONFIG_ETH_SOFT_FLOW_CONTROL // we need to do extra checking of remained frames in case there are no unhandled frames left, but pause frame is still undergoing diff --git a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c index 0beeb3b38b..75d173e0f9 100644 --- a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c +++ b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c @@ -691,32 +691,30 @@ static void emac_ksz8851snl_task(void *arg) uint32_t frame_len = ETH_MAX_PACKET_SIZE; uint8_t *buffer; emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len); - /* there is a waiting frame */ - if (frame_len) { - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { - emac_ksz8851_flush_recv_queue(emac); - free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); - } - } else { - ESP_LOGE(TAG, "frame read from module failed"); + /* we have memory to receive the frame of maximal size previously defined */ + if (buffer != NULL) { + uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { emac_ksz8851_flush_recv_queue(emac); free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); } } else { - ESP_LOGE(TAG, "no mem for receive buffer"); + ESP_LOGE(TAG, "frame read from module failed"); emac_ksz8851_flush_recv_queue(emac); + free(buffer); } + /* if allocation failed and there is a waiting frame */ + } else if (frame_len) { + ESP_LOGE(TAG, "no mem for receive buffer"); + emac_ksz8851_flush_recv_queue(emac); } } ksz8851_write_reg(emac, KSZ8851_IER, ier); diff --git a/components/esp_eth/src/esp_eth_mac_w5500.c b/components/esp_eth/src/esp_eth_mac_w5500.c index 2e3d97896a..dc75a1c0c5 100644 --- a/components/esp_eth/src/esp_eth_mac_w5500.c +++ b/components/esp_eth/src/esp_eth_mac_w5500.c @@ -664,30 +664,28 @@ static void emac_w5500_task(void *arg) /* define max expected frame len */ frame_len = ETH_MAX_PACKET_SIZE; emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len); - /* there is a waiting frame */ - if (frame_len) { - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { - free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); - } - } else { - ESP_LOGE(TAG, "frame read from module failed"); + /* we have memory to receive the frame of maximal size previously defined */ + if (buffer != NULL) { + buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); } } else { - ESP_LOGE(TAG, "no mem for receive buffer"); - emac_w5500_flush_recv_frame(emac); + ESP_LOGE(TAG, "frame read from module failed"); + free(buffer); } + /* if allocation failed and there is a waiting frame */ + } else if (frame_len) { + ESP_LOGE(TAG, "no mem for receive buffer"); + emac_w5500_flush_recv_frame(emac); } } while (emac->packets_remain); }