[PATCH] Warn about explicit usage of sizeof(void)

[PATCH] Warn about explicit usage of sizeof(void)

[Christopher Li]

On Wed, Dec 24, 2008 at 4:15 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Wed, Dec 24, 2008 at 3:35 PM, Tommy Thorn <tommy@numba-tu.com> wrote:
>>
>> You can't have one without the other as you will break identities like
>>
>>  (uintptr_t) (x + k) === (uintptr_t) x + sizeof (typeof (x)) * k
>>
>> which could appear in a macro.
>
> Right. That is exactly the place I actuall want to know. We should
> consider fix that in the source to have proper type. I expect there is
> not much place in the kernel use that.

I think you do have a point that from the "spase as a compiler" point
of view, there is need to maintain the compatibility with gcc. And I do
care about the current patch breaking the assumption of using
void_ctype.bit_size.

So here is what I got. A patch address both of our need. It gives warning
of using sizeof(void) explicitly. void* + offset will continue to work without
warnings. It will also make is_byte_type() continue to work as it was
before.

Here is my test script:

void *p;

int i = sizeof(void);
int j = sizeof(*p);

void* foo(void)
{
	return p + 1;
}

I run the kernel compile with C=2, guess what. None of the current
kernel code use sizeof(void) explicitly. So that is good news.

Every one happy?

Chris


============================================================

Warn about explicit usage of sizeof(void)

sizeof(void) still evaluate as 1 after the warning.
void_ctype.bit_size remain zero so is_byte_type()
will continue to work.

Signed-Off-By: Christopher Li <sparse@chrisli.org>

Index: sparse.chrisl/evaluate.c
===================================================================
--- sparse.chrisl.orig/evaluate.c
+++ sparse.chrisl/evaluate.c
@@ -584,7 +584,7 @@ static struct symbol *evaluate_ptr_add(s
 	}

 	/* Get the size of whatever the pointer points to */
-	multiply = bits_to_bytes(base->bit_size);
+	multiply = (base == &void_ctype) ? 1 : bits_to_bytes(base->bit_size);

 	if (ctype == &null_ctype)
 		ctype = &ptr_ctype;
