You may use this one if you find more problems.
I did not, but the the new trendy thing did ... Not trying to paraphrase, I copy paste here the finding :
• Findings (ordered by severity)
- Critical: sioAsyncReadX / sioAsyncWriteX macro argument order is reversed.
- sio_lld_read / sio_lld_write signatures are (SIODriver*, uint8_t *buffer, size_t n) but macros pass (siop, size, buffer), so callers pass a pointer as size and size_t as buffer.
- File: ChibiOS_21.11_stable/os/hal/include/hal_sio.h:437 and :453.
- High: bsioStart() doesn’t restore siop->arg after sioStop() clears it.
- sioStop() sets siop->arg = NULL (hal_sio.c:301–305).
- bsioStart() only sets the callback, not arg (hal_buffered_sio.c:202–207).
- __bsio_default_cb() dereferences siop->arg without a NULL check (hal_buffered_sio.c:70–72).
- Result: stop → start → first IRQ dereferences NULL in ISR.
- Files: hal_sio.c:301–305, hal_buffered_sio.c:70–72, hal_buffered_sio.c:202–207.
- Low: extension field guards are misspelled (*_FIELS).
- SIO_CONFIG_EXT_FIELS / SIO_DRIVER_EXT_FIELS won’t match the expected *_FIELDS macro name, so user extensions are never included.
- File: hal_sio.h:171–173, :243–245.
I also have, not a bug report, but a small change request
Using SIO with continuous DMA, I'm using the SIO_EV_RXIDLE event to detect the end of a frame (variable length). My callback looks something like this:
Code: Select all
static void sioIdleCb(SIODriver *siop) {
// ... handle DMA partial transfer ...
// Need to clear the hardware IDLE flag before re-enabling the event
siop->usart->ICR = USART_ICR_IDLECF; // <--- Direct hardware access
sioSetEnableFlagsX(siop, SIO_EV_RXIDLE);
}
The Issue:
Currently, there is no portable way to clear a specific event flag in the SIO API.
*
sioGetAndClearEventsX(siop) exists, but it is a "read-and-clear-all" operation, I would prefer to selectively clear one event (IDLE here).
* Directly writing to ICR violates usage of abstractions
Proposal:
add a
sioClearEvents API to the SIO driver
High Level (hal_sio.h):
Code: Select all
/**
* @brief Clears specific SIO event flags.
* @param[in] siop pointer to the @p SIODriver object
* @param[in] mask event flags to be cleared
* @xclass
*/
#define sioClearEventsX(siop, mask) sio_lld_clear_events(siop, mask)
Low Level (STM32 LLD example):
Code: Select all
void sio_lld_clear_events(SIODriver *siop, sioevents_t mask) {
uint32_t icr = 0;
if ((mask & SIO_EV_RXIDLE) != 0U) icr |= USART_ICR_IDLECF;
if ((mask & SIO_EV_TXDONE) != 0U) icr |= USART_ICR_TCCF;
// ... map other flags ...
siop->usart->ICR = icr;
}
This would allow for cleaner, more portable application code:
Code: Select all
sioClearEventsX(siop, SIO_EV_RXIDLE);
sioSetEnableFlagsX(siop, SIO_EV_RXIDLE);
This is just a proposal, and any abstraction will be fine for me.
Alexandre
