[lib/libX11] Don't use caddr_t casts

Submitted by Jon Turney on Feb. 19, 2012, 2:31 p.m.

Details

Message ID 4F4107DA.2030308@dronecode.org.uk
State Accepted, archived
Commit ed00b460acb08787b695f27b864e96102dfd4867
Headers show

Not browsing as part of any series.

Commit Message

Jon Turney Feb. 19, 2012, 2:31 p.m. 
On 18/02/2012 19:11, Jamey Sharp wrote:
> On Sat, Feb 18, 2012 at 05:21:24PM +0000, Jon TURNEY wrote:
>> Casting a (const char *) to (caddr_t) to assign to iovec.io_base
>> seems pointless. caddr_t isn't used anywhere else in xcb or libX11
> 
> According to the libxcb git history, I replaced (caddr_t) with (char *)
> in 2006 to "help DragonFly and Solaris". I'd be fine with this patch if
> it explicitly cast to (char *), which I believe suppresses the constness
> warning.

Updated patch attached.

But you still get a warning that you are discarding constness, because you are :-)

Using an explicit (char *) cast just changes the warning from:

/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c: In function '_XSend':
/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:481:18: warning: assignment
discards qualifiers from pointer target type
/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:483:18: warning: assignment
discards qualifiers from pointer target type

to:

/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c: In function '_XSend':
/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:481:2: warning: cast
discards qualifiers from pointer target type [-Wcast-qual]
/opt/wip/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:483:2: warning: cast
discards qualifiers from pointer target type [-Wcast-qual]

>> Note: there's a warning about dropping constness here, but that's
>> going to be unfixable as long as xcb_writev() takes a non-const struct
>> iovec as a parameter.
> 
> C's rules regarding const always confuse me, but I'm pretty sure even
> that wouldn't help. If the struct iovec is declared const, that just
> means XCB won't change where the iov_base pointers point. It doesn't
> mean XCB promises to refrain from writing through those pointers.

You are correct, I mis-stated the issue here. You are always going to get a
warning here because struct iovec can't represent the constness correctly.

I'm guessing xcb functions don't modify the iovec itself, so it could be
const, but that's a separate issue.
From 616b7284ee91c39450f98ac5c93a9e719b148ffd Mon Sep 17 00:00:00 2001
From: Jon TURNEY <jon.turney@dronecode.org.uk>
Date: Fri, 28 Oct 2011 11:09:20 -0500
Subject: [PATCH lib/libX11] Don't use caddr_t casts

(caddr_t) isn't used anywhere else in xcb or libX11.
Cast to (char *) for consistency.

Removing this cast allows building for MinGW without patching.

v2: Cast to (char *) rather than just dropping the cast

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 src/xcb_io.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/src/xcb_io.c b/src/xcb_io.c
index 0af47d8..300ef57 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -478,9 +478,9 @@  void _XSend(Display *dpy, const char *data, long size)
 
 	vec[0].iov_base = dpy->buffer;
 	vec[0].iov_len = dpy->bufptr - dpy->buffer;
-	vec[1].iov_base = (caddr_t) data;
+	vec[1].iov_base = (char *)data;
 	vec[1].iov_len = size;
-	vec[2].iov_base = (caddr_t) pad;
+	vec[2].iov_base = (char *)pad;
 	vec[2].iov_len = -size & 3;
 
 	for(ext = dpy->flushes; ext; ext = ext->next_flush)

Comments

Alan Coopersmith March 9, 2012, 4:45 a.m. 
On 02/19/12 06:31 AM, Jon TURNEY wrote:
> On 18/02/2012 19:11, Jamey Sharp wrote:
>> On Sat, Feb 18, 2012 at 05:21:24PM +0000, Jon TURNEY wrote:
>>> Casting a (const char *) to (caddr_t) to assign to iovec.io_base
>>> seems pointless. caddr_t isn't used anywhere else in xcb or libX11
>>
>> According to the libxcb git history, I replaced (caddr_t) with (char *)
>> in 2006 to "help DragonFly and Solaris". I'd be fine with this patch if
>> it explicitly cast to (char *), which I believe suppresses the constness
>> warning.

BTW, Solaris 11 now uses (void *), but Solaris 10 & older used caddr_t there,
which is why you needed it back then.

