Re: [PATCH] signed char fixes for scripts

[Paulo Marques <pmarques@grupopie.com>]

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

Bernd Petrovitsch wrote:
> On Thu, 2005-07-28 at 11:02 +0100, Paulo Marques wrote:
>>J.A. Magallon wrote:
>>[...]
>>>All the problems are born here:
>>>
>>>struct sym_entry {
>>>    unsigned long long addr;
>>>    unsigned int len;
>>>    unsigned char *sym;
>>>};
>>
>>What are you guys talking about?
> 
> "unsigned char *" is simply the wrong type for mere text strings. "char
> *" ist the corrrect one. These are BTW two completely different types
> (yes, "char" can be promoted into an "unsigned char" but essentially
> these are two completely different types like "int" and "long long *").

You're comming really late in this thread :)

The problem is that "sym" isn't really a string. It starts out as a 
string, but as the compression scheme begins to work it just becomes a 
"bunch of bytes" using all the values in the range 0-255 for which 
unsigned char is the perfect type.

Since only the loading of the symbols use string functions, and all the 
compression process treats these as bytes, it seemed better to treat 
them as unsigned chars and just typecast the first few uses.

The union suggested by J.A.Magallon might be a reasonable solution, but 
we only need 4 casts in the 500 lines of code of scripts/kallsyms.c to 
solve all problems, so this seems really overkill.

>>Is my compiler version the problem (3.3.2), or are you testing with the 
> 
> Compiler version - zse gcc-4.*.

Yes, I know J.A.Magallon is trying to silence the warnings of gcc 4.0, 
but as I understood it, gcc 3 would also complain of the same problems 
if -Wsign-compare were specified. It was just that gcc4 would complain 
even without -Wsign-compare.

So the question is: is gcc4 complaining about signedness problems that 
gcc3 doesn't, even with -Wsign-compare?

Now that I look at the source, I can see that it must be complaining! 
There are still 3 calls to strcmp that use sym directly, and gcc3 
doesn't say a thing.

I thought that these were already taken care of. In any cased the 
attached patch should fix those and make the code more readable too. 
With this patch we end up only having 2 casts to (char *) in the whole 
source.

Can someone with gcc 4 apply this to the latest -mm and check that it 
fixes everything?

-- 
Paulo Marques - www.grupopie.com

It is a mistake to think you can solve any major problems
just with potatoes.
Douglas Adams

[-- Attachment #2: kallsyms_sign.patch --]
[-- Type: text/x-patch, Size: 1503 bytes --]

--- ./scripts/kallsyms.c.orig	2005-07-28 11:31:29.000000000 +0100
+++ ./scripts/kallsyms.c	2005-07-28 11:33:58.000000000 +0100
@@ -150,11 +150,13 @@ static int symbol_valid(struct sym_entry
 		"_SDA2_BASE_",		/* ppc */
 		NULL };
 	int i;
-	int offset = 1;
+	char *name;
+
+	name = (char *) s->sym + 1;
 
 	/* skip prefix char */
-	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
-		offset++;
+	if (symbol_prefix_char && *name == symbol_prefix_char)
+		name++;
 
 	/* if --all-symbols is not specified, then symbols outside the text
 	 * and inittext sections are discarded */
@@ -169,18 +171,18 @@ static int symbol_valid(struct sym_entry
 		 * move then they may get dropped in pass 2, which breaks the
 		 * kallsyms rules.
 		 */
-		if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
-		    (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")) ||
-		    (s->addr == _eextratext && strcmp(s->sym + offset, "_eextratext")))
+		if ((s->addr == _etext && strcmp(name, "_etext")) ||
+		    (s->addr == _einittext && strcmp(name, "_einittext")) ||
+		    (s->addr == _eextratext && strcmp(name, "_eextratext")))
 			return 0;
 	}
 
 	/* Exclude symbols which vary between passes. */
-	if (strstr((char *)s->sym + offset, "_compiled."))
+	if (strstr(name, "_compiled."))
 		return 0;
 
 	for (i = 0; special_symbols[i]; i++)
-		if( strcmp((char *)s->sym + offset, special_symbols[i]) == 0 )
+		if( strcmp(name, special_symbols[i]) == 0 )
 			return 0;
 
 	return 1;