public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix an uninitialised variable warning in the 8250 driver
@ 2008-11-14 16:20 David Howells
  2008-11-14 16:34 ` Alan Cox
  0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2008-11-14 16:20 UTC (permalink / raw)
  To: linux-serial, akpm; +Cc: alan, dhowells, linux-kernel

Fix an uninitialised variable warning in the 8250 driver:

  CC      drivers/serial/8250.o
drivers/serial/8250.c: In function 'serial8250_shutdown':
drivers/serial/8250.c:1612: warning: 'i' may be used uninitialized in this function

This has the added advantage of removing a conditional test from the main path
as the BUG_ON() that indicated the loop dropped off the end is now bypassed in
the event of a successful search.  Indeed that BUG_ON() can now be a BUG() as
the only way to reach it automatically makes its condition true.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 drivers/serial/8250.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)


diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 303272a..62cf7e5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1620,10 +1620,11 @@ static void serial_unlink_irq_chain(struct uart_8250_port *up)
 	hlist_for_each(n, h) {
 		i = hlist_entry(n, struct irq_info, node);
 		if (i->irq == up->port.irq)
-			break;
+			goto found;
 	}
+	BUG();
 
-	BUG_ON(n == NULL);
+found:
 	BUG_ON(i->head == NULL);
 
 	if (list_empty(i->head))

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix an uninitialised variable warning in the 8250 driver
  2008-11-14 16:20 [PATCH] Fix an uninitialised variable warning in the 8250 driver David Howells
@ 2008-11-14 16:34 ` Alan Cox
  2008-11-14 16:51   ` David Howells
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Cox @ 2008-11-14 16:34 UTC (permalink / raw)
  Cc: linux-serial, akpm, dhowells, linux-kernel

On Fri, 14 Nov 2008 16:20:38 +0000
David Howells <dhowells@redhat.com> wrote:

> Fix an uninitialised variable warning in the 8250 driver:
> 
>   CC      drivers/serial/8250.o
> drivers/serial/8250.c: In function 'serial8250_shutdown':
> drivers/serial/8250.c:1612: warning: 'i' may be used uninitialized in this function

Current versions of gcc that I've tested get this all correct just fine
for x86 at least.

Do check that the BUG_ON() helper for whatever platform you are using
needs to correctly indicate that its BUG spewing path may not return
(__attibute((__noreturn__)). I seem to remember some of the architectures
are broken on that.

Alan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix an uninitialised variable warning in the 8250 driver
  2008-11-14 16:34 ` Alan Cox
@ 2008-11-14 16:51   ` David Howells
  2008-11-14 19:01     ` Alan Cox
  0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2008-11-14 16:51 UTC (permalink / raw)
  To: Alan Cox; +Cc: dhowells, linux-serial, akpm, linux-kernel

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> Current versions of gcc that I've tested get this all correct just fine
> for x86 at least.

gcc version 4.1.2 20070925 (Red Hat 4.1.2-33)

> Do check that the BUG_ON() helper for whatever platform you are using
> needs to correctly indicate that its BUG spewing path may not return
> (__attibute((__noreturn__)). I seem to remember some of the architectures
> are broken on that.

x86_64 - which doesn't do that.  It uses asm volatile and an infinite for-loop.

Surely having BUG() retreat through a noreturn function may lose important
state, such as the function that issued the bug as the compiler is under no
requirement to save the return value, and the file/line info may not be
available.

David

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix an uninitialised variable warning in the 8250 driver
  2008-11-14 16:51   ` David Howells
@ 2008-11-14 19:01     ` Alan Cox
  0 siblings, 0 replies; 4+ messages in thread
From: Alan Cox @ 2008-11-14 19:01 UTC (permalink / raw)
  Cc: dhowells, linux-serial, akpm, linux-kernel

On Fri, 14 Nov 2008 16:51:08 +0000
David Howells <dhowells@redhat.com> wrote:

> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > Current versions of gcc that I've tested get this all correct just fine
> > for x86 at least.
> 
> gcc version 4.1.2 20070925 (Red Hat 4.1.2-33)

Newer gcc (4.3 and above) gets it right.

> x86_64 - which doesn't do that.  It uses asm volatile and an infinite for-loop.

Which it seems 4.3 knows about

> Surely having BUG() retreat through a noreturn function may lose important
> state, such as the function that issued the bug as the compiler is under no
> requirement to save the return value, and the file/line info may not be
> available.

Its arch specific code so arch specific things can be done.

Alan

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-11-14 19:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-14 16:20 [PATCH] Fix an uninitialised variable warning in the 8250 driver David Howells
2008-11-14 16:34 ` Alan Cox
2008-11-14 16:51   ` David Howells
2008-11-14 19:01     ` Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox