[PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 968 bytes --]

From d457b05faabde1a51b8e4b8f6fc13af9f07809f8 Mon Sep 17 00:00:00 2001

drm_handle_t appears to be assigned values from void*. As such unsigned
int is certainly not the same size on 64-bit. Convert to uintptr_t in
all cases as it is defined for this purpose.

I fear this patch changes ABI, but I am not sure.

Matthew W.S. Bell

---
 include/drm/drm.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4822159..95141a2 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -40,7 +40,7 @@
 
 #include <linux/types.h>
 #include <asm/ioctl.h>
-typedef unsigned int drm_handle_t;
+typedef uintptr_t drm_handle_t;
 
 #else /* One of the BSDs */
 
@@ -54,7 +54,7 @@ typedef int32_t  __s32;
 typedef uint32_t __u32;
 typedef int64_t  __s64;
 typedef uint64_t __u64;
-typedef unsigned long drm_handle_t;
+typedef uintptr_t drm_handle_t;
 
 #endif
 
-- 
1.7.0



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Dave Airlie]

> 
> drm_handle_t appears to be assigned values from void*. As such unsigned
> int is certainly not the same size on 64-bit. Convert to uintptr_t in
> all cases as it is defined for this purpose.

No, its "designed" as is. We can't change it now as its ABI. We make sure 
we only use 32-bit handles anyways.

Dave.

> 
> I fear this patch changes ABI, but I am not sure.
> 
> Matthew W.S. Bell
> 
> ---
>  include/drm/drm.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 4822159..95141a2 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -40,7 +40,7 @@
>  
>  #include <linux/types.h>
>  #include <asm/ioctl.h>
> -typedef unsigned int drm_handle_t;
> +typedef uintptr_t drm_handle_t;
>  
>  #else /* One of the BSDs */
>  
> @@ -54,7 +54,7 @@ typedef int32_t  __s32;
>  typedef uint32_t __u32;
>  typedef int64_t  __s64;
>  typedef uint64_t __u64;
> -typedef unsigned long drm_handle_t;
> +typedef uintptr_t drm_handle_t;
>  
>  #endif
>  
> 

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 462 bytes --]

On Sat, 2010-04-03 at 08:49 +0100, Dave Airlie wrote:
> No, its "designed" as is. We can't change it now as its ABI. We make sure 
> we only use 32-bit handles anyways.

OK, is this documented anywhere, as I'd like to pull some of that into a
comment? (It appears, on a casual glance, that this type is not used in
the kernel ABI, but only in the libdrm, and associates?, ABI, so could
be changed if the SONAME got bumped and anyone cared.)

Matthew


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 296 bytes --]

On Sat, 2010-04-03 at 08:49 +0100, Dave Airlie wrote:
> No, its "designed" as is. We can't change it now as its ABI. We make sure 
> we only use 32-bit handles anyways.

Sorry, the comment about the ABI is, of course, nonsense, as the
assumption is implicit in the kernel ABI.

Matthew


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Dave Airlie]

On Sat, Apr 3, 2010 at 11:09 PM, Matthew W. S. Bell
<matthew@bells23.org.uk> wrote:
> On Sat, 2010-04-03 at 08:49 +0100, Dave Airlie wrote:
>> No, its "designed" as is. We can't change it now as its ABI. We make sure
>> we only use 32-bit handles anyways.

The thing is "unsigned int" is correct for the handles, they are defined to
be 32-bit numbers no matter what system they are on, its the use of the
void * that is actually the problem.

Its probably not documented well anywhere, though I think the handles are
32-bit is written down somewhere.

Dave.
>
> OK, is this documented anywhere, as I'd like to pull some of that into a
> comment? (It appears, on a casual glance, that this type is not used in
> the kernel ABI, but only in the libdrm, and associates?, ABI, so could
> be changed if the SONAME got bumped and anyone cared.)
>
> Matthew
>
>
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>
>

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH] typdef uintptr_t drm_handle_t; unsigned int is wrong on 64-bit.

[Robert Noland]

