[PATCH:libXt 4/6] Fix char vs. unsigned char warnings.

[Thomas Klausner]

On Thu, Jun 27, 2013 at 04:23:06AM -0400, Thomas E. Dickey wrote:
> On Thu, Jun 27, 2013 at 08:33:02AM +0200, Thomas Klausner wrote:
> > On Wed, Jun 26, 2013 at 08:55:35PM -0400, Thomas E. Dickey wrote:
> > > On Tue, Jun 25, 2013 at 11:02:48PM +0200, Thomas Klausner wrote:
> > > > ---
> > > >  src/ResConfig.c | 4 ++--
> > > >  src/TMparse.c   | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/ResConfig.c b/src/ResConfig.c
> > > > index 152d9cf..5a7f6d2 100644
> > > > --- a/src/ResConfig.c
> > > > +++ b/src/ResConfig.c
> > > > @@ -892,7 +892,7 @@ _XtResourceConfigurationEH (
> > > >  	int		actual_format;
> > > >  	unsigned long	nitems;
> > > >  	unsigned long	leftover;
> > > > -	unsigned char	*data = NULL;
> > > > +	char		*data = NULL;
> > > >  	unsigned long	resource_len;
> > > >  	char		*data_ptr;
> > > >  	char		*resource;
> > > > @@ -952,7 +952,7 @@ _XtResourceConfigurationEH (
> > > >  		pd->rcm_data, 0L, 8192L,
> > > >  		TRUE, XA_STRING,
> > > >  		&actual_type, &actual_format, &nitems, &leftover,
> > > > -		&data ) == Success && actual_type == XA_STRING
> > > > +		(unsigned char **)&data ) == Success && actual_type == XA_STRING
> > > >  			   && actual_format == 8) {
> > > 
> > > One problem with casts is that they're telling the compiler to ignore its type-checking.
> > > Casting an address like that happens to work - usually - but not always.
> > 
> > The problem is that the same variable is used in strtoul as first
> > argument, which wants a 'const char *', and in XGetWindowProperty as
> > 12th (if I didn't miscount), which wants an unsigned char **.
> 
> But taking the address of a pointer tends to get additional compiler warnings cautioning
> about alignment, etc.  I would have probably put the cast on the strtoul call.

So that we all know what we're talking about, I wanted to fix this
complaint from clang-3.4 (against the code without my patch):

ResConfig.c:967:10: warning: initializing 'char *' with an expression of type 'unsigned char *' converts between pointers to integer types with different sign [-Wpointer-sign]
                        char *data_end = data + nitems;
                              ^          ~~~~~~~~~~~~~
ResConfig.c:970:28: warning: passing 'unsigned char *' to parameter of type 'const char *' converts between pointers to integer types with different sign [-Wpointer-sign]
                        resource_len = strtoul (data, &data_ptr, 10);
                                                ^~~~
/usr/include/stdlib.h:125:34: note: passing argument to parameter here
         strtoul(const char * __restrict, char ** __restrict, int);
                                        ^
2 warnings generated.


The diff I sent is the smallest that gets rid of this issue. I also
tried leaving the type of 'data' alone, but then other variables need
to be changed and the diff gets larger, and we still need casts for
the reason above.

As for your question:

> You might
> want to compare results from these -
> 
> OPTS="-O -Wall -Wmissing-prototypes -Wstrict-prototypes -Wshadow -Wconversion
>     -W
> 	-Wbad-function-cast
> 	-Wcast-align
> 	-Wcast-qual
> 	-Wmissing-declarations
> 	-Wnested-externs
> 	-Wpointer-arith
> 	-Wwrite-strings
> 	-ansi
> 	-pedantic
>      "
> gcc $OPTS "$@"

  CC       ResConfig.lo
In file included from /usr/pkg/include/X11/Xlib.h:47:0,
                 from ../include/X11/Intrinsic.h:53,
                 from ResConfig.c:58:
/usr/pkg/include/X11/Xfuncproto.h:136:24: warning: ISO C does not permit named variadic macros
ResConfig.c: In function '_set_resource_values':
ResConfig.c:264:3: warning: cast discards qualifiers from pointer target type
ResConfig.c: In function '_search_widget_tree':
ResConfig.c:705:2: warning: conversion to 'int' from 'size_t' may alter its value
ResConfig.c:706:2: warning: conversion to 'int' from 'size_t' may alter its value
ResConfig.c: In function '_locate_children':
ResConfig.c:777:3: warning: conversion to 'Cardinal' from 'int' may change the sign of the result
ResConfig.c:777:3: warning: conversion to 'int' from 'Cardinal' may change the sign of the result
ResConfig.c:779:3: warning: conversion to 'Cardinal' from 'int' may change the sign of the result
ResConfig.c:779:3: warning: conversion to 'int' from 'Cardinal' may change the sign of the result
ResConfig.c:786:3: warning: conversion to 'unsigned int' from 'int' may change the sign of the result

> versus
> 
> OPTS="-Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wconversion"
> gcc $OPTS "$@"

  CC       ResConfig.lo
ResConfig.c: In function '_set_resource_values':
ResConfig.c:264:3: warning: cast discards qualifiers from pointer target type
ResConfig.c: In function '_search_widget_tree':
ResConfig.c:705:2: warning: conversion to 'int' from 'size_t' may alter its value
ResConfig.c:706:2: warning: conversion to 'int' from 'size_t' may alter its value
ResConfig.c: In function '_locate_children':
ResConfig.c:777:3: warning: conversion to 'Cardinal' from 'int' may change the sign of the result
ResConfig.c:777:3: warning: conversion to 'int' from 'Cardinal' may change the sign of the result
ResConfig.c:779:3: warning: conversion to 'Cardinal' from 'int' may change the sign of the result
ResConfig.c:779:3: warning: conversion to 'int' from 'Cardinal' may change the sign of the result
ResConfig.c:786:3: warning: conversion to 'unsigned int' from 'int' may change the sign of the result

The difference is:
In file included from /usr/pkg/include/X11/Xlib.h:47:0,
                 from ../include/X11/Intrinsic.h:53,
                 from ResConfig.c:58:
/usr/pkg/include/X11/Xfuncproto.h:136:24: warning: ISO C does not permit named variadic macros

which has nothing to do with the topic at hand.

 Thomas