[PATCH 2/2] serial: cascade needless conditionals

[PATCH 2/2] serial: cascade needless conditionals

[André Goddard Rosa]

>From 0f6b1bcb358532007f6098da9bf70794e6a99828 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= <andre.goddard@gmail.com>
Date: Sat, 24 Oct 2009 11:07:22 -0200
Subject: [PATCH 2/2] serial: cascade needless conditionals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: André Goddard Rosa <andre.goddard@gmail.com>

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 885eabe..047530b 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -342,11 +342,11 @@ uart_get_baud_rate(struct uart_port *port,
struct ktermios *termios,

 	if (flags == UPF_SPD_HI)
 		altbaud = 57600;
-	if (flags == UPF_SPD_VHI)
+	else if (flags == UPF_SPD_VHI)
 		altbaud = 115200;
-	if (flags == UPF_SPD_SHI)
+	else if (flags == UPF_SPD_SHI)
 		altbaud = 230400;
-	if (flags == UPF_SPD_WARP)
+	else if (flags == UPF_SPD_WARP)
 		altbaud = 460800;

 	for (try = 0; try < 2; try++) {
@@ -1217,9 +1217,8 @@ static void uart_set_termios(struct tty_struct *tty,
 	/* Handle transition to B0 status */
 	if ((old_termios->c_cflag & CBAUD) && !(cflag & CBAUD))
 		uart_clear_mctrl(state->uart_port, TIOCM_RTS | TIOCM_DTR);
-
 	/* Handle transition away from B0 status */
-	if (!(old_termios->c_cflag & CBAUD) && (cflag & CBAUD)) {
+	else if (!(old_termios->c_cflag & CBAUD) && (cflag & CBAUD)) {
 		unsigned int mask = TIOCM_DTR;
 		if (!(cflag & CRTSCTS) ||
 		    !test_bit(TTY_THROTTLED, &tty->flags))
@@ -1234,9 +1233,8 @@ static void uart_set_termios(struct tty_struct *tty,
 		__uart_start(tty);
 		spin_unlock_irqrestore(&state->uart_port->lock, flags);
 	}
-
 	/* Handle turning on CRTSCTS */
-	if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
+	else if (!(old_termios->c_cflag & CRTSCTS) && (cflag & CRTSCTS)) {
 		spin_lock_irqsave(&state->uart_port->lock, flags);
 		if (!(state->uart_port->ops->get_mctrl(state->uart_port) & TIOCM_CTS)) {
 			tty->hw_stopped = 1;
-- 
1.6.5.1.75.g02d56

Re: [PATCH 2/2] serial: cascade needless conditionals

[Henrique de Moraes Holschuh]

On Sun, 25 Oct 2009, André Goddard Rosa wrote:
>  	if (flags == UPF_SPD_HI)
>  		altbaud = 57600;
> -	if (flags == UPF_SPD_VHI)
> +	else if (flags == UPF_SPD_VHI)
>  		altbaud = 115200;
> -	if (flags == UPF_SPD_SHI)
> +	else if (flags == UPF_SPD_SHI)
>  		altbaud = 230400;
> -	if (flags == UPF_SPD_WARP)
> +	else if (flags == UPF_SPD_WARP)
>  		altbaud = 460800;

This changes code behaviour if more than one bit is set (which might never
happen for all I know...).  You should invert the order of the tests if you
want to make sure it is side-effect-free. 

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 2/2] serial: cascade needless conditionals

[André Goddard Rosa]

Hi, Henrique!

On 10/25/09, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> On Sun, 25 Oct 2009, André Goddard Rosa wrote:
>>  	if (flags == UPF_SPD_HI)
>>  		altbaud = 57600;
>> -	if (flags == UPF_SPD_VHI)
>> +	else if (flags == UPF_SPD_VHI)
>>  		altbaud = 115200;
>> -	if (flags == UPF_SPD_SHI)
>> +	else if (flags == UPF_SPD_SHI)
>>  		altbaud = 230400;
>> -	if (flags == UPF_SPD_WARP)
>> +	else if (flags == UPF_SPD_WARP)
>>  		altbaud = 460800;
>
> This changes code behaviour if more than one bit is set (which might never
> happen for all I know...).  You should invert the order of the tests if you
> want to make sure it is side-effect-free.
>

Do you mind explaining why? Notice that it's not (var & flag), it's =='.
Can flags be equal more than one flag at the same time?

Thanks,
André

Re: [PATCH 2/2] serial: cascade needless conditionals

[Henrique de Moraes Holschuh]

On Sun, 25 Oct 2009, André Goddard Rosa wrote:
> On 10/25/09, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> > On Sun, 25 Oct 2009, André Goddard Rosa wrote:
> >>  	if (flags == UPF_SPD_HI)
> >>  		altbaud = 57600;
> >> -	if (flags == UPF_SPD_VHI)
> >> +	else if (flags == UPF_SPD_VHI)
> >>  		altbaud = 115200;
> >> -	if (flags == UPF_SPD_SHI)
> >> +	else if (flags == UPF_SPD_SHI)
> >>  		altbaud = 230400;
> >> -	if (flags == UPF_SPD_WARP)
> >> +	else if (flags == UPF_SPD_WARP)
> >>  		altbaud = 460800;
> >
> > This changes code behaviour if more than one bit is set (which might never
> > happen for all I know...).  You should invert the order of the tests if you
> > want to make sure it is side-effect-free.
> 
> Do you mind explaining why? Notice that it's not (var & flag), it's =='.
> Can flags be equal more than one flag at the same time?

No, I obviously am going through coffee withdrawal :(  Sorry about this.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh