[PATCH] usb: fix warning in usbtest module

[PATCH] usb: fix warning in usbtest module

[Greg Dietsche]

On amd64 unsigned is not as wide as pointer and this causes
a compiler warning. Switching to uintptr_t fixes the problem
in an arch independent manner.

Signed-off-by: Greg Dietsche <gregory.dietsche@cuw.edu>
---
 drivers/usb/misc/usbtest.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 388cc12..c6b2082 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -268,9 +268,9 @@ static inline void simple_fill_buf(struct urb *urb)
 	}
 }
 
-static inline unsigned buffer_offset(void *buf)
+static inline uintptr_t buffer_offset(void *buf)
 {
-	return (unsigned)buf & (ARCH_KMALLOC_MINALIGN - 1);
+	return (uintptr_t)buf & (ARCH_KMALLOC_MINALIGN - 1);
 }
 
 static int check_guard_bytes(struct usbtest_dev *tdev, struct urb *urb)
@@ -329,7 +329,7 @@ static int simple_check_buf(struct usbtest_dev *tdev, struct urb *urb)
 
 static void simple_free_urb(struct urb *urb)
 {
-	unsigned offset = buffer_offset(urb->transfer_buffer);
+	uintptr_t offset = buffer_offset(urb->transfer_buffer);
 
 	if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)
 		usb_free_coherent(
-- 
1.7.2.5

Re: [PATCH] usb: fix warning in usbtest module

[Alan Stern]

On Sat, 7 May 2011, Greg Dietsche wrote:

> On amd64 unsigned is not as wide as pointer and this causes
> a compiler warning. Switching to uintptr_t fixes the problem
> in an arch independent manner.

People tend to prefer to see non-typedef'ed type names, whenever 
possible.  In this case, it would be enough to change the type to 
unsigned long.

Lots of code throughout the kernel stores pointer values in unsigned 
long variables.  I've never heard any recommendation for using 
uintptr_t instead.

Alan Stern

Re: [PATCH] usb: fix warning in usbtest module

[Greg Dietsche]

On 05/08/2011 09:37 AM, Alan Stern wrote:
> On Sat, 7 May 2011, Greg Dietsche wrote:
>
>    
>> On amd64 unsigned is not as wide as pointer and this causes
>> a compiler warning. Switching to uintptr_t fixes the problem
>> in an arch independent manner.
>>      
> People tend to prefer to see non-typedef'ed type names, whenever
> possible.  In this case, it would be enough to change the type to
> unsigned long.
>
> Lots of code throughout the kernel stores pointer values in unsigned
> long variables.  I've never heard any recommendation for using
> uintptr_t instead.
>
>    
I was leaning towards unsigned long at first too, but a several things 
made me reconsider:
1) uintptr_t adapts correctly to the size of a pointer on all 
architectures per C99
2) I greped the kernel source and found a number of instances where 
uintptr_t is used
3) unsigned long is technically too wide (though this is better than too 
small...) for some architectures

If the general consensus is that unsigned long is a better choice for 
the kernel, I will update my patch. I do, however think that uintptr_t 
is the best choice from a technical perspective and prefer it over 
unsigned long.

Thanks,
Greg

Re: [PATCH] usb: fix warning in usbtest module

[Greg KH]

On Sun, May 08, 2011 at 02:12:59PM -0500, Greg Dietsche wrote:
> On 05/08/2011 09:37 AM, Alan Stern wrote:
> >On Sat, 7 May 2011, Greg Dietsche wrote:
> >
> >>On amd64 unsigned is not as wide as pointer and this causes
> >>a compiler warning. Switching to uintptr_t fixes the problem
> >>in an arch independent manner.
> >People tend to prefer to see non-typedef'ed type names, whenever
> >possible.  In this case, it would be enough to change the type to
> >unsigned long.
> >
> >Lots of code throughout the kernel stores pointer values in unsigned
> >long variables.  I've never heard any recommendation for using
> >uintptr_t instead.
> >
> I was leaning towards unsigned long at first too, but a several
> things made me reconsider:
> 1) uintptr_t adapts correctly to the size of a pointer on all
> architectures per C99
> 2) I greped the kernel source and found a number of instances where
> uintptr_t is used
> 3) unsigned long is technically too wide (though this is better than
> too small...) for some architectures
> 
> If the general consensus is that unsigned long is a better choice
> for the kernel, I will update my patch. I do, however think that
> uintptr_t is the best choice from a technical perspective and prefer
> it over unsigned long.

Sorry, but no, use 'unsigned long' please.  In the kernel, it's
guaranteed to hold the size of a pointer, that is one of the
requirements of Linux.

And as for C99, those types don't make any sense in the kernel, only in
userspace.  See Linus's posts on this a few years back on lkml if you
want all of the details.

So please redo this patch with 'unsigned long' and I will be glad to
queue it up.

thanks,

greg k-h

[PATCH] usb: fix warning in usbtest module v2

[Greg Dietsche]

On amd64 unsigned is not as wide as pointer and this causes a compiler 
warning. Switching to unsigned long corrects the problem.

Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
 drivers/usb/misc/usbtest.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 388cc12..a5b505a 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -268,9 +268,9 @@ static inline void simple_fill_buf(struct urb *urb)
 	}
 }
 
-static inline unsigned buffer_offset(void *buf)
+static inline unsigned long buffer_offset(void *buf)
 {
-	return (unsigned)buf & (ARCH_KMALLOC_MINALIGN - 1);
+	return (unsigned long)buf & (ARCH_KMALLOC_MINALIGN - 1);
 }
 
 static int check_guard_bytes(struct usbtest_dev *tdev, struct urb *urb)
@@ -329,7 +329,7 @@ static int simple_check_buf(struct usbtest_dev *tdev, struct urb *urb)
 
 static void simple_free_urb(struct urb *urb)
 {
-	unsigned offset = buffer_offset(urb->transfer_buffer);
+	unsigned long offset = buffer_offset(urb->transfer_buffer);
 
 	if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)
 		usb_free_coherent(
-- 
1.7.2.5

Re: [PATCH] usb: fix warning in usbtest module

[Greg Dietsche]

On 05/08/2011 05:58 PM, Greg KH wrote:
> Sorry, but no, use 'unsigned long' please. In the kernel, it's
> guaranteed to hold the size of a pointer, that is one of the
> requirements of Linux.
>
> And as for C99, those types don't make any sense in the kernel, only in
> userspace.  See Linus's posts on this a few years back on lkml if you
> want all of the details.
>    
Thank-you for the explanation! I will go and look up Linus's post. Since 
I'm new around here, please let me know if there are things I can do 
better and/or differently.

Thanks,
Greg