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 <[email protected]>
---
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
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
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
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
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 <[email protected]>
---
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
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