From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52432) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X7JGR-0005TJ-99 for qemu-devel@nongnu.org; Wed, 16 Jul 2014 03:04:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X7JGM-0000f8-Io for qemu-devel@nongnu.org; Wed, 16 Jul 2014 03:04:31 -0400 Received: from [2001:41d0:8:2b42::1] (port=43527 helo=greensocs.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X7JGM-0000f0-9A for qemu-devel@nongnu.org; Wed, 16 Jul 2014 03:04:26 -0400 Message-ID: <53C623F8.9000809@greensocs.com> Date: Wed, 16 Jul 2014 09:04:24 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1405437524-26536-1-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] cadence_uart: check for serial backend before using it. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , mark.burton@greensocs.com, "qemu-devel@nongnu.org Developers" On 16/07/2014 05:48, Peter Crosthwaite wrote: > On Wed, Jul 16, 2014 at 1:18 AM, wrote: >> From: KONRAD Frederic >> >> This checks that s->chr is not NULL before using it. >> >> Signed-off-by: KONRAD Frederic >> --- >> 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 > > 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 >> >>