Fix for #4565 (rx fifo length), protect access to rx_buffer #4568
Conversation
cores/esp8266/uart.c
Outdated
| return uart_rx_buffer_available(uart) + uart_rx_fifo_available(uart); | ||
| ETS_UART_INTR_DISABLE(); | ||
| int uartrxbufferavailable = uart_rx_buffer_available(uart); | ||
| ETS_UART_INTR_ENABLE(); |
There was a problem hiding this comment.
Nice, for more clarity, what about renaming uart_rx_buffer_available to uart_rx_buffer_available_unprotected ?
cores/esp8266/uart.c
Outdated
| if (!uart_rx_available(uart)) { | ||
| ETS_UART_INTR_ENABLE(); |
There was a problem hiding this comment.
uart_rx_available() also conflictly calls INTR_DISABLE+ENABLE. Maybe adding a new static uart_rx_available_unprotected() would help (L82 too)
There was a problem hiding this comment.
You're right, this needs rework.
cores/esp8266/uart.c
Outdated
| @@ -140,15 +140,17 @@ int uart_peek_char(uart_t* uart) | |||
| if(uart == NULL || !uart->rx_enabled) { | |||
There was a problem hiding this comment.
I can't comment right next to uart_rx_copy_fifo_to_buffer(). For more clarity, I guess it could be renamed to uart_rx_copy_fifo_to_buffer_unprotected() too.
|
Alright, did a more thorough rework of the file, including some refactoring and styling. It's gonna be a bit hard to understand some of the changes, but basically I put all the unsafe functions together, factored out some of the uart register magic to unify the relevant code, tracked all the function entrypoints to make sure there are no uart interrupt double enable/disables, and added a dab of constness. |
|
@d-a-v looks like CI builds now. I have no idea why my local workspace built before. |
…sp8266#4568) * Fix for esp8266#4565 (rx fifo length), protect access to rx_buffer * Fix typo * reworked to separate safe from unsafe functions, factorized some code, constness * additional rework for uart_rx_fifo_available() * swapped unsafe function definition order * Remove static for overrun string * Some shorthand for perf and readability (cherry picked from commit 29580e8)
Fixes #4565 , and protects access to rx_buffer by disabling/enabling the uart interrupt.