@@ -2049,8 +2049,14 @@ static struct symbol *evaluate_sizeof(st
 		return NULL;

 	size = type->bit_size;
+
+	if (type->ctype.base_type == &void_ctype) {
+		warning(expr->pos, "expression using sizeof(void)");
+		size = bits_in_char;
+	}
 	if ((size < 0) || (size & (bits_in_char - 1)))
 		expression_error(expr, "cannot size expression");
+
 	expr->type = EXPR_VALUE;
 	expr->value = bits_to_bytes(size);
 	expr->taint = 0;
Index: sparse.chrisl/symbol.c
===================================================================
--- sparse.chrisl.orig/symbol.c
+++ sparse.chrisl/symbol.c
@@ -834,7 +834,7 @@ static const struct ctype_declare {
 	struct symbol *base_type;
 } ctype_declaration[] = {
 	{ &bool_ctype,	    SYM_BASETYPE, MOD_UNSIGNED,		    &bits_in_bool,	
   &max_int_alignment, &int_type },
-	{ &void_ctype,	    SYM_BASETYPE, 0,			    &bits_in_char,	     NULL,		 NULL },
+	{ &void_ctype,	    SYM_BASETYPE, 0,			    NULL,	     NULL,		 NULL },
 	{ &type_ctype,	    SYM_BASETYPE, MOD_TYPE,		    NULL,		     NULL,		 NULL },
 	{ &incomplete_ctype,SYM_BASETYPE, 0,			    NULL,		     NULL,		 NULL },
 	{ &bad_ctype,	    SYM_BASETYPE, 0,			    NULL,		     NULL,		 NULL },

Re: [PATCH] Warn about explicit usage of sizeof(void)

[Tommy Thorn]

Christopher Li wrote:
> So here is what I got. A patch address both of our need. It gives warning
> of using sizeof(void) explicitly. void* + offset will continue to work without
> warnings. It will also make is_byte_type() continue to work as it was
> before.
>
> Here is my test script:
>
> void *p;
>
> int i = sizeof(void);
> int j = sizeof(*p);
>   

I can't test it right now, but does it give a warning for both sizeof's 
above? If just first results in a warning, then I think that quite 
reasonable.

Tommy

[PATCH] Warn about explicit usage of sizeof(void) and sizeof(function)

[Alexey Zaytsev]

Tommy Thorn wrote:
> Christopher Li wrote:
>> So here is what I got. A patch address both of our need. It gives warning
>> of using sizeof(void) explicitly. void* + offset will continue to work without
>> warnings. It will also make is_byte_type() continue to work as it was
>> before.
>>
>> Here is my test script:
>>
>> void *p;
>>
>> int i = sizeof(void);
>> int j = sizeof(*p);
>>   

> I can't test it right now, but does it give a warning for both sizeof's 
> above? If just first results in a warning, then I think that quite 
> reasonable.

Both trigger the warning. I'm not sure this is a problem, as there are
no such usage cases in the kernel.

I added (hopefully the right way) handling of (sizeof(function)) to the
patch. function++ was already prohibited.

Running the test on the kernel right now.

--


From: Christopher Li <sparse@chrisli.org>

sizeof(void) and sizeof(function) still evaluate as 1
after the warning. void_ctype.bit_size remain zero so
is_byte_type() will continue to work.

Signed-Off-By: Christopher Li <sparse@chrisli.org>
[sizeof(function) added by Alexey Zaytsev]
Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
 evaluate.c |   13 ++++++++++++-
 symbol.c   |    2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index f976645..e82be53 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -579,7 +579,7 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
 	}
 
 	/* Get the size of whatever the pointer points to */
-	multiply = bits_to_bytes(base->bit_size);
+	multiply = (base == &void_ctype) ? 1 : bits_to_bytes(base->bit_size);
 
 	if (ctype == &null_ctype)
 		ctype = &ptr_ctype;
@@ -2044,8 +2044,19 @@ static struct symbol *evaluate_sizeof(struct expression *expr)
 		return NULL;
 
 	size = type->bit_size;
+
+	if (type->ctype.base_type == &void_ctype) {
+		warning(expr->pos, "expression using sizeof(void)");
+		size = bits_in_char;
+	}
+
+	if (is_function(type->ctype.base_type)) {
+		warning(expr->pos, "expression using sizeof on a function");
+		size = bits_in_char;
+	}
 	if ((size < 0) || (size & (bits_in_char - 1)))
 		expression_error(expr, "cannot size expression");
+
 	expr->type = EXPR_VALUE;
 	expr->value = bits_to_bytes(size);
 	expr->taint = 0;
diff --git a/symbol.c b/symbol.c
index 02844cf..4da253b 100644
--- a/symbol.c
+++ b/symbol.c
@@ -834,7 +834,7 @@ static const struct ctype_declare {
 	struct symbol *base_type;
 } ctype_declaration[] = {
 	{ &bool_ctype,	    SYM_BASETYPE, MOD_UNSIGNED,		    &bits_in_bool,	     &max_int_alignment, &int_type },
-	{ &void_ctype,	    SYM_BASETYPE, 0,			    &bits_in_char,	     NULL,		 NULL },
+	{ &void_ctype,	    SYM_BASETYPE, 0,			    NULL,		     NULL,		 NULL },
 	{ &type_ctype,	    SYM_BASETYPE, MOD_TYPE,		    NULL,		     NULL,		 NULL },
 	{ &incomplete_ctype,SYM_BASETYPE, 0,			    NULL,		     NULL,		 NULL },
 	{ &bad_ctype,	    SYM_BASETYPE, 0,			    NULL,		     NULL,		 NULL },

Re: [PATCH] Warn about explicit usage of sizeof(void) and sizeof(function)

[Christopher Li]

Hi Alexey,

On Thu, Dec 25, 2008 at 10:36 AM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> I added (hopefully the right way) handling of (sizeof(function)) to the
> patch. function++ was already prohibited.

Can you send me an incremental patch for the sizeof(function) change?
BTW, is it one of the gcc special treatment as well?

Thanks

Chris

On Thu, Dec 25, 2008 at 10:36 AM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> Tommy Thorn wrote:
>> Christopher Li wrote:
>>> So here is what I got. A patch address both of our need. It gives warning
>>> of using sizeof(void) explicitly. void* + offset will continue to work without
>>> warnings. It will also make is_byte_type() continue to work as it was
>>> before.
>>>
>>> Here is my test script:
>>>
> I added (hopefully the right way) handling of (sizeof(function)) to the
> patch. function++ was already prohibited.


>>> void *p;
>>>
>>> int i = sizeof(void);
>>> int j = sizeof(*p);
>>>
>
>> I can't test it right now, but does it give a warning for both sizeof's
>> above? If just first results in a warning, then I think that quite
>> reasonable.
>
> Both trigger the warning. I'm not sure this is a problem, as there are
> no such usage cases in the kernel.
>

>
> Running the test on the kernel right now.
>
> --
>
>
> From: Christopher Li <sparse@chrisli.org>
>
> sizeof(void) and sizeof(function) still evaluate as 1
> after the warning. void_ctype.bit_size remain zero so
> is_byte_type() will continue to work.
>
> Signed-Off-By: Christopher Li <sparse@chrisli.org>
> [sizeof(function) added by Alexey Zaytsev]
> Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> ---
>  evaluate.c |   13 ++++++++++++-
>  symbol.c   |    2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/evaluate.c b/evaluate.c
> index f976645..e82be53 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -579,7 +579,7 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
>        }
>
>        /* Get the size of whatever the pointer points to */
> -       multiply = bits_to_bytes(base->bit_size);
> +       multiply = (base == &void_ctype) ? 1 : bits_to_bytes(base->bit_size);
>
>        if (ctype == &null_ctype)
>                ctype = &ptr_ctype;
> @@ -2044,8 +2044,19 @@ static struct symbol *evaluate_sizeof(struct expression *expr)
>                return NULL;
>
>        size = type->bit_size;
> +
> +       if (type->ctype.base_type == &void_ctype) {
> +               warning(expr->pos, "expression using sizeof(void)");
> +               size = bits_in_char;
> +       }
> +
> +       if (is_function(type->ctype.base_type)) {
> +               warning(expr->pos, "expression using sizeof on a function");
> +               size = bits_in_char;
> +       }
>        if ((size < 0) || (size & (bits_in_char - 1)))
>                expression_error(expr, "cannot size expression");
> +
>        expr->type = EXPR_VALUE;
>        expr->value = bits_to_bytes(size);
>        expr->taint = 0;
> diff --git a/symbol.c b/symbol.c
> index 02844cf..4da253b 100644
> --- a/symbol.c
> +++ b/symbol.c
> @@ -834,7 +834,7 @@ static const struct ctype_declare {
>        struct symbol *base_type;
>  } ctype_declaration[] = {
>        { &bool_ctype,      SYM_BASETYPE, MOD_UNSIGNED,             &bits_in_bool,           &max_int_alignment, &int_type },
> -       { &void_ctype,      SYM_BASETYPE, 0,                        &bits_in_char,           NULL,               NULL },
> +       { &void_ctype,      SYM_BASETYPE, 0,                        NULL,                    NULL,               NULL },
>        { &type_ctype,      SYM_BASETYPE, MOD_TYPE,                 NULL,                    NULL,               NULL },
>        { &incomplete_ctype,SYM_BASETYPE, 0,                        NULL,                    NULL,               NULL },
>        { &bad_ctype,       SYM_BASETYPE, 0,                        NULL,                    NULL,               NULL },
>
>

[PATCH] Also warn about sizeof(function)

[Alexey Zaytsev]

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---

> Hi Alexey,
> 
> On Thu, Dec 25, 2008 at 10:36 AM, Alexey Zaytsev
> <alexey.zaytsev@gmail.com> wrote:
>> I added (hopefully the right way) handling of (sizeof(function)) to the
>> patch. function++ was already prohibited.
> 
> Can you send me an incremental patch for the sizeof(function) change?
Sure. Btw, your patches come demaged lately. Could you please use
git-send-email to send them in the future?

> BTW, is it one of the gcc special treatment as well?

It was just mentioned in the same option's description:
http://www.redhat.com/docs/manuals/enterprise/RHEL-4-Manual/gcc/pointer-arith.html

 evaluate.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index e23c4eb..1424ce1 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2049,6 +2049,12 @@ static struct symbol *evaluate_sizeof(struct expression *expr)
 		warning(expr->pos, "expression using sizeof(void)");
 		size = bits_in_char;
 	}
+
+	if (is_function(type->ctype.base_type)) {
+		warning(expr->pos, "expression using sizeof on a function");
+		size = bits_in_char;
+	}
+
 	if ((size < 0) || (size & (bits_in_char - 1)))
 		expression_error(expr, "cannot size expression");
 

Re: [PATCH] Also warn about sizeof(function)

[Christopher Li]

Thanks, apply and pushed.

On Thu, Dec 25, 2008 at 12:10 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> ---
>
>> Hi Alexey,
>>
> Sure. Btw, your patches come demaged lately. Could you please use
> git-send-email to send them in the future?

Sorry about that. I will use attachment to send them next time. I am
still learning my way to use git properly.

Chris

[PATCH] Null ctype should have ptr_ctype as its base type.

[Alexey Zaytsev]

Amirite?

Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---

On Thu, Dec 25, 2008 at 21:36, Alexey Zaytsev <alexey.zaytsev@gmail.com> wrote:
> Tommy Thorn wrote:
>> Christopher Li wrote:
>>> So here is what I got. A patch address both of our need. It gives warning
>>> of using sizeof(void) explicitly. void* + offset will continue to work without
>>> warnings. It will also make is_byte_type() continue to work as it was
>>> before.
>>>
>>> Here is my test script:
>>>
>>> void *p;
>>>
>>> int i = sizeof(void);
>>> int j = sizeof(*p);
>>>
>
>> I can't test it right now, but does it give a warning for both sizeof's
>> above? If just first results in a warning, then I think that quite
>> reasonable.
>
> Both trigger the warning. I'm not sure this is a problem, as there are
> no such usage cases in the kernel.
>
> I added (hopefully the right way) handling of (sizeof(function)) to the
> patch. function++ was already prohibited.
>
> Running the test on the kernel right now.
>

And it found something.

2158a2159
> fs/compat_ioctl.c:787:10: warning: expression using sizeof on a function
9524a9526,9543
> drivers/media/video/gspca/sonixb.c:494:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:494:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:494:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:495:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:495:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:495:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:496:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:498:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:498:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:498:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:500:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:500:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:502:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:502:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:502:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:504:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:504:1: warning: expression using sizeof(void)
> drivers/media/video/gspca/sonixb.c:504:1: warning: expression using sizeof(void)
15293a15313,15325
> net/netfilter/nf_conntrack_sip.c:282:21: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:282:21: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:287:23: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:287:23: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:288:29: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:586:23: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:586:23: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:587:25: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:588:29: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:589:25: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:590:29: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:591:22: warning: expression using sizeof(void)
> net/netfilter/nf_conntrack_sip.c:591:22: warning: expression using sizeof(void)

The first one is

        if (copy_in_user(&sgio->status, sgio32->status,
                         (4 * sizeof(unsigned char)) +
                         (2 * sizeof(unsigned (short))) +       <------- oops
                         (3 * sizeof(int))))
                return -EFAULT;

a bug. So, if anyone ever missed his sgio.info, now we know why.
Patch sent.

drivers/media/video/gspca/sonixb.c looks like

SENS(initHv7131, NULL, hv7131_sensor_init, NULL, NULL, 0, NO_EXPO|NO_FREQ, 0),

#define SENS(bridge_1, bridge_3, sensor, sensor_1, \
        sensor_3, _flags, _ctrl_dis, _sensor_addr) \
{ \
        .bridge_init = { bridge_1, bridge_3 }, \
        .bridge_init_size = { sizeof(bridge_1), sizeof(bridge_3) }, \

So, we are getting a sizeof(NULL), or a sizeof((void *)0). It triggers here
because we are getting a null_ctype, and it's base_type points to
void_ctype. I'm not sure if this patch is correct, but it seems to do
the trick, all void warnings are gone. Christopher?

 symbol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/symbol.c b/symbol.c
index 4da253b..df760d4 100644
--- a/symbol.c
+++ b/symbol.c
@@ -861,7 +861,7 @@ static const struct ctype_declare {
 
 	{ &string_ctype,    SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &char_ctype },
 	{ &ptr_ctype,	    SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &void_ctype },
-	{ &null_ctype,	    SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &void_ctype },
+	{ &null_ctype,	    SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &ptr_ctype  },
 	{ &label_ctype,	    SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &void_ctype },
 	{ &lazy_ptr_ctype,  SYM_PTR,	  0,			    &bits_in_pointer,        &pointer_alignment, &void_ctype },
 	{ NULL, }

Re: [PATCH] Null ctype should have ptr_ctype as its base type.

[Christopher Li]

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

On Sun, Dec 28, 2008 at 7:14 AM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:

> So, we are getting a sizeof(NULL), or a sizeof((void *)0). It triggers here

That is my fault. My test for void type should have been more careful.

> because we are getting a null_ctype, and it's base_type points to
> void_ctype. I'm not sure if this patch is correct, but it seems to do
> the trick, all void warnings are gone. Christopher?

No, that is not the right way to fix it. Now you declare NULL as pointer
to a pointer type "(void**) 0".

Thanks for finding it out. Does my patch work for you?

Chris

[-- Attachment #2: 0001-Correct-testing-for-void-type.patch --]
[-- Type: application/octet-stream, Size: 1549 bytes --]

From a7b5de90ea3ea8485882ba348b045279709bd6d9 Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@chrisli.org>
Date: Sun, 28 Dec 2008 12:31:50 -0800
Subject: [PATCH] Correct testing for void type.

The last sizeof(void) warning commit too eager to
declare type is void. It end up sizeof(NULL) == 1
as well.

Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 evaluate.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index d99d34f..2fff46e 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -337,6 +337,13 @@ static inline int is_byte_type(struct symbol *type)
 	return type->bit_size == bits_in_char && type->type != SYM_BITFIELD;
 }
 
+static inline int is_void_type(struct symbol *type)
+{
+	if (type->type == SYM_NODE)
+		type = type->ctype.base_type;
+	return type == &void_ctype;
+}
+
 enum {
 	TYPE_NUM = 1,
 	TYPE_BITFIELD = 2,
@@ -584,7 +591,7 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
 	}
 
 	/* Get the size of whatever the pointer points to */
-	multiply = (base == &void_ctype) ? 1 : bits_to_bytes(base->bit_size);
+	multiply = is_void_type(base) ? 1 : bits_to_bytes(base->bit_size);
 
 	if (ctype == &null_ctype)
 		ctype = &ptr_ctype;
@@ -2050,7 +2057,7 @@ static struct symbol *evaluate_sizeof(struct expression *expr)
 
 	size = type->bit_size;
 
-	if (type->ctype.base_type == &void_ctype) {
+	if (size < 0 && is_void_type(type)) {
 		warning(expr->pos, "expression using sizeof(void)");
 		size = bits_in_char;
 	}
-- 
1.6.0.6

Re: [PATCH] Null ctype should have ptr_ctype as its base type.

[Alexey Zaytsev]

On Sun, Dec 28, 2008 at 23:52, Christopher Li <sparse@chrisli.org> wrote:
> On Sun, Dec 28, 2008 at 7:14 AM, Alexey Zaytsev
> <alexey.zaytsev@gmail.com> wrote:
>
>> So, we are getting a sizeof(NULL), or a sizeof((void *)0). It triggers here
>
> That is my fault. My test for void type should have been more careful.
>
>> because we are getting a null_ctype, and it's base_type points to
>> void_ctype. I'm not sure if this patch is correct, but it seems to do
>> the trick, all void warnings are gone. Christopher?
>
> No, that is not the right way to fix it. Now you declare NULL as pointer
> to a pointer type "(void**) 0".
>
> Thanks for finding it out. Does my patch work for you?

Yes, it works, thank you.
There is one problem left:

11091a11093,11130
> drivers/net/wireless/wavelan_cs.c:362:16: error: subtraction of different types can't work (different base types)
> drivers/net/wireless/wavelan_cs.c:379:17: error: subtraction of different types can't work (different base types)
> drivers/net/wireless/wavelan_cs.c:385:21: error: subtraction of different types can't work (different base types)
[...]

It looks like:

... mmroff(0, mmr_fee_status) ..

#define mmroff(p,f)     (unsigned short)((void *)(&((mmr_t *)((void
*)0 + (p)))->f) - (void *)0)

