Re: [PATCH] i4l:add some checks for valid drvid and driver pointer

[Karsten Keil]

On Thu, Jun 29, 2006 at 01:29:31PM +0200, Arjan van de Ven wrote:
> On Thu, 2006-06-29 at 13:16 +0200, Karsten Keil wrote:
> > If all drivers go away before all ISDN network interfaces are closed we got
> > a OOps on removing interfaces, this patch avoid it.
> > 
> > Signed-off-by: Karsten Keil <kkeil@xxxxxxx>
> > 
> > diff -ur linux-2.6.4.org/drivers/isdn/i4l/isdn_common.c linux-2.6.4/drivers/isdn/i4l/isdn_common.c
> > --- linux-2.6.4.org/drivers/isdn/i4l/isdn_common.c	2004-03-11 03:55:25.000000000 +0100
> > +++ linux-2.6.4/drivers/isdn/i4l/isdn_common.c	2004-03-30 18:35:38.000000000 +0200
> > @@ -341,6 +341,16 @@
> >  		printk(KERN_WARNING "isdn_command command(%x) driver -1\n", cmd->command);
> >  		return(1);
> >  	}
> > +	if (!dev->drv[cmd->driver]) {
> > +		printk(KERN_WARNING "isdn_command command(%x) dev->drv[%d] NULL\n",
> > +			cmd->command, cmd->driver);
> > +		return(1);
> > +	}
> 
> Hi,
> 
> if this is a "legal" condition, you really shouldn't printk about it. If
> it's not a normal legal condition, this isn't a fix but a hacky
> workaround ;)
> 

I also was thinking about removing the printk, they are in my patch for some
time to prove (which runs for long time on my systems) that this are legal
conditions. But you are correct, now it should be removed, no need to spam
syslog.

> Also..  return is not a function, so return 1; is the preferred form,
> not return(1)..
> 

OK

(I don't know why, but for me return() looks better, but that pure
private taste)


So I will send a new patch.

-- 
Karsten Keil
SuSE Labs
ISDN development
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/