Merge branch 'fix/no_function_call_in_min_max_v6.0' into 'release/v6.0'

refactor: avoid function calls inside MIN/MAX macros (v6.0)

See merge request espressif/esp-idf!43476

components/efuse/src/esp_efuse_fields.c

@@ -44,7 +44,8 @@ void esp_efuse_reset(void)
 uint32_t esp_efuse_read_secure_version(void)
 {
     uint32_t secure_version = 0;
-    size_t size = MIN(APP_SEC_VER_SIZE_EFUSE_FIELD, esp_efuse_get_field_size(ESP_EFUSE_SECURE_VERSION));
+    size_t field_size = esp_efuse_get_field_size(ESP_EFUSE_SECURE_VERSION);
+    size_t size = MIN(APP_SEC_VER_SIZE_EFUSE_FIELD, field_size);
     esp_efuse_read_field_blob(ESP_EFUSE_SECURE_VERSION, &secure_version, size);
     return __builtin_popcount(secure_version & ((1ULL << size) - 1));
 }
@@ -66,7 +67,8 @@ bool esp_efuse_check_secure_version(uint32_t secure_version)
 
 esp_err_t esp_efuse_update_secure_version(uint32_t secure_version)
 {
-    size_t size = MIN(APP_SEC_VER_SIZE_EFUSE_FIELD, esp_efuse_get_field_size(ESP_EFUSE_SECURE_VERSION));
+    size_t field_size = esp_efuse_get_field_size(ESP_EFUSE_SECURE_VERSION);
+    size_t size = MIN(APP_SEC_VER_SIZE_EFUSE_FIELD, field_size);
     if (size < secure_version) {
         ESP_LOGE(TAG, "Max secure version is %u. Given %"PRIu32" version can not be written.", (unsigned)size, secure_version);
         return ESP_ERR_INVALID_ARG;

components/esp_driver_i2c/i2c_master.c

@@ -190,7 +190,8 @@ static bool s_i2c_write_command(i2c_master_bus_handle_t i2c_master, i2c_operatio
     uint8_t data_fill = 0;
 
     // data_fill refers to the length to fill the data
-    data_fill = MIN(remaining_bytes, I2C_FIFO_LEN(i2c_master->base->port_num) - *address_fill);
+    uint32_t fifo_len = I2C_FIFO_LEN(i2c_master->base->port_num);
+    data_fill = MIN(remaining_bytes, fifo_len - *address_fill);
     write_pr = i2c_operation->data + i2c_operation->bytes_used;
     i2c_operation->bytes_used += data_fill;
     hw_cmd.byte_num = data_fill + *address_fill;
@@ -286,7 +287,8 @@ static bool s_i2c_read_command(i2c_master_bus_handle_t i2c_master, i2c_operation
     i2c_bus_handle_t handle = i2c_master->base;
     i2c_ll_hw_cmd_t hw_cmd = i2c_operation->hw_cmd;
 
-    *fifo_fill = MIN(remaining_bytes, I2C_FIFO_LEN(i2c_master->base->port_num) - i2c_master->read_len_static);
+    uint32_t fifo_len = I2C_FIFO_LEN(i2c_master->base->port_num);
+    *fifo_fill = MIN(remaining_bytes, fifo_len - i2c_master->read_len_static);
     i2c_master->rx_cnt = *fifo_fill;
     hw_cmd.byte_num = *fifo_fill;
 

components/esp_driver_i2s/lp_i2s.c

@@ -169,7 +169,8 @@ esp_err_t lp_i2s_channel_read(lp_i2s_chan_handle_t chan, lp_i2s_trans_t *trans,
         return ESP_ERR_TIMEOUT;
     }
 
-    size_t len = MIN(lp_i2s_ll_get_rx_mem_fifo_cnt(chan->ctlr->hal.dev), trans->buflen);
+    size_t fifo_cnt = lp_i2s_ll_get_rx_mem_fifo_cnt(chan->ctlr->hal.dev);
+    size_t len = MIN(fifo_cnt, trans->buflen);
     lp_i2s_ll_read_buffer(chan->ctlr->hal.dev, trans->buffer, len);
     trans->received_size = len;
     portENTER_CRITICAL(&g_i2s.spinlock);
@@ -305,7 +306,8 @@ static void IRAM_ATTR s_i2s_default_isr(void *arg)
     ESP_DRAM_LOGD(TAG, "in isr, rx_mem_fifo_cnt: %d bytes", lp_i2s_ll_get_rx_mem_fifo_cnt(ctlr->hal.dev));
 
     if (ctlr->rx_chan->cbs.on_request_new_trans) {
-        size_t len = MIN(lp_i2s_ll_get_rx_mem_fifo_cnt(ctlr->hal.dev), ctlr->rx_chan->trans.buflen);
+        size_t fifo_cnt = lp_i2s_ll_get_rx_mem_fifo_cnt(ctlr->hal.dev);
+        size_t len = MIN(fifo_cnt, ctlr->rx_chan->trans.buflen);
         ESP_DRAM_LOGD(TAG, "len: %d", len);
         lp_i2s_ll_read_buffer(ctlr->hal.dev, ctlr->rx_chan->trans.buffer, len);
         lp_i2s_ll_rx_clear_interrupt_status(ctlr->hal.dev, LP_I2S_LL_EVENT_RX_MEM_THRESHOLD_INT);

components/esp_driver_ledc/src/ledc.c

@@ -1192,11 +1192,13 @@ uint32_t ledc_find_suitable_duty_resolution(uint32_t src_clk_freq, uint32_t time
 {
     // Highest resolution is calculated when LEDC_CLK_DIV = 1 (i.e. div_param = 1 << LEDC_LL_FRACTIONAL_BITS)
     uint32_t div = (src_clk_freq + timer_freq / 2) / timer_freq; // rounded
-    uint32_t duty_resolution = MIN(ilog2(div), SOC_LEDC_TIMER_BIT_WIDTH);
+    uint32_t ilog2_div = ilog2(div);
+    uint32_t duty_resolution = MIN(ilog2_div, SOC_LEDC_TIMER_BIT_WIDTH);
     uint32_t div_param = ledc_calculate_divisor(src_clk_freq, timer_freq, 1 << duty_resolution);
     if (LEDC_IS_DIV_INVALID(div_param)) {
         div = src_clk_freq / timer_freq; // truncated
-        duty_resolution = MIN(ilog2(div), SOC_LEDC_TIMER_BIT_WIDTH);
+        ilog2_div = ilog2(div);
+        duty_resolution = MIN(ilog2_div, SOC_LEDC_TIMER_BIT_WIDTH);
         div_param = ledc_calculate_divisor(src_clk_freq, timer_freq, 1 << duty_resolution);
         if (LEDC_IS_DIV_INVALID(div_param)) {
             duty_resolution = 0;

components/esp_driver_uart/src/uart.c

@@ -1618,7 +1618,8 @@ static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool
         }
         xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void *) &evt, sizeof(uart_tx_data_t), portMAX_DELAY);
         while (size > 0) {
-            size_t send_size = MIN(size, xRingbufferGetCurFreeSize(p_uart_obj[uart_num]->tx_ring_buf));
+            size_t free_size = xRingbufferGetCurFreeSize(p_uart_obj[uart_num]->tx_ring_buf);
+            size_t send_size = MIN(size, free_size);
             xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void *)(src + offset), send_size, portMAX_DELAY);
             size -= send_size;
             offset += send_size;

components/esp_hw_support/dma/gdma.c

@@ -198,6 +198,7 @@ err:
     if (alloc_rx_channel) {
         free(alloc_rx_channel);
     }
+    // coverity[dead_error_condition] - defensive check for code maintainability
     if (pair) {
         gdma_release_pair_handle(pair);
         // pair release will also release group if it's the last reference

components/esp_hw_support/esp_clk.c

@@ -87,7 +87,8 @@ int IRAM_ATTR esp_clk_apb_freq(void)
 {
     // TODO: IDF-5173 Require cleanup, implementation should be unified
 #if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32C2
-    return MIN(s_get_cpu_freq_mhz() * MHZ, APB_CLK_FREQ);
+    uint32_t cpu_freq_hz = s_get_cpu_freq_mhz() * MHZ;
+    return MIN(cpu_freq_hz, APB_CLK_FREQ);
 #else // for all later targets
     return rtc_clk_apb_freq_get();
 #endif

components/esp_hw_support/sleep_modes.c

@@ -1578,9 +1578,9 @@ esp_err_t esp_light_sleep_start(void)
         *    min_protect(2);
         * 4. All the adjustment time which is s_config.sleep_time_adjustment below.
         */
+        uint32_t rtc_slowclk_us = rtc_time_slowclk_to_us(RTC_MODULE_SLEEP_PREPARE_CYCLES + RTC_CNTL_MIN_SLP_VAL_MIN, s_config.rtc_clk_cal_period);
         const uint32_t vddsdio_pd_sleep_duration = MAX(FLASH_PD_MIN_SLEEP_TIME_US,
-                        flash_enable_time_us + s_config.sleep_time_adjustment
-                        + rtc_time_slowclk_to_us(RTC_MODULE_SLEEP_PREPARE_CYCLES + RTC_CNTL_MIN_SLP_VAL_MIN, s_config.rtc_clk_cal_period));
+                        flash_enable_time_us + s_config.sleep_time_adjustment + rtc_slowclk_us);
 
         if (can_power_down_vddsdio(sleep_flags, vddsdio_pd_sleep_duration)) {
             if (s_config.sleep_time_overhead_out < flash_enable_time_us) {

components/esp_system/port/arch/xtensa/debug_helpers.c

@@ -192,7 +192,8 @@ esp_err_t ESP_SYSTEM_IRAM_ATTR esp_backtrace_print_all_tasks(int depth)
                             &ctrl.cur_tasks[core_id].next_pc);
 
     // Get snapshot of all tasks in the system
-    const UBaseType_t num_snapshots = MIN(num_tasks, uxTaskGetSnapshotAll(task_snapshots, num_tasks, NULL));
+    UBaseType_t snapshot_count = uxTaskGetSnapshotAll(task_snapshots, num_tasks, NULL);
+    const UBaseType_t num_snapshots = MIN(num_tasks, snapshot_count);
     // Print the backtrace of every task in the system
     for (UBaseType_t task_idx = 0; task_idx < num_snapshots; task_idx++) {
         bool cur_running = false;

components/esp_timer/src/esp_timer.c

@@ -161,7 +161,8 @@ esp_err_t ESP_TIMER_IRAM_ATTR esp_timer_restart(esp_timer_handle_t timer, uint64
          * - if the alarm was a one-shot one, i.e. `period` is 0, it remains non-periodic. */
         if (period != 0) {
             /* Remove function got rid of the alarm and period fields, restore them */
-            const uint64_t new_period = MAX(timeout_us, esp_timer_impl_get_min_period_us());
+            const uint64_t min_period = esp_timer_impl_get_min_period_us();
+            const uint64_t new_period = MAX(timeout_us, min_period);
             timer->alarm = now + new_period;
             timer->period = new_period;
         } else {
@@ -218,7 +219,8 @@ esp_err_t ESP_TIMER_IRAM_ATTR esp_timer_start_periodic(esp_timer_handle_t timer,
     if (!is_initialized()) {
         return ESP_ERR_INVALID_STATE;
     }
-    period_us = MAX(period_us, esp_timer_impl_get_min_period_us());
+    uint64_t min_period = esp_timer_impl_get_min_period_us();
+    period_us = MAX(period_us, min_period);
     int64_t alarm = esp_timer_get_time() + period_us;
     esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD;
     esp_err_t err;

components/esp_timer/test_apps/main/test_esp_timer.c

@@ -457,7 +457,8 @@ static void timer_test_monotonic_values_task(void* arg)
             state->pass = false;
         }
         state->avg_diff += diff;
-        state->max_error = MAX(state->max_error, llabs(diff));
+        int64_t abs_diff = llabs(diff);
+        state->max_error = MAX(state->max_error, abs_diff);
         state->test_cnt++;
     }
     state->avg_diff /= state->test_cnt;

components/esp_usb_cdc_rom_console/vfs_cdcacm.c

@@ -191,7 +191,8 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size)
     } else {
         /* process pending interrupts before requesting available data */
         esp_usb_console_poll_interrupts();
-        size = MIN(size, cdcacm_data_length_in_buffer());
+        size_t available = cdcacm_data_length_in_buffer();
+        size = MIN(size, available);
     }
 
     while (received < size) {

components/hal/systimer_hal.c

@@ -92,7 +92,8 @@ void systimer_hal_set_alarm_target(systimer_hal_context_t *hal, uint32_t alarm_i
 {
     int64_t offset = hal->us_to_ticks(1) * 2;
     uint64_t now_time = systimer_hal_get_counter_value(hal, 0);
-    systimer_counter_value_t alarm = { .val = MAX(hal->us_to_ticks(timestamp), now_time + offset) };
+    uint64_t timestamp_ticks = hal->us_to_ticks(timestamp);
+    systimer_counter_value_t alarm = { .val = MAX(timestamp_ticks, now_time + offset) };
     do {
         systimer_ll_enable_alarm(hal->dev, alarm_id, false);
         systimer_ll_set_alarm_target(hal->dev, alarm_id, alarm.val);

components/mbedtls/port/aes/dma/esp_aes_dma_core.c

@@ -244,8 +244,10 @@ static int esp_aes_process_dma_ext_ram(esp_aes_context *ctx, const unsigned char
 #ifdef SOC_GDMA_EXT_MEM_ENC_ALIGNMENT
     if (efuse_hal_flash_encryption_enabled()) {
         if (esp_ptr_external_ram(input) || esp_ptr_external_ram(output) || esp_ptr_in_drom(input) || esp_ptr_in_drom(output)) {
-            input_alignment = MAX(get_cache_line_size(input), SOC_GDMA_EXT_MEM_ENC_ALIGNMENT);
-            output_alignment = MAX(get_cache_line_size(output), SOC_GDMA_EXT_MEM_ENC_ALIGNMENT);
+            size_t input_cache_line_size = get_cache_line_size(input);
+            size_t output_cache_line_size = get_cache_line_size(output);
+            input_alignment = MAX(input_cache_line_size, SOC_GDMA_EXT_MEM_ENC_ALIGNMENT);
+            output_alignment = MAX(output_cache_line_size, SOC_GDMA_EXT_MEM_ENC_ALIGNMENT);
 
             input_heap_caps = MALLOC_CAP_8BIT | (esp_ptr_external_ram(input) ? MALLOC_CAP_SPIRAM : MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL);
             output_heap_caps = MALLOC_CAP_8BIT | (esp_ptr_external_ram(output) ? MALLOC_CAP_SPIRAM : MALLOC_CAP_DMA | MALLOC_CAP_INTERNAL);

components/spi_flash/spi_flash_os_func_app.c

@@ -217,7 +217,8 @@ static void* get_buffer_malloc(void* arg, size_t reqest_size, size_t* out_size)
     unsigned retries = 5;
     size_t read_chunk_size = reqest_size;
     while(ret == NULL && retries--) {
-        read_chunk_size = MIN(read_chunk_size, heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT));
+        size_t largest_free = heap_caps_get_largest_free_block(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
+        read_chunk_size = MIN(read_chunk_size, largest_free);
         read_chunk_size = (read_chunk_size + 3) & ~3;
         ret = heap_caps_malloc(read_chunk_size, MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
     }

tools/ci/sg_rules/no_function_call_in_min_max_macro.yml

@@ -0,0 +1,37 @@
+id: no-function-call-in-min-max
+message: "Avoid calling functions inside MIN/MAX macros"
+severity: error
+note: "MIN/MAX macros may evaluate arguments multiple times. Store function results in a variable first."
+language: C
+files:
+  - "components/**/*"
+ignores:
+  - "components/bt/**/*"
+  - "components/spiffs/**/*"
+rule:
+  kind: call_expression
+  pattern: $FUNC($ARG1, $ARG2)
+  any:
+    - has:
+        kind: call_expression
+        field: arguments
+        stopBy: end
+        position: 0
+        not:
+          has:
+            kind: identifier
+            regex: '^(MIN|MAX)$'
+            field: function
+    - has:
+        kind: call_expression
+        field: arguments
+        stopBy: end
+        position: 1
+        not:
+          has:
+            kind: identifier
+            regex: '^(MIN|MAX)$'
+            field: function
+constraints:
+  FUNC:
+    regex: '^(MIN|MAX)$'