CMSIS Driver v1.10 UART_STM32F4xx.c Bug

CMSIS Driver UART Revision: V1.02

file C:\Keil\ARM\Pack\Keil\STM32F4xx_DFP\1.0.8\RTE_Driver\UART_STM32F4xx

function UART_ReadData

static int32_t UART_ReadData (uint8_t         *data,
                              uint32_t         size,
                              UART_RESOURCES  *ptr_uart) {
  volatile uint32_t val;

  osMutexWait(ptr_uart->info->mutex_read, osWaitForever);

  if ((size == 0) || (ptr_uart->info->rx.empty == true)) {
    osMutexRelease(ptr_uart->info->mutex_read);
    return 0;
  }

  val = (ptr_uart->info->rx.idx_in - ptr_uart->info->rx.idx_out) & (ptr_uart->rx_buffer_size - 1);
  if (val == 0) val = ptr_uart->rx_buffer_size;

  if (size > val) size = val;

  if ((ptr_uart->hw_flow == true) && (ptr_uart->manual_rts_cts == false)) {
    if (ptr_uart->flow_ctrl_threshold < val)
      GPIO_PinWrite(ptr_uart->pin_rts.port, ptr_uart->pin_rts.num, 0);
  }

  if (size > (ptr_uart->rx_buffer_size - ptr_uart->info->rx.idx_out)) {
    memcpy(data, ptr_uart->rx_buffer + ptr_uart->info->rx.idx_out, val);
   
    memcpy(data + val, ptr_uart->rx_buffer, size - val);
   

    if (ptr_uart->info->rx.idx_in == (size - val))
      ptr_uart->info->rx.empty = true;

    ptr_uart->info->rx.idx_out = size - val;
  } else {
    memcpy(data, ptr_uart->rx_buffer + ptr_uart->info->rx.idx_out, size);
    val = (ptr_uart->info->rx.idx_out + size) & (ptr_uart->rx_buffer_size - 1);
    if (ptr_uart->info->rx.idx_in == val)
      ptr_uart->info->rx.empty = true;

    ptr_uart->info->rx.idx_out = val;
  }

Guess what will be if variable size is less than val?

Parents
  • It's called production-ready code.

    Someone else have already set up the test bench and walked through all corner cases.

    Or maybe not...

    But the code is at least well documented with source code comments.

    And I like a UART driver that not only uses a ring buffer with individual rx and tx pointers. But also adds a separate flag "empty". All to make sure that it suddenly needs to use a mutex for synchronization. And here I thought people used ring buffers because it separated the ownership of the rx and tx pointers, allowing the producer and consumer to access the ring buffer without any locking.

Reply
  • It's called production-ready code.

    Someone else have already set up the test bench and walked through all corner cases.

    Or maybe not...

    But the code is at least well documented with source code comments.

    And I like a UART driver that not only uses a ring buffer with individual rx and tx pointers. But also adds a separate flag "empty". All to make sure that it suddenly needs to use a mutex for synchronization. And here I thought people used ring buffers because it separated the ownership of the rx and tx pointers, allowing the producer and consumer to access the ring buffer without any locking.

Children
More questions in this forum