[patch 2.6.22-rc2-git] spidev compiler warning gone

[patch 2.6.22-rc2-git] spidev compiler warning gone

[David Brownell]

Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
add an extra cast using "ptrdiff_t" to convert the u64 to the correct
size integer, and only then casting it into a "void *" pointer.

Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
 drivers/spi/spidev.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- g26.orig/drivers/spi/spidev.c	2007-05-24 12:04:46.000000000 -0700
+++ g26/drivers/spi/spidev.c	2007-05-25 10:31:56.000000000 -0700
@@ -181,7 +181,8 @@ static int spidev_message(struct spidev_
 		}
 		if (u_tmp->tx_buf) {
 			k_tmp->tx_buf = buf;
-			if (copy_from_user(buf, (const u8 __user *)u_tmp->tx_buf,
+			if (copy_from_user(buf, (const u8 __user *)
+						(ptrdiff_t) u_tmp->tx_buf,
 					u_tmp->len))
 				goto done;
 		}
@@ -213,7 +214,8 @@ static int spidev_message(struct spidev_
 	buf = spidev->buffer;
 	for (n = n_xfers, u_tmp = u_xfers; n; n--, u_tmp++) {
 		if (u_tmp->rx_buf) {
-			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
+			if (__copy_to_user((u8 __user *)
+					(ptrdiff_t) u_tmp->rx_buf, buf,
 					u_tmp->len)) {
 				status = -EFAULT;
 				goto done;

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [patch 2.6.22-rc2-git] spidev compiler warning gone

[Andrew Morton]

On Fri, 25 May 2007 10:35:07 -0700
David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:

> Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
> add an extra cast using "ptrdiff_t" to convert the u64 to the correct
> size integer, and only then casting it into a "void *" pointer.
> 
> Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/spi/spidev.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- g26.orig/drivers/spi/spidev.c	2007-05-24 12:04:46.000000000 -0700
> +++ g26/drivers/spi/spidev.c	2007-05-25 10:31:56.000000000 -0700
> @@ -181,7 +181,8 @@ static int spidev_message(struct spidev_
>  		}
>  		if (u_tmp->tx_buf) {
>  			k_tmp->tx_buf = buf;
> -			if (copy_from_user(buf, (const u8 __user *)u_tmp->tx_buf,
> +			if (copy_from_user(buf, (const u8 __user *)
> +						(ptrdiff_t) u_tmp->tx_buf,
>  					u_tmp->len))
>  				goto done;
>  		}
> @@ -213,7 +214,8 @@ static int spidev_message(struct spidev_
>  	buf = spidev->buffer;
>  	for (n = n_xfers, u_tmp = u_xfers; n; n--, u_tmp++) {
>  		if (u_tmp->rx_buf) {
> -			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
> +			if (__copy_to_user((u8 __user *)
> +					(ptrdiff_t) u_tmp->rx_buf, buf,
>  					u_tmp->len)) {
>  				status = -EFAULT;
>  				goto done;

This seems overly tricky.  The way we normally squish that warning is
via a (long) cast.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [patch 2.6.22-rc2-git] spidev compiler warning gone

[David Brownell]

On Tuesday 29 May 2007, Andrew Morton wrote:
> On Fri, 25 May 2007 10:35:07 -0700
> David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> 
> > Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
> > add an extra cast using "ptrdiff_t" to convert the u64 to the correct
> > size integer, and only then casting it into a "void *" pointer.
> > 
> >	...
> > -			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
> > +			if (__copy_to_user((u8 __user *)
> > +					(ptrdiff_t) u_tmp->rx_buf, buf,
> > 	...
> 
> This seems overly tricky.  The way we normally squish that warning is
> via a (long) cast.

Well, "ptrdiff_t" seems equivalent to "long" in common cases.

Presumably C99 guarantees pointer-to-long conversions work in
both directions?  It seemed there must necessarily be such a
guarantee for "ptrdiff_t", even if it weren't true for "long".
I didn't have a copy of a C spec to consult.  ;)

- Dave

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [patch 2.6.22-rc2-git] spidev compiler warning gone

[Andrew Morton]

On Tue, 29 May 2007 15:39:04 -0700
David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:

> On Tuesday 29 May 2007, Andrew Morton wrote:
> > On Fri, 25 May 2007 10:35:07 -0700
> > David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> > 
> > > Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
> > > add an extra cast using "ptrdiff_t" to convert the u64 to the correct
> > > size integer, and only then casting it into a "void *" pointer.
> > > 
> > >	...
> > > -			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
> > > +			if (__copy_to_user((u8 __user *)
> > > +					(ptrdiff_t) u_tmp->rx_buf, buf,
> > > 	...
> > 
> > This seems overly tricky.  The way we normally squish that warning is
> > via a (long) cast.
> 
> Well, "ptrdiff_t" seems equivalent to "long" in common cases.
> 
> Presumably C99 guarantees pointer-to-long conversions work in
> both directions?  It seemed there must necessarily be such a
> guarantee for "ptrdiff_t", even if it weren't true for "long".
> I didn't have a copy of a C spec to consult.  ;)
> 

Thing is, the kernel does a cast of an integer-type to a pointer-type in a
*lot* of places.  And the standard way in which we squish the warning is
via a (long) cast.

So it's a standard pattern and people understand it.  Whereas the ptrdiff_t
thing will make readers go "wtf?".  Even if it deos happen to work.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [patch 2.6.22-rc2-git] spidev compiler warning gone

[David Brownell]

> > > > Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
> > > > add an extra cast using "ptrdiff_t" to convert the u64 to the correct
> > > > size integer, and only then casting it into a "void *" pointer.
> > > > 
> > > >	...
> > > > -			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
> > > > +			if (__copy_to_user((u8 __user *)
> > > > +					(ptrdiff_t) u_tmp->rx_buf, buf,
> > > > 	...
> > > 
> > > This seems overly tricky.  The way we normally squish that warning is
> > > via a (long) cast.
> > 
> > Well, "ptrdiff_t" seems equivalent to "long" in common cases.
> > 
> > Presumably C99 guarantees pointer-to-long conversions work in
> > both directions?  It seemed there must necessarily be such a
> > guarantee for "ptrdiff_t", even if it weren't true for "long".
> > I didn't have a copy of a C spec to consult.  ;)
> > 
> 
> Thing is, the kernel does a cast of an integer-type to a pointer-type in a
> *lot* of places.  And the standard way in which we squish the warning is
> via a (long) cast.

Good to know, but I didn't notice any examples anywhere in the source
code of doing that.  I used the first solution I found, when I went
hunting for a fix.  It wasn't "long".  And it appears that "long" has
fewer portability guarantees...

Something in Documentation/* should talk about 64-bit cleanliness,
and maybe mention such issues.


> So it's a standard pattern and people understand it.  Whereas the ptrdiff_t
> thing will make readers go "wtf?".  Even if it deos happen to work.

At this point, "long" would surprise *me*!  I notice you didn't
answer my question about C specs and "long".  "ptrdiff_t" sure seems
to be a standard C data type designed specifically to solve that
class of 32/64/... portability problem.  "intptr_t" would seem to be
the most standard type, but I don't see it in kernel includes.

I notice that GCC docs say ptrdiff_t "will probably be one of the
standard integer types (short, int, long) but might be a nonstandard
type that exists only for this purpose".  So using "long" isn't as
portable, and isn't even guaranteed to work everywhere.

- Dave


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [patch 2.6.22-rc2-git] spidev compiler warning gone

[Andrew Morton]

On Tue, 29 May 2007 19:05:34 -0700 David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:

> 
> > > > > Get rid of annoying GCC warning on 32-bit platforms.  The trick is to
> > > > > add an extra cast using "ptrdiff_t" to convert the u64 to the correct
> > > > > size integer, and only then casting it into a "void *" pointer.
> > > > > 
> > > > >	...
> > > > > -			if (__copy_to_user((u8 __user *)u_tmp->rx_buf, buf,
> > > > > +			if (__copy_to_user((u8 __user *)
> > > > > +					(ptrdiff_t) u_tmp->rx_buf, buf,
> > > > > 	...
> > > > 
> > > > This seems overly tricky.  The way we normally squish that warning is
> > > > via a (long) cast.
> > > 
> > > Well, "ptrdiff_t" seems equivalent to "long" in common cases.
> > > 
> > > Presumably C99 guarantees pointer-to-long conversions work in
> > > both directions?  It seemed there must necessarily be such a
> > > guarantee for "ptrdiff_t", even if it weren't true for "long".
> > > I didn't have a copy of a C spec to consult.  ;)
> > > 
> > 
> > Thing is, the kernel does a cast of an integer-type to a pointer-type in a
> > *lot* of places.  And the standard way in which we squish the warning is
> > via a (long) cast.
> 
> Good to know, but I didn't notice any examples anywhere in the source
> code of doing that.  I used the first solution I found, when I went
> hunting for a fix.  It wasn't "long".  And it appears that "long" has
> fewer portability guarantees...

Kernel assumes that sizeof(long)==sizeof(foo *) in a tremendous number of 
places.  That's why we safely use (long) for this.

box:/usr/src/linux-2.6.22-rc3> fgrep -r '*)(long)' . | wc -l
95
box:/usr/src/linux-2.6.22-rc3> fgrep -r '(int)(long)' . | wc -l 
45
box:/usr/src/linux-2.6.22-rc3> fgrep -r '(ptrdiff_t)' . | wc -l         
7

> Something in Documentation/* should talk about 64-bit cleanliness,
> and maybe mention such issues.

spose so.
 
> 
> > So it's a standard pattern and people understand it.  Whereas the ptrdiff_t
> > thing will make readers go "wtf?".  Even if it deos happen to work.
> 
> At this point, "long" would surprise *me*!  I notice you didn't
> answer my question about C specs and "long".

oh.  I wasn't very interested, sorry ;)  I'm just pointing out that
(long) will cause least-surprise to most-people.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/