mmr_fee_status being an element in struct mmr.
Here we end up substracting null from non-null void * pointer.
Looks quite pointless, but I think sparse should be able to
cope with this?

Re: [PATCH] Null ctype should have ptr_ctype as its base type.

[Christopher Li]

On Sun, Dec 28, 2008 at 1:38 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> 11091a11093,11130
>> drivers/net/wireless/wavelan_cs.c:362:16: error: subtraction of different types can't work (different base types)
>> drivers/net/wireless/wavelan_cs.c:379:17: error: subtraction of different types can't work (different base types)
>> drivers/net/wireless/wavelan_cs.c:385:21: error: subtraction of different types can't work (different base types)
> [...]
>
> It looks like:
>
> ... mmroff(0, mmr_fee_status) ..
>
> #define mmroff(p,f)     (unsigned short)((void *)(&((mmr_t *)((void
> *)0 + (p)))->f) - (void *)0)

That is one piece of ugly code.

>
> mmr_fee_status being an element in struct mmr.
> Here we end up substracting null from non-null void * pointer.
> Looks quite pointless, but I think sparse should be able to
> cope with this?

Do you have minimal code to duplicate this bug?

I try with the following testing code without much luck.

Chris

typedef struct mmr_t	mmr_t;

struct mmr_t {
	int a;
	int mmr_fee_status;
};

