[PATCH] Pointer cast warnings in scripts/

[PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 236 bytes --]

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.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

[-- Attachment #2: signed-char-warnings.patch --]
[-- Type: text/x-patch, Size: 7893 bytes --]

Index: linux-wbsd/scripts/basic/fixdep.c
===================================================================
--- linux-wbsd/scripts/basic/fixdep.c	(revision 134)
+++ linux-wbsd/scripts/basic/fixdep.c	(working copy)
@@ -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);
Index: linux-wbsd/scripts/basic/docproc.c
===================================================================
--- linux-wbsd/scripts/basic/docproc.c	(revision 134)
+++ linux-wbsd/scripts/basic/docproc.c	(working copy)
@@ -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++;
Index: linux-wbsd/scripts/basic/split-include.c
===================================================================
--- linux-wbsd/scripts/basic/split-include.c	(revision 134)
+++ linux-wbsd/scripts/basic/split-include.c	(working copy)
@@ -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. */
Index: linux-wbsd/scripts/kconfig/mconf.c
===================================================================
--- linux-wbsd/scripts/kconfig/mconf.c	(revision 153)
+++ linux-wbsd/scripts/kconfig/mconf.c	(working copy)
@@ -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;
Index: linux-wbsd/scripts/kconfig/confdata.c
===================================================================
--- linux-wbsd/scripts/kconfig/confdata.c	(revision 153)
+++ linux-wbsd/scripts/kconfig/confdata.c	(working copy)
@@ -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"
Index: linux-wbsd/scripts/kconfig/conf.c
===================================================================
--- linux-wbsd/scripts/kconfig/conf.c	(revision 153)
+++ linux-wbsd/scripts/kconfig/conf.c	(working copy)
@@ -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;

Re: [PATCH] Pointer cast warnings in scripts/

[Mikael Pettersson]

Pierre Ossman writes:
 > 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.

 > Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
 > Index: linux-wbsd/scripts/basic/fixdep.c
 > ===================================================================
 > --- linux-wbsd/scripts/basic/fixdep.c	(revision 134)
 > +++ linux-wbsd/scripts/basic/fixdep.c	(working copy)
 > @@ -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, ':');

Clearly this is not the way to do text processing in C.

Re: [PATCH] Pointer cast warnings in scripts/

[Roman Zippel]

Hi,

On Tue, 21 Jun 2005, Pierre Ossman wrote:

> 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

Re: [PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

Roman Zippel wrote:

>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

Re: [PATCH] Pointer cast warnings in scripts/

[Roman Zippel]

Hi,

On Tue, 21 Jun 2005, Pierre Ossman wrote:

> 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

Re: [PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

Roman Zippel wrote:

>
>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

Re: [PATCH] Pointer cast warnings in scripts/

[Bernd Petrovitsch]

On Tue, 2005-06-21 at 14:59 +0200, Pierre Ossman wrote:
[...]
> 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
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: [PATCH] Pointer cast warnings in scripts/

[Roman Zippel]

Hi,

On Tue, 21 Jun 2005, Pierre Ossman wrote:

> 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

Re: [PATCH] Pointer cast warnings in scripts/

[Andreas Schwab]

Pierre Ossman <drzeus-list@drzeus.cx> writes:

> 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.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
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."

Re: [PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

Roman Zippel wrote:

>
>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

Re: [PATCH] Pointer cast warnings in scripts/

[Roman Zippel]

Hi,

On Tue, 21 Jun 2005, Pierre Ossman wrote:

> 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

Re: [PATCH] Pointer cast warnings in scripts/

[J.A. Magallon]

On 06.21, Roman Zippel wrote:
> Hi,
> 
> On Tue, 21 Jun 2005, Pierre Ossman wrote:
> 
> > 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))

Re: [PATCH] Pointer cast warnings in scripts/

[J.A. Magallon]

On 06.21, Roman Zippel wrote:
> Hi,
> 
> On Tue, 21 Jun 2005, Pierre Ossman wrote:
> 
> > 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))

Re: [PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

Bernd Petrovitsch wrote:

>
>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

Re: [PATCH] Pointer cast warnings in scripts/

[Bernd Petrovitsch]

On Wed, 2005-06-22 at 11:21 +0200, Pierre Ossman wrote:
> Bernd Petrovitsch wrote:
> >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
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: [PATCH] Pointer cast warnings in scripts/

[Pierre Ossman]

Bernd Petrovitsch wrote:

>On Wed, 2005-06-22 at 11:21 +0200, Pierre Ossman wrote:
>  
>
>>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

Re: [PATCH] Pointer cast warnings in scripts/

[Russell King]

On Wed, Jun 22, 2005 at 01:12:06PM +0200, Pierre Ossman wrote:
> 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.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core