Re: [PATCH] tlclk.c: pointers are handled by %p

[Dmitry Torokhov <dtor_core@ameritech.net>]

On Thursday 15 December 2005 04:57, Russell King wrote:
> On Thu, Dec 15, 2005 at 09:18:35AM +0000, Al Viro wrote:
> > diff --git a/drivers/char/tlclk.c b/drivers/char/tlclk.c
> > index 12167c0..e8467dc 100644
> > --- a/drivers/char/tlclk.c
> > +++ b/drivers/char/tlclk.c
> > @@ -776,8 +776,8 @@ static int __init tlclk_init(void)
> >  	tlclk_device = platform_device_register_simple("telco_clock",
> >  				-1, NULL, 0);
> >  	if (!tlclk_device) {
> > -		printk(KERN_ERR " platform_device_register retruns 0x%X\n",
> > -			(unsigned int) tlclk_device);
> > +		printk(KERN_ERR " platform_device_register retruns 0x%p\n",
> > +			tlclk_device);
> 
> This looks really strange - we know what tlclk_device will be at that
> printk - it'll be NULL because if it's anything different we wouldn't
> be inside this if(){ }.
> 
> Moreover, this code is obviously bogus.  platform_device_register_simple
> does not return NULL for the error case.  It should be something like:
> 
> 	if (IS_ERR(tlclk_device)) {
> 		ret = PTR_ERR(tlclk_device);
> 		printk(KERN_ERR "platform_device_register returns %d\n",
> 		        ret);
> 		goto out4;
> 	}
> 

I have a patch killing usage of platform_register_device_simple in this
driver (converting to platform_device_alloc() + _add()). Will post in a
minute.

-- 
Dmitry