Skip to content

Commit 27d68f5

Browse files
committed
Merge branch 'fix/driver_issue_by_coverity' into 'master'
Fix some false issue report by coverity Closes IDF-13116, IDF-13120, IDF-13104, IDF-13106, IDF-13107, IDF-13109, IDF-13112, IDF-13124, and IDF-13126 See merge request espressif/esp-idf!39287
2 parents 6a57d5f + 7af3bdd commit 27d68f5

File tree

12 files changed

+131
-107
lines changed

12 files changed

+131
-107
lines changed

components/esp_driver_ana_cmpr/ana_cmpr_etm.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,11 @@ static esp_err_t ana_cmpr_del_etm_event(esp_etm_event_handle_t base_event)
2323

2424
esp_err_t ana_cmpr_new_etm_event(ana_cmpr_handle_t cmpr, const ana_cmpr_etm_event_config_t *config, esp_etm_event_handle_t *ret_event)
2525
{
26-
esp_err_t ret = ESP_OK;
27-
ana_cmpr_etm_event_t *event = NULL;
2826
ana_cmpr_unit_t unit = ana_cmpr_get_unit_id(cmpr);
2927
ESP_RETURN_ON_FALSE(((int)unit) >= 0, ESP_ERR_INVALID_ARG, TAG, "invalid analog comparator handle");
3028
ESP_RETURN_ON_FALSE(config && ret_event, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
31-
event = heap_caps_calloc(1, sizeof(ana_cmpr_etm_event_t), ETM_MEM_ALLOC_CAPS);
32-
ESP_GOTO_ON_FALSE(event, ESP_ERR_NO_MEM, err, TAG, "no mem for analog comparator event");
29+
ana_cmpr_etm_event_t *event = heap_caps_calloc(1, sizeof(ana_cmpr_etm_event_t), ETM_MEM_ALLOC_CAPS);
30+
ESP_RETURN_ON_FALSE(event, ESP_ERR_NO_MEM, TAG, "no mem for analog comparator event");
3331

3432
uint32_t event_id = ANALOG_CMPR_LL_ETM_SOURCE(unit, config->event_type);
3533
event->base.del = ana_cmpr_del_etm_event;
@@ -38,11 +36,4 @@ esp_err_t ana_cmpr_new_etm_event(ana_cmpr_handle_t cmpr, const ana_cmpr_etm_even
3836
ESP_LOGD(TAG, "new event @%p, event_id=%"PRIu32", unit_id=%d", event, event_id, unit);
3937
*ret_event = &event->base;
4038
return ESP_OK;
41-
42-
err:
43-
if (event) {
44-
free(event);
45-
event = NULL;
46-
}
47-
return ret;
4839
}

components/esp_driver_bitscrambler/include/driver/bitscrambler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ typedef struct {
5050
/**
5151
* @brief Allocate BitScrambler handle for a hardware channel
5252
*
53+
* @note This function can only be used to create a single direction BitScrambler handle.
54+
* If you need a loopback BitScrambler, call bitscrambler_loopback_create() instead.
55+
*
5356
* @param config Configuration for requested BitScrambler
5457
* @param[out] handle BitScrambler controller handle
5558
*

components/esp_driver_bitscrambler/src/bitscrambler.c

Lines changed: 68 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
#include <string.h>
77
#include <stdatomic.h>
8+
#include "soc/soc_caps.h"
89
#include "esp_log.h"
910
#include "esp_heap_caps.h"
1011
#include "driver/bitscrambler.h"
@@ -20,6 +21,13 @@ static const char *TAG = "bitscrambler";
2021
#define BITSCRAMBLER_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT
2122
#endif
2223

24+
#if !SOC_RCC_IS_INDEPENDENT
25+
// Reset and Clock Control registers are mixing with other peripherals, so we need to use a critical section
26+
#define BS_RCC_ATOMIC() PERIPH_RCC_ATOMIC()
27+
#else
28+
#define BS_RCC_ATOMIC()
29+
#endif
30+
2331
#define BITSCRAMBLER_BINARY_VER 1 //max version we're compatible with
2432
#define BITSCRAMBLER_HW_REV 0
2533

@@ -54,95 +62,92 @@ typedef struct {
5462
atomic_flag tx_in_use = ATOMIC_FLAG_INIT;
5563
atomic_flag rx_in_use = ATOMIC_FLAG_INIT;
5664

57-
// Claim both TX and RX channels for loopback use
58-
// Returns true on success, false if any of the two directions already is claimed.
59-
static bool claim_channel_loopback(void)
60-
{
61-
bool old_val_tx = atomic_flag_test_and_set(&tx_in_use);
62-
if (old_val_tx) {
63-
return false;
64-
}
65-
bool old_val_rx = atomic_flag_test_and_set(&rx_in_use);
66-
if (old_val_rx) {
67-
atomic_flag_clear(&tx_in_use);
68-
return false;
69-
}
70-
return true;
71-
}
65+
// This is a reference count for the BitScrambler module. It is used to keep track of how many clients are using the module.
66+
atomic_int group_ref_count = 0;
7267

7368
// Claim a channel using the direction it indicated.
7469
// Returns true on success, false if the direction already is claimed
7570
static bool claim_channel(bitscrambler_direction_t dir)
7671
{
72+
int old_use_count = atomic_fetch_add(&group_ref_count, 1);
73+
if (old_use_count == 0) {
74+
BS_RCC_ATOMIC() {
75+
// This is the first client using the module, so we need to enable the sys clock
76+
bitscrambler_ll_set_bus_clock_sys_enable(true);
77+
bitscrambler_ll_reset_sys();
78+
// also power on the memory
79+
bitscrambler_ll_mem_power_by_pmu();
80+
}
81+
}
7782
if (dir == BITSCRAMBLER_DIR_TX) {
7883
bool old_val = atomic_flag_test_and_set(&tx_in_use);
7984
if (old_val) {
80-
return false;
85+
goto err;
86+
} else {
87+
BS_RCC_ATOMIC() {
88+
bitscrambler_ll_set_bus_clock_tx_enable(true);
89+
bitscrambler_ll_reset_tx();
90+
}
8191
}
82-
} else if (dir == BITSCRAMBLER_DIR_RX) {
92+
} else {
8393
bool old_val = atomic_flag_test_and_set(&rx_in_use);
8494
if (old_val) {
85-
return false;
95+
goto err;
96+
} else {
97+
BS_RCC_ATOMIC() {
98+
bitscrambler_ll_set_bus_clock_rx_enable(true);
99+
bitscrambler_ll_reset_rx();
100+
}
86101
}
87102
}
88103
return true;
104+
err:
105+
atomic_fetch_sub(&group_ref_count, 1);
106+
return false;
89107
}
90108

91-
//Initialize the BitScrambler object and hardware using the given config.
92-
static esp_err_t init_from_config(bitscrambler_t *bs, const bitscrambler_config_t *config)
93-
{
94-
bs->cfg = *config; //Copy config over
95-
bs->hw = BITSCRAMBLER_LL_GET_HW(0); //there's only one as of now; if there's more, we need to handle them as a pool.
96-
return ESP_OK;
97-
}
98-
99-
static void enable_clocks(bitscrambler_t *bs)
109+
// Release the channel using the direction it indicated.
110+
static void release_channel(bitscrambler_direction_t dir)
100111
{
101-
PERIPH_RCC_ACQUIRE_ATOMIC(PERIPH_BITSCRAMBLER_MODULE, ref_count) {
102-
if (ref_count == 0) { //we're the first to enable the BitScrambler module
103-
bitscrambler_ll_set_bus_clock_sys_enable(1);
104-
bitscrambler_ll_reset_sys();
105-
bitscrambler_ll_mem_power_by_pmu();
106-
}
107-
if (bs->cfg.dir == BITSCRAMBLER_DIR_RX || bs->loopback) {
108-
bitscrambler_ll_set_bus_clock_rx_enable(1);
109-
bitscrambler_ll_reset_rx();
110-
}
111-
if (bs->cfg.dir == BITSCRAMBLER_DIR_TX || bs->loopback) {
112-
bitscrambler_ll_set_bus_clock_tx_enable(1);
113-
bitscrambler_ll_reset_tx();
112+
if (dir == BITSCRAMBLER_DIR_TX) {
113+
atomic_flag_clear(&tx_in_use);
114+
} else if (dir == BITSCRAMBLER_DIR_RX) {
115+
atomic_flag_clear(&rx_in_use);
116+
}
117+
int old_use_count = atomic_fetch_sub(&group_ref_count, 1);
118+
if (old_use_count == 1) {
119+
// This is the last client using the module, so we need to disable the sys clock
120+
BS_RCC_ATOMIC() {
121+
bitscrambler_ll_set_bus_clock_sys_enable(false);
122+
bitscrambler_ll_mem_force_power_off();
114123
}
115124
}
116125
}
117126

118-
static void disable_clocks(bitscrambler_t *bs)
127+
//Initialize the BitScrambler object and hardware using the given config.
128+
static esp_err_t init_from_config(bitscrambler_t *bs, const bitscrambler_config_t *config)
119129
{
120-
PERIPH_RCC_RELEASE_ATOMIC(PERIPH_BITSCRAMBLER_MODULE, ref_count) {
121-
if (bs->cfg.dir == BITSCRAMBLER_DIR_RX || bs->loopback) {
122-
bitscrambler_ll_set_bus_clock_rx_enable(0);
123-
}
124-
if (bs->cfg.dir == BITSCRAMBLER_DIR_TX || bs->loopback) {
125-
bitscrambler_ll_set_bus_clock_tx_enable(0);
126-
}
127-
if (ref_count == 0) { //we're the last to disable the BitScrambler module
128-
bitscrambler_ll_set_bus_clock_sys_enable(0);
129-
bitscrambler_ll_mem_force_power_off();
130-
}
131-
}
130+
bs->cfg = *config; //Copy config over
131+
bs->hw = BITSCRAMBLER_LL_GET_HW(0); //there's only one as of now; if there's more, we need to handle them as a pool.
132+
return ESP_OK;
132133
}
133134

134-
//Private function: init an existing BitScrambler object as a loopback BitScrambler.
135+
// init an existing BitScrambler object as a loopback BitScrambler, only used by the bitscrambler loopback driver
135136
esp_err_t bitscrambler_init_loopback(bitscrambler_handle_t handle, const bitscrambler_config_t *config)
136137
{
137-
if (!claim_channel_loopback()) {
138+
// claim the TX channel first
139+
if (!claim_channel(BITSCRAMBLER_DIR_TX)) {
140+
return ESP_ERR_NOT_FOUND;
141+
}
142+
// claim the RX channel, if it fails, release the TX channel
143+
if (!claim_channel(BITSCRAMBLER_DIR_RX)) {
144+
release_channel(BITSCRAMBLER_DIR_TX);
138145
return ESP_ERR_NOT_FOUND;
139146
}
140147

141-
assert(config->dir == BITSCRAMBLER_DIR_TX);
148+
// mark the BitScrambler object as a loopback BitScrambler
142149
handle->loopback = true;
143-
enable_clocks(handle);
144-
esp_err_t r = init_from_config(handle, config);
145-
return r;
150+
return init_from_config(handle, config);
146151
}
147152

148153
esp_err_t bitscrambler_new(const bitscrambler_config_t *config, bitscrambler_handle_t *handle)
@@ -172,8 +177,6 @@ esp_err_t bitscrambler_new(const bitscrambler_config_t *config, bitscrambler_han
172177
return r;
173178
}
174179

175-
enable_clocks(bs);
176-
177180
// Return the handle
178181
*handle = bs;
179182
return ESP_OK;
@@ -304,14 +307,11 @@ void bitscrambler_free(bitscrambler_handle_t handle)
304307
if (!handle) {
305308
return;
306309
}
307-
disable_clocks(handle);
308310
if (handle->loopback) {
309-
atomic_flag_clear(&tx_in_use);
310-
atomic_flag_clear(&rx_in_use);
311-
} else if (handle->cfg.dir == BITSCRAMBLER_DIR_TX) {
312-
atomic_flag_clear(&tx_in_use);
313-
} else if (handle->cfg.dir == BITSCRAMBLER_DIR_RX) {
314-
atomic_flag_clear(&rx_in_use);
311+
release_channel(BITSCRAMBLER_DIR_TX);
312+
release_channel(BITSCRAMBLER_DIR_RX);
313+
} else {
314+
release_channel(handle->cfg.dir);
315315
}
316316
if (handle->extra_clean_up) {
317317
handle->extra_clean_up(handle, handle->clean_up_user_ctx);

components/esp_driver_parlio/src/parlio_rx.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,7 @@ esp_err_t parlio_rx_unit_enable(parlio_rx_unit_handle_t rx_unit, bool reset_queu
740740
parlio_ll_rx_enable_clock(hal->regs, false);
741741
}
742742
}
743+
assert(trans.delimiter);
743744
parlio_rx_set_delimiter_config(rx_unit, trans.delimiter);
744745
parlio_rx_mount_transaction_buffer(rx_unit, &trans);
745746
gdma_start(rx_unit->dma_chan, (intptr_t)rx_unit->curr_desc);

components/esp_driver_ppa/src/ppa_blend.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,8 @@ esp_err_t ppa_do_blend(ppa_client_handle_t ppa_client, const ppa_blend_oper_conf
258258

259259
esp_err_t ret = ESP_OK;
260260
ppa_trans_t *trans_elm = NULL;
261-
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0)) {
261+
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0) == pdTRUE) {
262+
assert(trans_elm);
262263
dma2d_trans_config_t *dma_trans_desc = trans_elm->trans_desc;
263264

264265
ppa_dma2d_trans_on_picked_config_t *trans_on_picked_desc = dma_trans_desc->user_config;

components/esp_driver_ppa/src/ppa_fill.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ esp_err_t ppa_do_fill(ppa_client_handle_t ppa_client, const ppa_fill_oper_config
113113

114114
esp_err_t ret = ESP_OK;
115115
ppa_trans_t *trans_elm = NULL;
116-
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0)) {
116+
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0) == pdTRUE) {
117+
assert(trans_elm);
117118
dma2d_trans_config_t *dma_trans_desc = trans_elm->trans_desc;
118119

119120
ppa_dma2d_trans_on_picked_config_t *trans_on_picked_desc = dma_trans_desc->user_config;

components/esp_driver_ppa/src/ppa_srm.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ esp_err_t ppa_do_scale_rotate_mirror(ppa_client_handle_t ppa_client, const ppa_s
252252

253253
esp_err_t ret = ESP_OK;
254254
ppa_trans_t *trans_elm = NULL;
255-
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0)) {
255+
if (xQueueReceive(ppa_client->trans_elm_ptr_queue, (void *)&trans_elm, 0) == pdTRUE) {
256+
assert(trans_elm);
256257
dma2d_trans_config_t *dma_trans_desc = trans_elm->trans_desc;
257258

258259
ppa_dma2d_trans_on_picked_config_t *trans_on_picked_desc = dma_trans_desc->user_config;

components/esp_driver_touch_sens/common/touch_sens_common.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ touch_sensor_handle_t g_touch = NULL;
4242
static void touch_channel_pin_init(int id)
4343
{
4444
gpio_num_t pin = touch_sensor_channel_io_map[id];
45+
assert(pin >= 0);
4546
if (esp_gpio_reserve(BIT64(pin)) & BIT64(pin)) {
4647
ESP_LOGW(TAG, "The GPIO%d is conflict with other module", (int)pin);
4748
}

components/esp_driver_touch_sens/hw_ver1/touch_version_specific.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ esp_err_t touch_priv_config_controller(touch_sensor_handle_t sens_handle, const
198198
sens_handle->sample_cfg_num = 1; // Only have one set of sampling configuration
199199

200200
/* Configure the hardware */
201+
assert(hal_cfg.sample_cfg_num == 1);
201202
TOUCH_ENTER_CRITICAL(TOUCH_PERIPH_LOCK);
202203
touch_hal_config_controller(&hal_cfg);
203204
touch_ll_reset_trigger_groups();

components/esp_driver_uart/src/uhci.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ static bool uhci_gdma_tx_callback_eof(gdma_channel_handle_t dma_chan, gdma_event
8888
expected_fsm = UHCI_TX_FSM_ENABLE;
8989
if (atomic_compare_exchange_strong(&uhci_ctrl->tx_dir.tx_fsm, &expected_fsm, UHCI_TX_FSM_RUN_WAIT)) {
9090
if (xQueueReceiveFromISR(uhci_ctrl->tx_dir.trans_queues[UHCI_TRANS_QUEUE_PROGRESS], &trans_desc, &do_yield) == pdTRUE) {
91+
// sanity check
92+
assert(trans_desc);
9193
atomic_store(&uhci_ctrl->tx_dir.tx_fsm, UHCI_TX_FSM_RUN);
9294
uhci_do_transmit(uhci_ctrl, trans_desc);
9395
if (do_yield) {
@@ -219,7 +221,7 @@ static esp_err_t uhci_gdma_initialize(uhci_controller_handle_t uhci_ctrl, const
219221
ESP_RETURN_ON_ERROR(gdma_new_link_list(&dma_link_config, &uhci_ctrl->rx_dir.dma_link), TAG, "DMA rx link list alloc failed");
220222
ESP_LOGD(TAG, "rx_dma node number is %d", uhci_ctrl->rx_dir.rx_num_dma_nodes);
221223

222-
uhci_ctrl->rx_dir.buffer_size_per_desc_node = heap_caps_calloc(uhci_ctrl->rx_dir.rx_num_dma_nodes, sizeof(uhci_ctrl->rx_dir.buffer_size_per_desc_node), UHCI_MEM_ALLOC_CAPS);
224+
uhci_ctrl->rx_dir.buffer_size_per_desc_node = heap_caps_calloc(uhci_ctrl->rx_dir.rx_num_dma_nodes, sizeof(*uhci_ctrl->rx_dir.buffer_size_per_desc_node), UHCI_MEM_ALLOC_CAPS);
223225
ESP_RETURN_ON_FALSE(uhci_ctrl->rx_dir.buffer_size_per_desc_node, ESP_ERR_NO_MEM, TAG, "no memory for recording buffer size for desc node");
224226
uhci_ctrl->rx_dir.buffer_pointers = heap_caps_calloc(uhci_ctrl->rx_dir.rx_num_dma_nodes, sizeof(*uhci_ctrl->rx_dir.buffer_pointers), UHCI_MEM_ALLOC_CAPS);
225227
ESP_RETURN_ON_FALSE(uhci_ctrl->rx_dir.buffer_pointers, ESP_ERR_NO_MEM, TAG, "no memory for recording buffer pointers for desc node");

0 commit comments

Comments
 (0)