> diff --git a/src/xcb_io.c b/src/xcb_io.c
> index 0af47d8..300ef57 100644
> --- a/src/xcb_io.c
> +++ b/src/xcb_io.c
> @@ -478,9 +478,9 @@ void _XSend(Display *dpy, const char *data, long size)
>
>  	vec[0].iov_base = dpy->buffer;
>  	vec[0].iov_len = dpy->bufptr - dpy->buffer;
> -	vec[1].iov_base = (caddr_t) data;
> +	vec[1].iov_base = (char *)data;
>  	vec[1].iov_len = size;
> -	vec[2].iov_base = (caddr_t) pad;
> +	vec[2].iov_base = (char *)pad;
>  	vec[2].iov_len = -size & 3;
>
>  	for(ext = dpy->flushes; ext; ext = ext->next_flush)

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Jon Turney March 11, 2012, 5:54 p.m. 
On 09/03/2012 04:45, Alan Coopersmith wrote:
> On 02/19/12 06:31 AM, Jon TURNEY wrote:
>> On 18/02/2012 19:11, Jamey Sharp wrote:
>>> On Sat, Feb 18, 2012 at 05:21:24PM +0000, Jon TURNEY wrote:
>>>> Casting a (const char *) to (caddr_t) to assign to iovec.io_base
>>>> seems pointless. caddr_t isn't used anywhere else in xcb or libX11
>>>
>>> According to the libxcb git history, I replaced (caddr_t) with (char *)
>>> in 2006 to "help DragonFly and Solaris". I'd be fine with this patch if
>>> it explicitly cast to (char *), which I believe suppresses the constness
>>> warning.
> 
> BTW, Solaris 11 now uses (void *), but Solaris 10 & older used caddr_t there,
> which is why you needed it back then.

This sounds a bit odd to me, that cast would only be needed if caddr_t was
defined as something which was incompatible with having a char * assigned to it?

>> diff --git a/src/xcb_io.c b/src/xcb_io.c
>> index 0af47d8..300ef57 100644
>> --- a/src/xcb_io.c
>> +++ b/src/xcb_io.c
>> @@ -478,9 +478,9 @@ void _XSend(Display *dpy, const char *data, long size)
>>
>>      vec[0].iov_base = dpy->buffer;
>>      vec[0].iov_len = dpy->bufptr - dpy->buffer;
>> -    vec[1].iov_base = (caddr_t) data;
>> +    vec[1].iov_base = (char *)data;
>>      vec[1].iov_len = size;
>> -    vec[2].iov_base = (caddr_t) pad;
>> +    vec[2].iov_base = (char *)pad;
>>      vec[2].iov_len = -size & 3;
>>
>>      for(ext = dpy->flushes; ext; ext = ext->next_flush)
> 
> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Thanks. I've pushed this.
   20adca0..ed00b46  master -> master
Alan Coopersmith March 12, 2012, 2:43 a.m. 
On 03/11/12 10:54 AM, Jon TURNEY wrote:
> On 09/03/2012 04:45, Alan Coopersmith wrote:
>> On 02/19/12 06:31 AM, Jon TURNEY wrote:
>>> On 18/02/2012 19:11, Jamey Sharp wrote:
>>>> On Sat, Feb 18, 2012 at 05:21:24PM +0000, Jon TURNEY wrote:
>>>>> Casting a (const char *) to (caddr_t) to assign to iovec.io_base
>>>>> seems pointless. caddr_t isn't used anywhere else in xcb or libX11
>>>>
>>>> According to the libxcb git history, I replaced (caddr_t) with (char *)
>>>> in 2006 to "help DragonFly and Solaris". I'd be fine with this patch if
>>>> it explicitly cast to (char *), which I believe suppresses the constness
>>>> warning.
>>
>> BTW, Solaris 11 now uses (void *), but Solaris 10&  older used caddr_t there,
>> which is why you needed it back then.
>
> This sounds a bit odd to me, that cast would only be needed if caddr_t was
> defined as something which was incompatible with having a char * assigned to it?

caddr_t is typedef'ed to char * on Solaris - the explanation is why char * was 
needed instead of the void * preferred for iovec members on modern OS'es.