#define mmroff(p,f)     (unsigned short)((void *)(&((mmr_t *)((void
*)0 + (p)))->f) - (void *)0)
int b;

int foo(void)
{
	return mmroff(0,mmr_fee_status);
}

Re: [PATCH] Null ctype should have ptr_ctype as its base type.

[Alexey Zaytsev]

On Mon, Dec 29, 2008 at 10:32, Christopher Li <sparse@chrisli.org> wrote:
> On Sun, Dec 28, 2008 at 1:38 PM, Alexey Zaytsev
> <alexey.zaytsev@gmail.com> wrote:
>> 11091a11093,11130
>>> drivers/net/wireless/wavelan_cs.c:362:16: error: subtraction of different types can't work (different base types)
>>> drivers/net/wireless/wavelan_cs.c:379:17: error: subtraction of different types can't work (different base types)
>>> drivers/net/wireless/wavelan_cs.c:385:21: error: subtraction of different types can't work (different base types)
>> [...]
>>
>> It looks like:
>>
>> ... mmroff(0, mmr_fee_status) ..
>>
>> #define mmroff(p,f)     (unsigned short)((void *)(&((mmr_t *)((void
>> *)0 + (p)))->f) - (void *)0)
>
> That is one piece of ugly code.
>
>>
>> mmr_fee_status being an element in struct mmr.
>> Here we end up substracting null from non-null void * pointer.
>> Looks quite pointless, but I think sparse should be able to
>> cope with this?
>
> Do you have minimal code to duplicate this bug?

Sorry, my bad. Ran it with a wrong sparse version.

Re: [PATCH] Warn about explicit usage of sizeof(void)

[Christopher Li]

On Thu, Dec 25, 2008 at 9:23 AM, Tommy Thorn <tommy@numba-tu.com> wrote:
> Christopher Li wrote:
>>
>> So here is what I got. A patch address both of our need. It gives warning
>> of using sizeof(void) explicitly. void* + offset will continue to work
>> without
>> warnings. It will also make is_byte_type() continue to work as it was
>> before.
>>
>> Here is my test script:
>>
>> void *p;
>>
>> int i = sizeof(void);
>> int j = sizeof(*p);
>>
>
> I can't test it right now, but does it give a warning for both sizeof's
> above? If just first results in a warning, then I think that quite
> reasonable.

It warn for both. They are the same thing. It will be strange warn one
not the other.

But my test run shows that none of the kernel code actually use
sizeof(*p). If they do, I think it desert a warning.

Thanks

Chris