| Message ID | 1405437524-26536-1-git-send-email-fred.konrad@greensocs.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 16, 2014 at 1:18 AM, <fred.konrad@greensocs.com> wrote: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > This checks that s->chr is not NULL before using it. > > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > 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? 300 if (!s->tx_count) { 301 return FALSE; 302 } With this hunk dropped, Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> 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 > >
On 16/07/2014 05:48, Peter Crosthwaite wrote: > On Wed, Jul 16, 2014 at 1:18 AM, <fred.konrad@greensocs.com> wrote: >> From: KONRAD Frederic <fred.konrad@greensocs.com> >> >> This checks that s->chr is not NULL before using it. >> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >> --- >> 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 <peter.crosthwaite@xilinx.com> > > 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 >> >>
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; } 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; }