Re: [Qemu-devel] [PATCH] cadence_uart: check for serial backend before using it.

[Frederic Konrad]

On 16/07/2014 05:48, Peter Crosthwaite wrote:
> On Wed, Jul 16, 2014 at 1:18 AM,  <address@hidden> wrote:
> > From: KONRAD Frederic <address@hidden>
> >
> > This checks that s->chr is not NULL before using it.
> >
> > Signed-off-by: KONRAD Frederic <address@hidden>
> > ---
> >   hw/char/cadence_uart.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> > index dbbc167..a5736cb 100644
> > --- a/hw/char/cadence_uart.c
> > +++ b/hw/char/cadence_uart.c
> > @@ -175,8 +175,10 @@ static void uart_send_breaks(UartState *s)
> >   {
> >       int break_enabled = 1;
> >
> > -    qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> > -                               &break_enabled);
> > +    if (s->chr) {
> > +        qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> > +                                   &break_enabled);
> > +    }
> >   }
> >
> >   static void uart_parameters_setup(UartState *s)
> > @@ -227,7 +229,9 @@ static void uart_parameters_setup(UartState *s)
> >
> >       packet_size += ssp.data_bits + ssp.stop_bits;
> >       s->char_tx_time = (get_ticks_per_sec() / ssp.speed) * packet_size;
> > -    qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
> > +    if (s->chr) {
> > +        qemu_chr_fe_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
> > +    }
> >   }
> >
> >   static int uart_can_receive(void *opaque)
> > @@ -295,6 +299,7 @@ static gboolean cadence_uart_xmit(GIOChannel *chan, 
> > GIOCondition cond,
> >       /* instant drain the fifo when there's no back-end */
> >       if (!s->chr) {
> >           s->tx_count = 0;
> > +        return FALSE;
> Is this needed? With s->tx_count forced to 0 in the NULL dev case,
> won't the next if trigger immediately and return?

True,
I'll make this change and resend.

Thanks,
Fred

> 300     if (!s->tx_count) {
> 301         return FALSE;
> 302     }
>
> With this hunk dropped,
>
> Reviewed-by: Peter Crosthwaite <address@hidden>
>
> And recommended for 2.1.
>
> Regards,
> Peter
>
> >       }
> >
> >       if (!s->tx_count) {
> > @@ -375,7 +380,9 @@ static void uart_read_rx_fifo(UartState *s, uint32_t *c)
> >           *c = s->rx_fifo[rx_rpos];
> >           s->rx_count--;
> >
> > -        qemu_chr_accept_input(s->chr);
> > +        if (s->chr) {
> > +            qemu_chr_accept_input(s->chr);
> > +        }
> >       } else {
> >           *c = 0;
> >       }
> > --
> > 1.9.0