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

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

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>


---

 drivers/char/tlclk.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

f1bbf945c86b729c20199133daba358284fa3c32
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);
 		ret = -EBUSY;
 		goto out4;
 	}
-- 
0.99.9.GIT

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

[Russell King]

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;
	}

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

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

[Dmitry Torokhov]

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