Dave Airlie wrote:
> On Sat, Apr 3, 2010 at 11:09 PM, Matthew W. S. Bell
> <matthew@bells23.org.uk> wrote:
>> On Sat, 2010-04-03 at 08:49 +0100, Dave Airlie wrote:
>>> No, its "designed" as is. We can't change it now as its ABI. We make sure
>>> we only use 32-bit handles anyways.
> 
> The thing is "unsigned int" is correct for the handles, they are defined to
> be 32-bit numbers no matter what system they are on, its the use of the
> void * that is actually the problem.
> 
> Its probably not documented well anywhere, though I think the handles are
> 32-bit is written down somewhere.

No, they are not.  They are only 32bit on linux.  The size of handle 
varies with architecture on FreeBSD as it is passed to mmap as an offset 
into memory.  Honestly, I would prefer that they always be 64bits, but 
that would break ABI...  The dual meaning of handle has frustrated me 
more than a few times.

robert.

> Dave.
>> OK, is this documented anywhere, as I'd like to pull some of that into a
>> comment? (It appears, on a casual glance, that this type is not used in
>> the kernel ABI, but only in the libdrm, and associates?, ABI, so could
>> be changed if the SONAME got bumped and anyone cared.)
>>
>> Matthew
>>
>>
>> ------------------------------------------------------------------------------
>> Download Intel&#174; Parallel Studio Eval
>> Try the new software tools for yourself. Speed compiling, find bugs
>> proactively, and fine-tune applications for parallel performance.
>> See why Intel Parallel Studio got high marks during beta.
>> http://p.sf.net/sfu/intel-sw-dev
>> --
>> _______________________________________________
>> Dri-devel mailing list
>> Dri-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/dri-devel
>>
>>
> 
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

[PATCH] drm_handle_t type

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 1908 bytes --]

On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> Its probably not documented well anywhere, though I think the handles are
> 32-bit is written down somewhere.

Well, here's a patch that silences the warnings and adds a little note.

Matthew

---
From 9111cd82560052a329b78cff8d7e45c6815e0276 Mon Sep 17 00:00:00 2001
From: Matthew W. S. Bell <matthew@bells23.org.uk>
Date: Fri, 9 Apr 2010 02:58:42 +0100

Add a comment about content of void* variable exchanged with kernel.
Add casts to silence warnings about pointer to integer of a different
size assignment.
---
 include/drm/drm.h |    2 ++
 xf86drm.c         |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4822159..e74d6d6 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -216,6 +216,8 @@ struct drm_map {
 	enum drm_map_flags flags;	 /**< Flags */
 	void *handle;		 /**< User-space: "Handle" to pass to mmap() */
 				 /**< Kernel-space: kernel-virtual address */
+				 /**  The value herein shall be the size of */
+				 /**  drm_handle_t despite the size of void *. */
 	int mtrr;		 /**< MTRR slot used */
 	/*   Private data */
 };
diff --git a/xf86drm.c b/xf86drm.c
index 220aaa1..012408a 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -959,7 +959,7 @@ int drmAddMap(int fd, drm_handle_t offset, drmSize size, drmMapType type,
     if (drmIoctl(fd, DRM_IOCTL_ADD_MAP, &map))
 	return -errno;
     if (handle)
-	*handle = (drm_handle_t)map.handle;
+	*handle = (uintptr_t)map.handle;
     return 0;
 }
 
@@ -2138,7 +2138,7 @@ int drmGetMap(int fd, int idx, drm_handle_t *offset, drmSize *size,
     *size   = map.size;
     *type   = map.type;
     *flags  = map.flags;
-    *handle = (unsigned long)map.handle;
+    *handle = (uintptr_t)map.handle;
     *mtrr   = map.mtrr;
     return 0;
 }
-- 
1.7.0




[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

[PATCH] drm_handle_t type

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 2667 bytes --]

On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> Its probably not documented well anywhere, though I think the handles are
> 32-bit is written down somewhere.

Ah sorry, I missed some.

From a92f45396cd13f91ffd0bd1ea27250e5606184f8 Mon Sep 17 00:00:00 2001
From: Matthew W. S. Bell <matthew@bells23.org.uk>
Date: Fri, 9 Apr 2010 02:58:42 +0100		

Add a comment about content of void* variable exchanged with kernel.
Add casts to silence warnings about pointer to integer of a different
size assignment.
---
 include/drm/drm.h |    2 ++
 xf86drm.c         |   10 +++++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/drm/drm.h b/include/drm/drm.h
