As outlined in the test case, we pull a total of 50 bits from the
transmitter in two rounds, pulling 25 bits at a time. In the default
8-N-1 configuration, 50 bits should ideally comprise 5 characters.
However, as observed, only a total of 4 characters are retrieved
from the application, leaving the remaining 10 bits (5 + 5) unused.
Change-Id: Ic2539681a4adf6c1822e0bc256e4c829813d0e21
This problem can only happen if the user is flush()ing the Rx buffer
manually by calling osmo_soft_uart_flush_rx(). Let's demonstrate it
in the unit test, so that we don't forget about it (add FIXME).
Change-Id: Iad932a505d6fd98360f90510651501f8708ff5d2
Coverity tells us that with the current logic it's possible (in theory)
that we may dereference NULL pointer in osmo_soft_uart_flush_rx(). This
is highly unlikely, because the Rx buffer gets allocated once when the
Rx is enabled and remains even after the Rx gets disabled. The Rx flags
cannot be anything than 0x00 before the Rx gets enabled.
Even though this NULL pointer dereference is unlikely, the Rx flushing
logic is still not entirely correct. As can be seen from the unit test
output, the Rx callback of the application may be called with an empty
msgb if the following conditions are both met:
a) the osmo_soft_uart_flush_rx() is invoked manually, and
b) a parity and/or a framing error has occurred previously.
We should not be checking suart->rx.flags in osmo_soft_uart_flush_rx(),
since this is already done in suart_rx_ch(), which is calling it.
Removing this check also eliminates a theoretical possibility of the
NULL pointer dereference, so we're killing two birds with one stone.
- Do not check suart->rx.flags in osmo_soft_uart_flush_rx().
- Add a unit test for various flush()ing scenarios.
Change-Id: I5179f5fd2361e4e96ac9bf48e80b99e53a7e4712
Fixes: CID#336545
According to Coverity, we check return value of this function in
all other cases except this one (9 out of 10 times), so let's add
the missing assert(), just to be sure.
Change-Id: I675f4089cc990be5fcda792276b6808742f6f0d7
Fixes: CID#336557
This is a partial revert of 0887188c6b.
We actually want to return number of bits pulled, because in the upcoming
commit implementing the flow control we want to be able to signal to the
caller that the buffer was not completely filled, but only partly.
Change-Id: I47a56f0fc36f2bc8f5a797d7fec64dfb56842388
Related: OS#4396
Check it once rather than doing this in a loop. Return -EAGAIN if
Rx or Tx is not enabled when calling osmo_soft_uart_{rx,tx}_ubits().
This [theoretically] improves performance by reducing the number of
conditional statements in loops. In the Tx path, this also prevents
calling the .tx_cb() when the transmitter is disabled, so that we
don't loose the application data.
Change-Id: I70f93b3655eb21c2323e451052c40cd305c016c8
Related: OS#4396
As can be seen, pulling a small number of bits at a time (smaller
than a single UART frame would fit into) results in calling the
.tx_cb() with a msgb having no room at all, and thus pulling the
stop bits instead of the actual data.
Change-Id: Icfee378f0fdc5e32fe9ce0afab5f75bc278653a9
Related: OS#4396
Whenever we encounter a parity and/or a framing error, we should
call the .rx_cb() immediately, even if this was the first
character in the receive buffer.
Change-Id: I73fab1a5c196d2dbdfe98b0c20d8dadbd22f4f64
Related: OS#4396
Vadim Yanitskiy