Re: [smatch stuff] altera_uart: inconsistent checks for null

[Tobias Klauser <tklauser@distanz.ch>]

On 2011-02-19 at 15:02:05 +0100, Dan Carpenter <error27@gmail.com> wrote:
> On Sat, Feb 19, 2011 at 02:22:17PM +0100, Tobias Klauser wrote:
> > On 2011-02-19 at 10:31:07 +0100, Dan Carpenter <error27@gmail.com> wrote:
> > > Patch 2780ad42f5fe67 "tty: serial: altera_uart: Use port->regshift to
> > > store bus shift" added a NULL check for platp in altera_uart_probe() but
> > > not consistently through out the function.
> > > 
> > > drivers/tty/serial/altera_uart.c +553 altera_uart_probe(43)
> > > 	error: we previously assumed 'platp' could be null.
> > > 
> > >    545          if (platp)
> > >                    ^^^^^^^
> > > checked here.
> > > 
> > >    546                  port->regshift = platp->bus_shift;
> > >    547          else
> > >    548                  port->regshift = 0;
> > >    549
> > >    550          port->line = i;
> > >    551          port->type = PORT_ALTERA_UART;
> > >    552          port->iotype = SERIAL_IO_MEM;
> > >    553          port->uartclk = platp->uartclk;
> > >                                 ^^^^^^^
> > > potential NULL dereference?
> > 
> > Not at the moment, because everybody is using platform data at the
> > momenti, so platp should never be NULL. Still this is inconsistent and
> > shouldn't be there like this. The dereference on line 553 will go away
> > with the patch introducing device tree support to altera_uart [1].
> > 
> > [1] https://lkml.org/lkml/2011/2/18/100
> > 
> > >    554          port->ops = &altera_uart_ops;
> > > 
> > > I don't know the right way to address this.
> > 
> > I guess either Greg or Grant will take the above mentioned patch (it
> > depends on a commit in Grant's tree).

Grant has taken the patch into his tree and it is now in linux-next.
I ran smatch against it and the error went away.

Cheers
Tobias