error, types differ in signedness

error, types differ in signedness

[Randy Dunlap]

Hi,

On Linux 2.6.20 and current sparse, I see these warnings:

net/core/skbuff.c:1838:5: error: symbol 'skb_append_datato_frags' redeclared with different type (originally declared at include/linux/skbuff.h:380) - incompatible argument 3 (different signedness)

fs/mpage.c:821:5: error: symbol 'mpage_writepage' redeclared with different type (originally declared at include/linux/mpage.h:22) - incompatible argument 2 (different signedness)



These are both in function pointers that are arguments to a function.
The skbuff.[hc] difference is in *getfrag vs. getfrag.
The mpage.[hc] difference is in  *get_block vs. getblock.

Questions:
1.  Is the warning valid?
2.  Isn't the '*' unnecessary?

Thanks,
---
~Randy

[PATCH] Re: error, types differ in signedness

[Christopher Li]

On Mon, Feb 05, 2007 at 10:54:20AM -0800, Randy Dunlap wrote:
> The skbuff.[hc] difference is in *getfrag vs. getfrag.
> The mpage.[hc] difference is in  *get_block vs. getblock.
> 
> Questions:
> 1.  Is the warning valid?

I don't think so.

> 2.  Isn't the '*' unnecessary?

It is valid in C without '*'. Sparse already know the function
is going to degenerated into function pointers. It wrongly inherent
the signedness. You get a signed pointer.

Can you please try this patch?

Chris

