Re: [Qemu-devel] [PATCH target-arm v1 1/2] char/cadence_uart: Handle qemu_chr_fe_write errors

[Peter Maydell]

On 12 February 2014 03:36, Peter Crosthwaite
<address@hidden> wrote:
> By just ignoring them and trying again later. This handles the
> EGAIN case properly (the previous implementation was only dealing
> with short returns and not errors).
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
>  hw/char/cadence_uart.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index 1012f1a..1985047 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -302,8 +302,11 @@ static gboolean cadence_uart_xmit(GIOChannel *chan, 
> GIOCondition cond,
>      }
>
>      ret = qemu_chr_fe_write(s->chr, s->tx_fifo, s->tx_count);
> -    s->tx_count -= ret;
> -    memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count);
> +
> +    if (ret >= 0) {
> +        s->tx_count -= ret;
> +        memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_count);
> +    }
>
>      if (s->tx_count) {
>          int r = qemu_chr_fe_add_watch(s->chr, G_IO_OUT, cadence_uart_xmit, 
> s);

If we got EAGAIN do we really need to re-add the watch and recompute
the UART status, or could we just return early?

Incidentally, this looks like the third or fourth patch fixing use of
qemu_chr_fe_write() in a UART model; perhaps we should audit
the others...

thanks
-- PMM