2005-12-07 01:05:47

by Dave Jones

[permalink] [raw]
Subject: Missing break in timedia serial setup.

Spotted during code review by a Fedora user.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=174967

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6.14/drivers/serial/8250_pci.c~ 2005-12-06 20:01:17.000000000 -0500
+++ linux-2.6.14/drivers/serial/8250_pci.c 2005-12-06 20:01:51.000000000 -0500
@@ -517,6 +517,7 @@ pci_timedia_setup(struct serial_private
case 3:
offset = board->uart_offset;
bar = 1;
+ break;
case 4: /* BAR 2 */
case 5: /* BAR 3 */
case 6: /* BAR 4 */


2005-12-07 09:34:38

by Russell King

[permalink] [raw]
Subject: Re: Missing break in timedia serial setup.

On Tue, Dec 06, 2005 at 08:05:26PM -0500, Dave Jones wrote:
> Spotted during code review by a Fedora user.
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=174967

Makes no difference.

switch (idx) {
case 3:
bar = 1;
case 4: /* BAR 2 */
case 5: /* BAR 3 */
case 6: /* BAR 4 */
case 7: /* BAR 5 */
bar = idx - 2;

If idx is 3, idx - 2 is 1. So the bar = 1 is actually redundant in
case 3.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2005-12-07 16:58:21

by Dave Jones

[permalink] [raw]
Subject: Re: Missing break in timedia serial setup.

On Wed, Dec 07, 2005 at 09:34:31AM +0000, Russell King wrote:
> On Tue, Dec 06, 2005 at 08:05:26PM -0500, Dave Jones wrote:
> > Spotted during code review by a Fedora user.
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=174967
>
> Makes no difference.
>
> switch (idx) {
> case 3:
> bar = 1;
> case 4: /* BAR 2 */
> case 5: /* BAR 3 */
> case 6: /* BAR 4 */
> case 7: /* BAR 5 */
> bar = idx - 2;
>
> If idx is 3, idx - 2 is 1. So the bar = 1 is actually redundant in
> case 3.

Duh, yes of course. Here's a patch to remove the redundant assignment,
but as it's harmless, I'll leave it up to you to decide whether or
not it's worth applying.

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6.14/drivers/serial/8250_pci.c~ 2005-12-07 11:56:13.000000000 -0500
+++ linux-2.6.14/drivers/serial/8250_pci.c 2005-12-07 11:56:41.000000000 -0500
@@ -516,7 +516,6 @@ pci_timedia_setup(struct serial_private
break;
case 3:
offset = board->uart_offset;
- bar = 1;
case 4: /* BAR 2 */
case 5: /* BAR 3 */
case 6: /* BAR 4 */

2005-12-07 17:05:23

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Missing break in timedia serial setup.


>
> Signed-off-by: Dave Jones <[email protected]>
>
> --- linux-2.6.14/drivers/serial/8250_pci.c~ 2005-12-07 11:56:13.000000000 -0500
> +++ linux-2.6.14/drivers/serial/8250_pci.c 2005-12-07 11:56:41.000000000 -0500
> @@ -516,7 +516,6 @@ pci_timedia_setup(struct serial_private
> break;
> case 3:
> offset = board->uart_offset;
> - bar = 1;
> case 4: /* BAR 2 */
> case 5: /* BAR 3 */
> case 6: /* BAR 4 */
> -

might as well add a /* fall through */ comment
so that this doesn't come up again in the future..


2005-12-07 17:44:23

by Dave Jones

[permalink] [raw]
Subject: Re: Missing break in timedia serial setup.

On Wed, Dec 07, 2005 at 06:05:19PM +0100, Arjan van de Ven wrote:

> might as well add a /* fall through */ comment
> so that this doesn't come up again in the future..

Remove redundant assignment, and mark fallthrough.

Signed-off-by: Dave Jones <[email protected]>

--- linux-2.6.14/drivers/serial/8250_pci.c~ 2005-12-07 12:42:58.000000000 -0500
+++ linux-2.6.14/drivers/serial/8250_pci.c 2005-12-07 12:43:08.000000000 -0500
@@ -516,7 +516,7 @@ pci_timedia_setup(struct serial_private
break;
case 3:
offset = board->uart_offset;
- bar = 1;
+ /* FALLTHROUGH */
case 4: /* BAR 2 */
case 5: /* BAR 3 */
case 6: /* BAR 4 */

2005-12-07 18:11:47

by Russell King

[permalink] [raw]
Subject: Re: Missing break in timedia serial setup.

On Wed, Dec 07, 2005 at 12:44:09PM -0500, Dave Jones wrote:
> On Wed, Dec 07, 2005 at 06:05:19PM +0100, Arjan van de Ven wrote:
>
> > might as well add a /* fall through */ comment
> > so that this doesn't come up again in the future..
>
> Remove redundant assignment, and mark fallthrough.

Thanks Dave, committed.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core