index 4822159..e74d6d6 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -216,6 +216,8 @@ struct drm_map {
 	enum drm_map_flags flags;	 /**< Flags */
 	void *handle;		 /**< User-space: "Handle" to pass to mmap() */
 				 /**< Kernel-space: kernel-virtual address */
+				 /**  The value herein shall be the size of */
+				 /**  drm_handle_t despite the size of void *. */
 	int mtrr;		 /**< MTRR slot used */
 	/*   Private data */
 };
diff --git a/xf86drm.c b/xf86drm.c
index 220aaa1..35576fa 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -959,7 +959,7 @@ int drmAddMap(int fd, drm_handle_t offset, drmSize size, drmMapType type,
     if (drmIoctl(fd, DRM_IOCTL_ADD_MAP, &map))
 	return -errno;
     if (handle)
-	*handle = (drm_handle_t)map.handle;
+	*handle = (uintptr_t)map.handle;
     return 0;
 }
 
@@ -967,7 +967,7 @@ int drmRmMap(int fd, drm_handle_t handle)
 {
     drm_map_t map;
 
-    map.handle = (void *)handle;
+    map.handle = (void *)(uintptr_t)handle;
 
     if(drmIoctl(fd, DRM_IOCTL_RM_MAP, &map))
 	return -errno;
@@ -2103,7 +2103,7 @@ int drmAddContextPrivateMapping(int fd, drm_context_t ctx_id,
     drm_ctx_priv_map_t map;
 
     map.ctx_id = ctx_id;
-    map.handle = (void *)handle;
+    map.handle = (void *)(uintptr_t)handle;
 
     if (drmIoctl(fd, DRM_IOCTL_SET_SAREA_CTX, &map))
 	return -errno;
@@ -2120,7 +2120,7 @@ int drmGetContextPrivateMapping(int fd, drm_context_t ctx_id,
     if (drmIoctl(fd, DRM_IOCTL_GET_SAREA_CTX, &map))
 	return -errno;
     if (handle)
-	*handle = (drm_handle_t)map.handle;
+	*handle = (uintptr_t)map.handle;
 
     return 0;
 }
@@ -2138,7 +2138,7 @@ int drmGetMap(int fd, int idx, drm_handle_t *offset, drmSize *size,
     *size   = map.size;
     *type   = map.type;
     *flags  = map.flags;
-    *handle = (unsigned long)map.handle;
+    *handle = (uintptr_t)map.handle;
     *mtrr   = map.mtrr;
     return 0;
 }
-- 
1.7.0



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 345 bytes --]

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] drm_handle_t type

[Robert Noland]

On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
> On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> > Its probably not documented well anywhere, though I think the handles are
> > 32-bit is written down somewhere.
> 
> Ah sorry, I missed some.

drm_handle_t is correct here... 

robert.