Index: sparse/symbol.h
===================================================================
--- sparse.orig/symbol.h	2007-02-04 23:46:07.000000000 -0800
+++ sparse/symbol.h	2007-02-05 12:18:30.000000000 -0800
@@ -191,6 +191,7 @@ struct symbol {
 #define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG)
 #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
 	MOD_ASSIGNED | MOD_USERTYPE | MOD_FORCE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
+#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
 
 
 /* Current parsing/evaluation function */
Index: sparse/evaluate.c
===================================================================
--- sparse.orig/evaluate.c	2007-02-04 00:47:46.000000000 -0800
+++ sparse/evaluate.c	2007-02-05 12:20:08.000000000 -0800
@@ -1282,7 +1282,7 @@ static void examine_fn_arguments(struct 
 				else
 					ptr->ctype.base_type = arg;
 				ptr->ctype.as |= s->ctype.as;
-				ptr->ctype.modifiers |= s->ctype.modifiers;
+				ptr->ctype.modifiers |= s->ctype.modifiers & MOD_PTRINHERIT;
 
 				s->ctype.base_type = ptr;
 				s->ctype.as = 0;
@@ -1313,8 +1313,6 @@ static struct symbol *convert_to_as_mod(
 	return sym;
 }
 
-#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
-
 static struct symbol *create_pointer(struct expression *expr, struct symbol *sym, int degenerate)
 {
 	struct symbol *node = alloc_symbol(expr->pos, SYM_NODE);

Re: [PATCH] Re: error, types differ in signedness

[Randy Dunlap]

On Mon, 5 Feb 2007 12:06:17 -0800 Christopher Li wrote:

> On Mon, Feb 05, 2007 at 10:54:20AM -0800, Randy Dunlap wrote:
> > The skbuff.[hc] difference is in *getfrag vs. getfrag.
> > The mpage.[hc] difference is in  *get_block vs. getblock.
> > 
> > Questions:
> > 1.  Is the warning valid?
> 
> I don't think so.
> 
> > 2.  Isn't the '*' unnecessary?
> 
> It is valid in C without '*'. Sparse already know the function
> is going to degenerated into function pointers. It wrongly inherent
> the signedness. You get a signed pointer.
> 
> Can you please try this patch?

Yes, that fixes the problems that I was seeing.
Thanks!


> Index: sparse/symbol.h
> ===================================================================
> --- sparse.orig/symbol.h	2007-02-04 23:46:07.000000000 -0800
> +++ sparse/symbol.h	2007-02-05 12:18:30.000000000 -0800
> @@ -191,6 +191,7 @@ struct symbol {
>  #define MOD_SIZE	(MOD_CHAR | MOD_SHORT | MOD_LONG | MOD_LONGLONG)
>  #define MOD_IGNORE (MOD_TOPLEVEL | MOD_STORAGE | MOD_ADDRESSABLE |	\
>  	MOD_ASSIGNED | MOD_USERTYPE | MOD_FORCE | MOD_ACCESSED | MOD_EXPLICITLY_SIGNED)
> +#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
>  
>  
>  /* Current parsing/evaluation function */
> Index: sparse/evaluate.c
> ===================================================================
> --- sparse.orig/evaluate.c	2007-02-04 00:47:46.000000000 -0800
> +++ sparse/evaluate.c	2007-02-05 12:20:08.000000000 -0800
> @@ -1282,7 +1282,7 @@ static void examine_fn_arguments(struct 
>  				else
>  					ptr->ctype.base_type = arg;
>  				ptr->ctype.as |= s->ctype.as;
> -				ptr->ctype.modifiers |= s->ctype.modifiers;
> +				ptr->ctype.modifiers |= s->ctype.modifiers & MOD_PTRINHERIT;
>  
>  				s->ctype.base_type = ptr;
>  				s->ctype.as = 0;
> @@ -1313,8 +1313,6 @@ static struct symbol *convert_to_as_mod(
>  	return sym;
>  }
>  
> -#define MOD_PTRINHERIT (MOD_VOLATILE | MOD_CONST | MOD_NODEREF | MOD_STORAGE)
> -
>  static struct symbol *create_pointer(struct expression *expr, struct symbol *sym, int degenerate)
>  {
>  	struct symbol *node = alloc_symbol(expr->pos, SYM_NODE);
> -

---
~Randy

Re: [PATCH] Re: error, types differ in signedness

[Pavel Roskin]

Chris,

On Mon, 2007-02-05 at 12:06 -0800, Christopher Li wrote:
> On Mon, Feb 05, 2007 at 10:54:20AM -0800, Randy Dunlap wrote:
> > The skbuff.[hc] difference is in *getfrag vs. getfrag.
> > The mpage.[hc] difference is in  *get_block vs. getblock.

I remember having this problem in MadWifi, and it was hard to understand
what was going on.  I had to put the declarations back-to-back and look
at them very carefully.

> > Questions:
> > 1.  Is the warning valid?
> 
> I don't think so.

I think any difference between declarations other than missing argument
names is an example of sloppy coding and should be avoided.

> > 2.  Isn't the '*' unnecessary?
> 
> It is valid in C without '*'. Sparse already know the function
> is going to degenerated into function pointers. It wrongly inherent
> the signedness. You get a signed pointer.
> 
> Can you please try this patch?

I would prefer that sparse emits a specific warning about function vs
function pointer mismatch.  Even if it doesn't matter, it makes the code
easier to read and safer to modify.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Re: error, types differ in signedness

[Christopher Li]

On Mon, Feb 05, 2007 at 05:27:11PM -0500, Pavel Roskin wrote:
> I think any difference between declarations other than missing argument
> names is an example of sloppy coding and should be avoided.

The original warning about pointer signedness difference is simply wrong.

There might be value to have warning about function vs function pointer. 
On the other hand, some attribute can be only specified in function prototype
declares but not the function body.

> I would prefer that sparse emits a specific warning about function vs
> function pointer mismatch.  Even if it doesn't matter, it makes the code
> easier to read and safer to modify.

I did not find a easy way to do it yet.

Chris

Re: [PATCH] Re: error, types differ in signedness

[Pavel Roskin]

Quoting Christopher Li <sparse@chrisli.org>:

> > I would prefer that sparse emits a specific warning about function vs
> > function pointer mismatch.  Even if it doesn't matter, it makes the code
> > easier to read and safer to modify.
>
> I did not find a easy way to do it yet.

I see.  I thought it was easy.  Then your patch is fine for me.  It's better to
have no warning than a misleading warning about a stylistic issue.

--
Regards,
Pavel Roskin

Re: [PATCH] Re: error, types differ in signedness

[Christopher Li]

On Mon, Feb 05, 2007 at 08:12:55PM -0500, Pavel Roskin wrote:
> 
> I see.  I thought it was easy.  Then your patch is fine for me.  It's better to
> have no warning than a misleading warning about a stylistic issue.

Sparse care about type difference. What you are asking is not difference
in type. It is the difference in which path it took to reach that type.
E.g. using default degenerating rules or typedefs. Sparse does not
store that information.

Chris