Pointer cast warnings in scripts/
PATCH 1/1 Pointer cast warnings in scripts/
GCC 4 checks the signedness of pointer casts and generates a whole bunch of warnings for code in scripts/ (which makes heavy use of signed char strings).
This patch adds explicit casts.
scripts/basic/fixdep.c +2 -2
@@ -242,7 +242,7 @@continue ; found: -use_config (p+7 , q-p-7 ); +use_config ((char *)p+7 , q-p-7 ); } }
@@ -296,7 +296,7 @@signed char *p;char s[PATH_MAX ]; - p =strchr (m,':' ); + p = (signed char *)strchr ((char *)m,':' );if (!p) {fprintf (stderr,"fixdep: parse error\n" );exit (1 );
scripts/basic/docproc.c +5 -5
@@ -181,8 +181,8 @@while (fgets (line,MAXLINESZ , fp)) {signed char *p;signed char *e; -if (((p =strstr (line,"EXPORT_SYMBOL_GPL" )) !=0 ) || - ((p =strstr (line,"EXPORT_SYMBOL" )) !=0 )) { +if (((p = (signed char *)strstr (line,"EXPORT_SYMBOL_GPL" )) !=0 ) || + ((p = (signed char *)strstr (line,"EXPORT_SYMBOL" )) !=0 )) {/* Skip EXPORT_SYMBOL{_GPL} */ while (isalnum (*p) || *p =='_' ) p++;
@@ -199,7 +199,7 @@while (isalnum (*e) || *e =='_' ) e++; *e ='\0' ; -add_new_symbol (sym, p); +add_new_symbol (sym, (char *)p); } }fclose (fp);
@@ -271,7 +271,7 @@if (startofsym) { startofsym =0 ; vec[idx++] =FUNCTION ; - vec[idx++] = &line[i]; + vec[idx++] = (char *)&line[i]; } } vec[idx++] = filename;
@@ -293,7 +293,7 @@signed char * s;while (fgets (line,MAXLINESZ , infile)) {if (line[0 ] =='!' ) { - s = line +2 ; + s = (signed char *)(line +2 );switch (line[1 ]) {case 'E' :while (*s && !isspace (*s)) s++;
scripts/basic/split-include.c +1 -1
@@ -110,7 +110,7 @@if (line[0 ] !='#' )continue ; -if ((str_config =strstr (line,"CONFIG_" )) ==NULL ) +if ((str_config = (signed char *)strstr (line,"CONFIG_" )) ==NULL )continue ;/* Make the output file name. */
scripts/kconfig/mconf.c +12 -12
@@ -334,9 +334,9 @@int res;if (!*argptr) - *argptr = bufptr; + *argptr = (char *)bufptr;va_start (ap, fmt); - res =vsprintf (bufptr, fmt, ap); + res =vsprintf ((char *)bufptr, fmt, ap);va_end (ap); bufptr += res;
@@ -354,9 +354,9 @@va_list ap;int res; - *argptr++ = bufptr; + *argptr++ = (char *)bufptr;va_start (ap, fmt); - res =vsprintf (bufptr, fmt, ap); + res =vsprintf ((char *)bufptr, fmt, ap);va_end (ap); bufptr += res; *bufptr++ =0 ;
@@ -547,7 +547,7 @@return ; } - sym_arr =sym_re_search (input_buf); + sym_arr =sym_re_search ((char *)input_buf); res =get_relations_str (sym_arr);free (sym_arr);show_textbox (_ ("Search Results" ),str_get (&res),0 ,0 );
@@ -758,11 +758,11 @@if (i >=sizeof (active_entry)) i =sizeof (active_entry) -1 ; input_buf[i] =0 ; -strcpy (active_entry, input_buf); +strcpy (active_entry, (char *)input_buf); sym =NULL ; submenu =NULL ; -if (sscanf (input_buf +1 ,"%p" , &submenu) ==1 ) +if (sscanf ((char *)(input_buf +1 ),"%p" , &submenu) ==1 ) sym = submenu->sym;switch (stat) {
@@ -912,12 +912,12 @@ stat =exec_conf ();switch (stat) {case 0 : -if (sscanf (input_buf,"%p" , &child) !=1 ) +if (sscanf ((char *)input_buf,"%p" , &child) !=1 )break ;sym_set_tristate_value (child->sym, yes);return ;case 1 : -if (sscanf (input_buf,"%p" , &child) ==1 ) { +if (sscanf ((char *)input_buf,"%p" , &child) ==1 ) {show_help (child); active = child->sym; }else
@@ -958,7 +958,7 @@ stat =exec_conf ();switch (stat) {case 0 : -if (sym_set_string_value (menu->sym, input_buf)) +if (sym_set_string_value (menu->sym, (char *)input_buf))return ;show_textbox (NULL ,_ ("You have made an invalid entry." ),5 ,43 );break ;
@@ -987,7 +987,7 @@case 0 :if (!input_buf[0 ])return ; -if (!conf_read (input_buf)) +if (!conf_read ((char *)input_buf))return ;show_textbox (NULL ,_ ("File does not exist!" ),5 ,38 );break ;
@@ -1016,7 +1016,7 @@case 0 :if (!input_buf[0 ])return ; -if (!conf_write (input_buf)) +if (!conf_write ((char *)input_buf))return ;show_textbox (NULL ,_ ("Can't create file! Probably a nonexistent directory." ),5 ,60 );break ;
scripts/kconfig/confdata.c +5 -5
@@ -36,8 +36,8 @@ res_value[0 ] =0 ; dst = name; -while ((src =strchr (in,'$' ))) { -strncat (res_value, in, src - in); +while ((src = (const signed char *)strchr ((const char *)in,'$' ))) { +strncat (res_value, (const char *)in, src - in); src++; dst = name;while (isalnum (*src) || *src =='_' )
@@ -48,7 +48,7 @@strcat (res_value,sym_get_string_value (sym)); in = src; } -strcat (res_value, in); +strcat (res_value, (const char *)in);return res_value; }
@@ -59,7 +59,7 @@static char fullname[PATH_MAX +1 ];char *env, *name; - name =conf_expand_value (conf_defname); + name =conf_expand_value ((const signed char *)conf_defname); env =getenv (SRCTREE );if (env) {sprintf (fullname,"%s/%s" , env, name);
@@ -85,7 +85,7 @@ }else {const char **names = conf_confnames;while ((name = *names++)) { - name =conf_expand_value (name); + name =conf_expand_value ((const signed char *)name); in =zconf_fopen (name);if (in) { printf(_("#\n"
scripts/kconfig/conf.c +9 -9
@@ -43,7 +43,7 @@while ((isspace (*p))) p++; - l =strlen (p); + l =strlen ((char *)p);if (p != str)memmove (str, p, l +1 );if (!l)
@@ -91,7 +91,7 @@check_stdin ();case ask_all:fflush (stdout); -fgets (line,128 , stdin); +fgets ((char *)line,128 , stdin);return ;case set_default:printf ("%s\n" , def);
@@ -184,8 +184,8 @@break ; }default : - line[strlen (line)-1 ] =0 ; - def = line; + line[strlen ((char *)line)-1 ] =0 ; + def = (char *)line; }if (def &&sym_set_string_value (sym, def))return 0 ;
@@ -233,7 +233,7 @@case 'n' :case 'N' : newval = no; -if (!line[1 ] || !strcmp (&line[1 ],"o" )) +if (!line[1 ] || !strcmp ((char *)&line[1 ],"o" ))break ;continue ;case 'm' :
@@ -245,7 +245,7 @@case 'y' :case 'Y' : newval = yes; -if (!line[1 ] || !strcmp (&line[1 ],"es" )) +if (!line[1 ] || !strcmp ((char *)&line[1 ],"es" ))break ;continue ;case 0 :
@@ -347,7 +347,7 @@check_stdin ();case ask_all:fflush (stdout); -fgets (line,128 , stdin); +fgets ((char *)line,128 , stdin);strip (line);if (line[0 ] =='?' ) { printf("\n%s\n", menu->sym->help ?
@@ -357,7 +357,7 @@if (!line[0 ]) cnt = def;else if (isdigit (line[0 ])) - cnt =atoi (line); + cnt =atoi ((char *)line);else continue ;break ;
@@ -381,7 +381,7 @@ }if (!child)continue ; -if (line[strlen (line) -1 ] =='?' ) { +if (line[strlen ((char *)line) -1 ] =='?' ) {printf ("\n%s\n" , child->sym->help ? child->sym->help : nohelp_text);continue ;
Quoted 5 lines · Pierre Ossman
GCC 4 checks the signedness of pointer casts and generates a whole bunch of warnings for code in
scripts/(which makes heavy use of signed char strings).This patch adds explicit casts.
This is way ugly. The _real_ bug is that someone decided to store plain text pointers in "signed char*" variables. s/signed char/char/g should be a much better fix.
Quoted 15 lines
Signed-off-by Pierre Ossman <[email protected]>Index:
linux-wbsd/scripts/basic/fixdep.c
===================================================================scripts/basic/fixdep.c +1
-1@@ -242,7 +242,7 @@continue ; found: -use_config (p+7 , q-p-7 ); +use_config ((char *)p+7 , q-p-7 ); } }
scripts/basic/fixdep.c line 299
Quoted 5 lines
scripts/basic/fixdep.c +1
-1@@ -296,7 +296,7 @@signed char *p;char s[PATH_MAX ]; - p =strchr (m,':' ); + p = (signed char *)strchr ((char *)m,':' );
Clearly this is not the way to do text processing in C.
Hi,
Quoted 5 lines · Pierre Ossman
GCC 4 checks the signedness of pointer casts and generates a whole bunch of warnings for code in
scripts/(which makes heavy use of signed char strings).This patch adds explicit casts.
Just adding explicit all over the place is really the worst solution. Check if you can adjust data types/function prototypes. Lots of the signed stuff was added as a warning fix for Solaris, I'd rather drop that than adding casts all over the place. So you also may want to check the file history, why certain constructs were added.
bye, Roman
Quoted 7 lines · Roman Zippel
Just adding explicit all over the place is really the worst solution. Check if you can adjust data types/function prototypes. Lots of the signed stuff was added as a warning fix for Solaris, I'd rather drop that than adding casts all over the place. So you also may want to check the file history, why certain constructs were added.
Is there some easy way to check the file history? Downloading a couple of old kernels just for one file is a bit of a hassle. And I don't run bk so I can't access that repository (is it even still present after Linus' move?).
I'll start looking at removing the signed chars.
Rgds
Pierre
Hi,
Quoted 4 lines · Pierre Ossman
Is there some easy way to check the file history? Downloading a couple of old kernels just for one file is a bit of a hassle. And I don't run bk so I can't access that repository (is it even still present after Linus' move?).
http://linux.bkbits.net/ still works.
bye, Roman
Quoted 5 lines · Roman Zippel
http://linux.bkbits.net/ still works.
Thanks.
It should only be a matter of reversing the patches for Solaris then. But that will of course bring back the warnings on that platform. I'd say we should stick with what the standard says. Unfortunatly I don't have access to the standard so I wouldn't know which is the correct version. If I'd have to guess I'd say the odds are in favor of glibc getting it more right than solaris.
Should I extract the changes for bkbits and make a reversed patch?
Rgds
Pierre
On Tue, 2005-06-21 at 14:59 +0200, Pierre Ossman wrote:
[...]
Quoted 3 lines
It should only be a matter of reversing the patches for Solaris then. But that will of course bring back the warnings on that platform. I'd say we should stick with what the standard says. Unfortunatly I don't
The C-standard about "char", "signed char" and "unsigned char"? These are 3 different types.
Bernd
Signature · 3 lines
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and ServicesQuoted 6 lines · Bernd Petrovitsch
The C-standard about "char", "signed char" and "unsigned char"? These are 3 different types.
I was referring to which of the three types is correct for str*().
Rgds
Pierre
Quoted 5 lines · Pierre Ossman
Quoted 2 lines · Bernd Petrovitsch
The C-standard about "char", "signed char" and "unsigned char"? These are 3 different types.
I was referring to which of the three types is correct for str*().
"char" as one can read in every man-page.
Bernd
Signature · 3 lines
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and ServicesQuoted 11 lines · Bernd Petrovitsch
Quoted 3 lines · Pierre Ossman
I was referring to which of the three types is correct for str*().
"char" as one can read in every man-page.
That doesn't really make it a standard though (de facto perhaps). :) The odds of all those man pages deviating from the standard is probably very low. But unless someone has actually read the damn thing we won't know for sure.
Rgds
Pierre
Quoted 4 lines · Pierre Ossman
That doesn't really make it a standard though (de facto perhaps). :) The odds of all those man pages deviating from the standard is probably very low. But unless someone has actually read the damn thing we won't know for sure.
You could check the C99 spec of course, which says gives (eg) strcmp as:
int strcmp(const char *s1, const char *s2);
rmk's rules of char:
1. use char for character strings and individual characters
2. use signed char if your data type relies upon negative char values
3. use unsigned char if your data type does not require negative char values, especially if it makes use of the positive values not present in the signed char range.
IOW, use signed/unsigned when you need to explicitly state your requirements, but omit it for true strings and characters.
Signature · 3 lines
Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
Hi,
Quoted 1 line · Pierre Ossman
Should I extract the changes for bkbits and make a reversed patch?
No, go through the warnings, analyze each one and choose an appropriate solution. You might want to keep notes, which you can post with the changelogs, so one can reproduce, why a certain change was done.
bye, Roman
Quoted 7 lines · Roman Zippel
No, go through the warnings, analyze each one and choose an appropriate solution. You might want to keep notes, which you can post with the changelogs, so one can reproduce, why a certain change was done.
The problem is that they're mostly calls to library functions (strlen, strcmp, fgets, etc.) so it's either the solaris way or the glibc way.
A (somewhat unclean) solution is to make the type change based on the platform. Are there any defines present to test if we're in a Solaris environment? I don't have access to any Solaris machines myself so I can't really test.
Rgds
Pierre
Hi,
Quoted 4 lines · Pierre Ossman
A (somewhat unclean) solution is to make the type change based on the platform. Are there any defines present to test if we're in a Solaris environment? I don't have access to any Solaris machines myself so I can't really test.
Just ignore it. If someone really cares, he has to redo the Solaris specific changes properly (or live with warnings).
bye, Roman
Quoted 4 lines · Roman Zippel
Hi,
On Tue, 21 Jun 2005, Pierre Ossman wrote:
Quoted 8 lines
Quoted 4 lines
A (somewhat unclean) solution is to make the type change based on the platform. Are there any defines present to test if we're in a Solaris environment? I don't have access to any Solaris machines myself so I can't really test.
Just ignore it. If someone really cares, he has to redo the Solaris specific changes properly (or live with warnings).
pilgor:~> uname -a
SunOS pilgor 5.9 Generic_117171-07 sun4u sparc SUNW,Sun-Fire-V440 Solaris pilgor:~> man strcpy
...
int strcmp(const char *s1, const char *s2);
int strncmp(const char *s1, const char *s2, size_t n);
char *strcpy(char *s1, const char *s2);
char *strncpy(char *s1, const char *s2, size_t n);They look normal...
What is the problem with solaris ?
--
J.A. Magallon <jamagallon()able!es> \ Software is like sex:
werewolf!able!es \ It's better when it's free
Mandriva Linux release 2006.0 (Cooker) for i586
Linux 2.6.12-jam1 (gcc 4.0.1 (4.0.1-0.2mdk for Mandriva Linux release 2006.0))
Quoted 10 lines · Roman Zippel
Hi,
Quoted 1 line · Pierre Ossman
Should I extract the changes for bkbits and make a reversed patch?
No, go through the warnings, analyze each one and choose an appropriate solution. You might want to keep notes, which you can post with the changelogs, so one can reproduce, why a certain change was done.
I started doing that, and the more warnings are related to strxxx functions. They are defined as receiving 'char *' pointers, I do not know why someone started to fill the sources with 'signed char *' for text strings...
I did not finish the cleaup due to lack of time, but I was planning to continue...
--
J.A. Magallon <jamagallon()able!es> \ Software is like sex:
werewolf!able!es \ It's better when it's free
Mandriva Linux release 2006.0 (Cooker) for i586
Linux 2.6.12-jam1 (gcc 4.0.1 (4.0.1-0.2mdk for Mandriva Linux release 2006.0))
Quoted 3 lines · Pierre Ossman
Is there some easy way to check the file history? Downloading a couple of old kernels just for one file is a bit of a hassle. And I don't run bk so I can't access that repository
You can use the web interface (bkbits.net).
Andreas.
Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."