> From a92f45396cd13f91ffd0bd1ea27250e5606184f8 Mon Sep 17 00:00:00 2001
> From: Matthew W. S. Bell <matthew@bells23.org.uk>
> Date: Fri, 9 Apr 2010 02:58:42 +0100		
> 
> Add a comment about content of void* variable exchanged with kernel.
> Add casts to silence warnings about pointer to integer of a different
> size assignment.
> ---
>  include/drm/drm.h |    2 ++
>  xf86drm.c         |   10 +++++-----
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 4822159..e74d6d6 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -216,6 +216,8 @@ struct drm_map {
>  	enum drm_map_flags flags;	 /**< Flags */
>  	void *handle;		 /**< User-space: "Handle" to pass to mmap() */
>  				 /**< Kernel-space: kernel-virtual address */
> +				 /**  The value herein shall be the size of */
> +				 /**  drm_handle_t despite the size of void *. */
>  	int mtrr;		 /**< MTRR slot used */
>  	/*   Private data */
>  };
> diff --git a/xf86drm.c b/xf86drm.c
> index 220aaa1..35576fa 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -959,7 +959,7 @@ int drmAddMap(int fd, drm_handle_t offset, drmSize size, drmMapType type,
>      if (drmIoctl(fd, DRM_IOCTL_ADD_MAP, &map))
>  	return -errno;
>      if (handle)
> -	*handle = (drm_handle_t)map.handle;
> +	*handle = (uintptr_t)map.handle;
>      return 0;
>  }
>  
> @@ -967,7 +967,7 @@ int drmRmMap(int fd, drm_handle_t handle)
>  {
>      drm_map_t map;
>  
> -    map.handle = (void *)handle;
> +    map.handle = (void *)(uintptr_t)handle;
>  
>      if(drmIoctl(fd, DRM_IOCTL_RM_MAP, &map))
>  	return -errno;
> @@ -2103,7 +2103,7 @@ int drmAddContextPrivateMapping(int fd, drm_context_t ctx_id,
>      drm_ctx_priv_map_t map;
>  
>      map.ctx_id = ctx_id;
> -    map.handle = (void *)handle;
> +    map.handle = (void *)(uintptr_t)handle;
>  
>      if (drmIoctl(fd, DRM_IOCTL_SET_SAREA_CTX, &map))
>  	return -errno;
> @@ -2120,7 +2120,7 @@ int drmGetContextPrivateMapping(int fd, drm_context_t ctx_id,
>      if (drmIoctl(fd, DRM_IOCTL_GET_SAREA_CTX, &map))
>  	return -errno;
>      if (handle)
> -	*handle = (drm_handle_t)map.handle;
> +	*handle = (uintptr_t)map.handle;
>  
>      return 0;
>  }
> @@ -2138,7 +2138,7 @@ int drmGetMap(int fd, int idx, drm_handle_t *offset, drmSize *size,
>      *size   = map.size;
>      *type   = map.type;
>      *flags  = map.flags;
> -    *handle = (unsigned long)map.handle;
> +    *handle = (uintptr_t)map.handle;
>      *mtrr   = map.mtrr;
>      return 0;
>  }
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
-- 
Robert Noland <rnoland@2hip.net>
2Hip Networks


------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--

Re: [PATCH] drm_handle_t type

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 759 bytes --]

On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
> On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
> > On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> > > Its probably not documented well anywhere, though I think the handles are
> > > 32-bit is written down somewhere.
> > 
> > Ah sorry, I missed some.
> 
> drm_handle_t is correct here... 

No, drm_handle_t can be of a different size to void *; converting
between integers and pointers of different sizes causes a warning. To
eliminate the warning, the value first needs to be passed between
uintptr_t and void *, which are of the same size, and then converted to
drm_handle_t. The last part is implicit; the drm_handle_t casts are
irrelevant/useless.

Matthew

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm_handle_t type

[Robert Noland]

On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
> On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
> > On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
> > > On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> > > > Its probably not documented well anywhere, though I think the handles are
> > > > 32-bit is written down somewhere.
> > > 
> > > Ah sorry, I missed some.
> > 
> > drm_handle_t is correct here... 
> 
> No, drm_handle_t can be of a different size to void *; converting
> between integers and pointers of different sizes causes a warning. To
> eliminate the warning, the value first needs to be passed between
> uintptr_t and void *, which are of the same size, and then converted to
> drm_handle_t. The last part is implicit; the drm_handle_t casts are
> irrelevant/useless.

No, if you look at the BSD define for drm_handle_t, it does the right
thing on each arch.  The problem is that the linux define for
drm_handle_t is u32.

robert.

> Matthew
-- 
Robert Noland <rnoland@2hip.net>
2Hip Networks

Re: [PATCH] drm_handle_t type

[Robert Noland]

On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
> On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
> > On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
> > > On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> > > > Its probably not documented well anywhere, though I think the handles are
> > > > 32-bit is written down somewhere.
> > > 
> > > Ah sorry, I missed some.
> > 
> > drm_handle_t is correct here... 
> 
> No, drm_handle_t can be of a different size to void *; converting
> between integers and pointers of different sizes causes a warning. To
> eliminate the warning, the value first needs to be passed between
> uintptr_t and void *, which are of the same size, and then converted to
> drm_handle_t. The last part is implicit; the drm_handle_t casts are
> irrelevant/useless.

My point being that the casts to drm_handle_t are correct.  Feel free to
fix linux' define for drm_handle_t.

robert. 

> Matthew
-- 
Robert Noland <rnoland@2hip.net>
2Hip Networks

Re: [PATCH] drm_handle_t type

[Dave Airlie]

On Wed, Apr 14, 2010 at 1:32 PM, Robert Noland <rnoland@2hip.net> wrote:
> On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
>> On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
>> > On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
>> > > On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
>> > > > Its probably not documented well anywhere, though I think the handles are
>> > > > 32-bit is written down somewhere.
>> > >
>> > > Ah sorry, I missed some.
>> >
>> > drm_handle_t is correct here...
>>
>> No, drm_handle_t can be of a different size to void *; converting
>> between integers and pointers of different sizes causes a warning. To
>> eliminate the warning, the value first needs to be passed between
>> uintptr_t and void *, which are of the same size, and then converted to
>> drm_handle_t. The last part is implicit; the drm_handle_t casts are
>> irrelevant/useless.
>
> My point being that the casts to drm_handle_t are correct.  Feel free to
> fix linux' define for drm_handle_t.
>

Weird the drm is defined on Linux and is considered the standard, and
uint32_t is considered correct handle sizing on Linux.

I'm not sure why FreeBSD didn't change to 32-bit handles at the same
time, I think nobody wanted to break stuff by accident at the time,
assuming FreeBSD maintainers would take care of it.

Dave.

Re: [PATCH] drm_handle_t type

[Matthew W. S. Bell]

[-- Attachment #1.1: Type: text/plain, Size: 1257 bytes --]

On Tue, 2010-04-13 at 22:32 -0500, Robert Noland wrote:
> On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
> > On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
> > > On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
> > > > On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
> > > > > Its probably not documented well anywhere, though I think the handles are
> > > > > 32-bit is written down somewhere.
> > > > 
> > > > Ah sorry, I missed some.
> > > 
> > > drm_handle_t is correct here... 
> > 
> > No, drm_handle_t can be of a different size to void *; converting
> > between integers and pointers of different sizes causes a warning. To
> > eliminate the warning, the value first needs to be passed between
> > uintptr_t and void *, which are of the same size, and then converted to
> > drm_handle_t. The last part is implicit; the drm_handle_t casts are
> > irrelevant/useless.
> 
> My point being that the casts to drm_handle_t are correct.  Feel free to
> fix linux' define for drm_handle_t.

Warnings; warnings. The point is to get rid of the warnings. warnings?
Did I mention that when compiling a warning is emitted? Warnings. This
code just silences the warnings. 

Warnings.
Matthew


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm_handle_t type

[Robert Noland]

Dave Airlie wrote:
> On Wed, Apr 14, 2010 at 1:32 PM, Robert Noland <rnoland@2hip.net> wrote:
>> On Wed, 2010-04-14 at 00:19 +0100, Matthew W. S. Bell wrote:
>>> On Sun, 2010-04-11 at 09:10 -0500, Robert Noland wrote:
>>>> On Sat, 2010-04-10 at 19:30 +0100, Matthew W. S. Bell wrote:
>>>>> On Mon, 2010-04-05 at 17:46 +1000, Dave Airlie wrote:
>>>>>> Its probably not documented well anywhere, though I think the handles are
>>>>>> 32-bit is written down somewhere.
>>>>> Ah sorry, I missed some.
>>>> drm_handle_t is correct here...
>>> No, drm_handle_t can be of a different size to void *; converting
>>> between integers and pointers of different sizes causes a warning. To
>>> eliminate the warning, the value first needs to be passed between
>>> uintptr_t and void *, which are of the same size, and then converted to
>>> drm_handle_t. The last part is implicit; the drm_handle_t casts are
>>> irrelevant/useless.
>> My point being that the casts to drm_handle_t are correct.  Feel free to
>> fix linux' define for drm_handle_t.
>>
> 
> Weird the drm is defined on Linux and is considered the standard, and
> uint32_t is considered correct handle sizing on Linux.
> 
> I'm not sure why FreeBSD didn't change to 32-bit handles at the same
> time, I think nobody wanted to break stuff by accident at the time,
> assuming FreeBSD maintainers would take care of it.

drm_handle_t is passed directly to mmap() and on FreeBSD that is 
expected to be an offset into the device memory.  If drm_handle_t were 
only a handle, this would all be fine, but due to the fact that it's 
meaning is overloaded and is supposed to represent the KVA as well, it 
either needs to vary with the architecture or be 64bits...  Last time I 
looked at just making it 64bits everywhere, some minor fixups and API 
breakage would have been required in the DDX drivers, which was too much 
trouble to argue about...

The define on FreeBSD is an unsigned long, which works fine on 32/64 bit 
platforms with current DDX drivers and doesn't produce warnings.

robert.

> Dave.