[PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[Michal Simek]

Initialize "i" irq_info pointer in serial_unlink_irq_chain.

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

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Ralf Baechle <ralf@linux-mips.org>
CC: Manuel Lauss <manuel.lauss@googlemail.com>
CC: linux-serial@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/serial/8250.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 24110f6..6c847d3 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1698,7 +1698,7 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 
 static void serial_unlink_irq_chain(struct uart_8250_port *up)
 {
-	struct irq_info *i;
+	struct irq_info *i = NULL;
 	struct hlist_node *n;
 	struct hlist_head *h;
 
-- 
1.5.5.6

[PATCH 2/2] serial: 8250pci: Recast inb return value

[Michal Simek]

inb returns u8 that's why is necessary to do recast.

Compilation warning:
  CC      drivers/serial/8250_pci.o
drivers/serial/8250_pci.c: In function 'pci_ite887x_init':
drivers/serial/8250_pci.c:825: warning: cast to pointer from integer of different size

Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Lennert Buytenhek <buytenh@wantstofly.org>
CC: Lytochkin Boris <lytboris@gmail.com>
CC: linux-serial@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/serial/8250_pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/8250_pci.c b/drivers/serial/8250_pci.c
index 53be4d3..cc11faa 100644
--- a/drivers/serial/8250_pci.c
+++ b/drivers/serial/8250_pci.c
@@ -822,7 +822,7 @@ static int pci_ite887x_init(struct pci_dev *dev)
 			/* write INTCBAR - ioport */
 			pci_write_config_dword(dev, ITE_887x_INTCBAR,
 								inta_addr[i]);
-			ret = inb(inta_addr[i]);
+			ret = (int) inb(inta_addr[i]);
 			if (ret != 0xff) {
 				/* ioport connected */
 				break;
-- 
1.5.5.6

Re: [PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[Alan Cox]

On Tue, 17 Aug 2010 09:59:06 +0200
Michal Simek <monstr@monstr.eu> wrote:

> Initialize "i" irq_info pointer in serial_unlink_irq_chain.
> 
> Compilation warning:
>   CC      drivers/serial/8250.o
> drivers/serial/8250.c: In function 'serial8250_shutdown':
> drivers/serial/8250.c:1701: warning: 'i' may be used uninitialized in
> this function

NAK (again)

Use a newer compiler - modern ones get this right.

Re: [PATCH 2/2] serial: 8250pci: Recast inb return value

[Alan Cox]

On Tue, 17 Aug 2010 09:59:07 +0200
Michal Simek <monstr@monstr.eu> wrote:

> inb returns u8 that's why is necessary to do recast.
> 
> Compilation warning:
>   CC      drivers/serial/8250_pci.o
> drivers/serial/8250_pci.c: In function 'pci_ite887x_init':
> drivers/serial/8250_pci.c:825: warning: cast to pointer from integer

NAK - this makes no sense at all

Re: [PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[Michal Simek]

Alan Cox wrote:
> On Tue, 17 Aug 2010 09:59:06 +0200
> Michal Simek <monstr@monstr.eu> wrote:
> 
>> Initialize "i" irq_info pointer in serial_unlink_irq_chain.
>>
>> Compilation warning:
>>   CC      drivers/serial/8250.o
>> drivers/serial/8250.c: In function 'serial8250_shutdown':
>> drivers/serial/8250.c:1701: warning: 'i' may be used uninitialized in
>> this function
> 
> NAK (again)
> 
> Use a newer compiler - modern ones get this right.

I wasn't sure if is bug or not.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH 2/2] serial: 8250pci: Recast inb return value

[Alan Cox]

On Tue, 17 Aug 2010 09:59:07 +0200
Michal Simek <monstr@monstr.eu> wrote:

> inb returns u8 that's why is necessary to do recast.

ret is an int, inb returns a u8, neither are pointers and the cast
required is implict and defined by the spec.


> drivers/serial/8250_pci.c: In function 'pci_ite887x_init':
> drivers/serial/8250_pci.c:825: warning: cast to pointer from integer of different size

Re: [PATCH 2/2] serial: 8250pci: Recast inb return value

[Michal Simek]

Alan Cox wrote:
> On Tue, 17 Aug 2010 09:59:07 +0200
> Michal Simek <monstr@monstr.eu> wrote:
> 
>> inb returns u8 that's why is necessary to do recast.
> 
> ret is an int, inb returns a u8, neither are pointers and the cast
> required is implict and defined by the spec.

ok. I am using gcc 4.1.2 which is likely the reason why I see that warnings.

Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[Andrew Morton]

On Tue, 17 Aug 2010 09:17:20 +0100
Alan Cox <alan@linux.intel.com> wrote:

> On Tue, 17 Aug 2010 09:59:06 +0200
> Michal Simek <monstr@monstr.eu> wrote:
> 
> > Initialize "i" irq_info pointer in serial_unlink_irq_chain.
> > 
> > Compilation warning:
> >   CC      drivers/serial/8250.o
> > drivers/serial/8250.c: In function 'serial8250_shutdown':
> > drivers/serial/8250.c:1701: warning: 'i' may be used uninitialized in
> > this function
> 
> NAK (again)
> 
> Use a newer compiler - modern ones get this right.

: static void serial_unlink_irq_chain(struct uart_8250_port *up)
: {
: 	struct irq_info *i;
: 	struct hlist_node *n;
: 	struct hlist_head *h;
: 
: 	mutex_lock(&hash_mutex);
: 
: 	h = &irq_lists[up->port.irq % NR_IRQ_HASH];
: 
: 	hlist_for_each(n, h) {
: 		i = hlist_entry(n, struct irq_info, node);
: 		if (i->irq == up->port.irq)
: 			break;
: 	}
: 
: 	BUG_ON(n == NULL);
: 	BUG_ON(i->head == NULL);

How can any compiler possibly determine that the hlist_for_each() is
never executed zero times?

Re: [PATCH 1/2] serial: 8250: Initialize pointer to irq_info

[Alan Cox]

> > Use a newer compiler - modern ones get this right.
> 
> : static void serial_unlink_irq_chain(struct uart_8250_port *up)
> : {
> : 	struct irq_info *i;
> : 	struct hlist_node *n;
> : 	struct hlist_head *h;
> : 
> : 	mutex_lock(&hash_mutex);
> : 
> : 	h = &irq_lists[up->port.irq % NR_IRQ_HASH];
> : 
> : 	hlist_for_each(n, h) {
> : 		i = hlist_entry(n, struct irq_info, node);
> : 		if (i->irq == up->port.irq)
> : 			break;
> : 	}
> : 
> : 	BUG_ON(n == NULL);
> : 	BUG_ON(i->head == NULL);
> 
> How can any compiler possibly determine that the hlist_for_each() is
> never executed zero times?

We had this argument was it two years ago ?

Modern gcc is capable of at least working out it can't work it out and
doesn't emit a spurious warning.