[PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Axel Lin]

This is to address the following warning during compilation time:

  CC      drivers/regulator/core.o
drivers/regulator/core.c: In function '_regulator_do_set_voltage':
drivers/regulator/core.c:2183:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 drivers/regulator/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f092588..41cbd74 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2180,7 +2180,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 
 	if (ret == 0 && best_val >= 0)
 		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
-				     (void *)best_val);
+				     (void *)(uintptr_t)best_val);
 
 	trace_regulator_set_voltage_complete(rdev_get_name(rdev), best_val);
 
-- 
1.7.9.5

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Mark Brown]

On Mon, Jul 30, 2012 at 07:47:13AM +0800, Axel Lin wrote:

>  		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
> -				     (void *)best_val);
> +				     (void *)(uintptr_t)best_val);

This also looks problematic, you should never need to cast a pointer to
or from void.

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Axel Lin]

於 一，2012-07-30 於 16:33 +0100，Mark Brown 提到：
> On Mon, Jul 30, 2012 at 07:47:13AM +0800, Axel Lin wrote:
> 
> >  		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
> > -				     (void *)best_val);
> > +				     (void *)(uintptr_t)best_val);
> 
> This also looks problematic, you should never need to cast a pointer to
> or from void.

If I remove the cast to (void *), I got below build warning:

  CC      drivers/regulator/core.o
drivers/regulator/core.c: In function '_regulator_do_set_voltage':
drivers/regulator/core.c:2183:10: warning: passing argument 3 of
'_notifier_call_chain' makes pointer from integer without a cast
[enabled by default]
drivers/regulator/core.c:94:13: note: expected 'void *' but argument is
of type 'long unsigned int'

( Sorry, I forgot to reply-all so I send the mail again )
Axel

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Mark Brown]

On Tue, Jul 31, 2012 at 10:21:56AM +0800, Axel Lin wrote:
> 於 一，2012-07-30 於 16:33 +0100，Mark Brown 提到：
> > On Mon, Jul 30, 2012 at 07:47:13AM +0800, Axel Lin wrote:

> > >  		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
> > > -				     (void *)best_val);
> > > +				     (void *)(uintptr_t)best_val);

> > This also looks problematic, you should never need to cast a pointer to
> > or from void.

> If I remove the cast to (void *), I got below build warning:

>   CC      drivers/regulator/core.o
> drivers/regulator/core.c: In function '_regulator_do_set_voltage':
> drivers/regulator/core.c:2183:10: warning: passing argument 3 of
> '_notifier_call_chain' makes pointer from integer without a cast
> [enabled by default]
> drivers/regulator/core.c:94:13: note: expected 'void *' but argument is
> of type 'long unsigned int'

So the above cast probably isn't right...  you shouldn't need a double
cast here.

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Axel Lin]

於 二，2012-07-31 於 16:41 +0100，Mark Brown 提到：
> On Tue, Jul 31, 2012 at 10:21:56AM +0800, Axel Lin wrote:
> > 於 一，2012-07-30 於 16:33 +0100，Mark Brown 提到：
> > > On Mon, Jul 30, 2012 at 07:47:13AM +0800, Axel Lin wrote:
> 
> > > >  		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
> > > > -				     (void *)best_val);
> > > > +				     (void *)(uintptr_t)best_val);
> 
> > > This also looks problematic, you should never need to cast a pointer to
> > > or from void.
> 
> > If I remove the cast to (void *), I got below build warning:
> 
> >   CC      drivers/regulator/core.o
> > drivers/regulator/core.c: In function '_regulator_do_set_voltage':
> > drivers/regulator/core.c:2183:10: warning: passing argument 3 of
> > '_notifier_call_chain' makes pointer from integer without a cast
> > [enabled by default]
> > drivers/regulator/core.c:94:13: note: expected 'void *' but argument is
> > of type 'long unsigned int'
> 
> So the above cast probably isn't right...  you shouldn't need a double
> cast here.

The issue of original warning is: ( I compile the kernel on x86_64 )
        warning: cast to pointer from integer of different size

What this patch does is:
cast best_val to uintptr_t, which is an unsigned integer large enough to
hold a void* pointer.

And then cast it to (void *).

Thus I did (void *)(uintptr_t)best_val.

Regards,
Axel

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Mark Brown]

On Wed, Aug 01, 2012 at 12:08:08AM +0800, Axel Lin wrote:
> 於 二，2012-07-31 於 16:41 +0100，Mark Brown 提到：

> > So the above cast probably isn't right...  you shouldn't need a double
> > cast here.

> The issue of original warning is: ( I compile the kernel on x86_64 )
>         warning: cast to pointer from integer of different size

> What this patch does is:
> cast best_val to uintptr_t, which is an unsigned integer large enough to
> hold a void* pointer.

> And then cast it to (void *).

> Thus I did (void *)(uintptr_t)best_val.

Perhaps we need a temporary variable here.  The double cast just looks
too horrible.

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Axel Lin]

2012/8/3 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Aug 01, 2012 at 12:08:08AM +0800, Axel Lin wrote:
>> 於 二，2012-07-31 於 16:41 +0100，Mark Brown 提到：
>
>> > So the above cast probably isn't right...  you shouldn't need a double
>> > cast here.
>
>> The issue of original warning is: ( I compile the kernel on x86_64 )
>>         warning: cast to pointer from integer of different size
>
>> What this patch does is:
>> cast best_val to uintptr_t, which is an unsigned integer large enough to
>> hold a void* pointer.
>
>> And then cast it to (void *).
>
>> Thus I did (void *)(uintptr_t)best_val.
>
> Perhaps we need a temporary variable here.  The double cast just looks
> too horrible.

Either is ok to me.
But the double case looks better to me because the intention is clear.
It seems uintptr_t is less commonly used in drivers code.
Maybe just use (void *)(unsigned long)best_val is better in readability.

Re: [PATCH] regulator: core: Add intermediate cast to uintptr_t before casting to pointer

[Mark Brown]

On Mon, Aug 06, 2012 at 12:10:33AM +0800, Axel Lin wrote:

> Either is ok to me.
> But the double case looks better to me because the intention is clear.
> It seems uintptr_t is less commonly used in drivers code.
> Maybe just use (void *)(unsigned long)best_val is better in readability.

Two casts just looks terrible to me to be honest, it'll set off alarm
bells any time I glance at the code just from the visual pattern.