avr-uart
avr-uart copied to clipboard
Buffer Indexes are not atomic when uint16_t(LARGE_BUFFER)
I am using a translator, so please understand. Thank you sir.
change the getc() code location
The code below is only applied to uart1, so it seems like it should be applied to other codes as well.
/* uart1_getc */
ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
if (UART1_RxHead == UART1_RxTail) {
return UART_NO_DATA; /* no data available */
}
/* calculate / store buffer index */
tmptail = (UART1_RxTail + 1) & UART_RX1_BUFFER_MASK;
UART1_RxTail = tmptail;
}
/* uart0_getc */
ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
if (UART_RxHead == UART_RxTail) {
return UART_NO_DATA; /* no data available */
}
}
/* calculate / store buffer index */
tmptail = (UART_RxTail + 1) & UART_RX0_BUFFER_MASK;
UART_RxTail = tmptail; // <- This is not atomic when LARGE_BUFFER.
I think it would be better to do something like below. It was also applied to peek.
/* uart1_getc */
#ifdef USART1_LARGE_BUFFER
ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
#endif
{
if (UART1_RxHead == UART1_RxTail) {
return UART_NO_DATA; /* no data available */
}
/* calculate / store buffer index */
tmptail = (UART1_RxTail + 1) & UART_RX1_BUFFER_MASK;
UART1_RxTail = tmptail;
}
putc atomic
When LARGE_BUFFER
, UART1_TxHead
is uint16_t
. So the code below is not atomic.
UART1_TxHead = tmphead;
I think it would be good to change it atomically as follows.
#ifdef USART1_LARGE_BUFFER
ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
#endif
{
UART1_TxHead = tmphead;
}
There are a lot of duplicates, so I think it would be better to do something like this:
#ifdef USART0_LARGE_BUFFER
#define UART0_ATOMIC_BLOCK ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
#else
#define UART0_ATOMIC_BLOCK
#endif
If you agree with this, I'll put in a pull request.