2009-10-25 13:18:42

by André Goddard Rosa

[permalink] [raw]
Subject: [PATCH 2/2] serial: cascade needless conditionals

>From 0f6b1bcb358532007f6098da9bf70794e6a99828 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Goddard=20Rosa?= <[email protected]>
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 <[email protected]>

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


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

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

2009-10-25 16:37:54

by André Goddard Rosa

[permalink] [raw]
Subject: Re: [PATCH 2/2] serial: cascade needless conditionals

Hi, Henrique!

On 10/25/09, Henrique de Moraes Holschuh <[email protected]> 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?

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

On Sun, 25 Oct 2009, Andr? Goddard Rosa wrote:
> On 10/25/09, Henrique de Moraes Holschuh <[email protected]> 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