[PATCH net-next] net: xfrm6: silence sparse warning

[PATCH net-next] net: xfrm6: silence sparse warning

[Ying Xue]

Fix below sparse warning:

net/ipv6/xfrm6_state.c:66:26: error: cannot size expression

Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 net/ipv6/xfrm6_state.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index 3fc9701..0c10dff 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -63,7 +63,7 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
 	int class[XFRM_MAX_DEPTH];
 	int count[maxclass];
 
-	memset(count, 0, sizeof(count));
+	memset(count, 0, sizeof(int) * maxclass);
 
 	for (i = 0; i < n; i++) {
 		int c;
-- 
1.7.9.5

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[David Miller]

From: Ying Xue <ying.xue@windriver.com>
Date: Wed, 8 Jan 2014 10:56:40 +0800

> Fix below sparse warning:
> 
> net/ipv6/xfrm6_state.c:66:26: error: cannot size expression
 ...
> -	memset(count, 0, sizeof(count));
> +	memset(count, 0, sizeof(int) * maxclass);

I think the fix belongs in sparse, making it respect these two expressions
as equivalent.

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[Ying Xue]

On 01/08/2014 11:43 AM, David Miller wrote:
> From: Ying Xue <ying.xue@windriver.com>
> Date: Wed, 8 Jan 2014 10:56:40 +0800
> 
>> Fix below sparse warning:
>>
>> net/ipv6/xfrm6_state.c:66:26: error: cannot size expression
>  ...
>> -	memset(count, 0, sizeof(count));
>> +	memset(count, 0, sizeof(int) * maxclass);
> 
> I think the fix belongs in sparse, making it respect these two expressions
> as equivalent.
> 
> 

Yes, you are right. Actually the fix just eliminates one 'error' warning
complained from sparse, instead its complete sparse warnings like:

  CHECK   net/ipv6/xfrm6_state.c
net/ipv6/xfrm6_state.c:64:19: warning: Variable length array is used.
net/ipv6/xfrm6_state.c:66:26: error: cannot size expression

But the first warning(ie, warning: Variable length array is used) to be
fixed needs a big change made, so I think it seems unnecessary.
Therefore, I just fix the latter in the patch.

Which action below should I take in next step?

1. I just update the fix commit description as below:

-----
Fix below sparse error:

  CHECK   net/ipv6/xfrm6_state.c
net/ipv6/xfrm6_state.c:66:26: error: cannot size expression
-----

2. Fix the former warning as well in the patch by changing the
allocation of count[] array from static mode to dynamical.

3. Just drop the patch

Please tell me.

Thanks,
Ying

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[David Miller]

From: Ying Xue <ying.xue@windriver.com>
Date: Wed, 8 Jan 2014 13:55:09 +0800

> 3. Just drop the patch

This is the only suitable thing to do.

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[Ben Hutchings]

On Tue, 2014-01-07 at 22:43 -0500, David Miller wrote:
> From: Ying Xue <ying.xue@windriver.com>
> Date: Wed, 8 Jan 2014 10:56:40 +0800
> 
> > Fix below sparse warning:
> > 
> > net/ipv6/xfrm6_state.c:66:26: error: cannot size expression
>  ...
> > -	memset(count, 0, sizeof(count));
> > +	memset(count, 0, sizeof(int) * maxclass);
> 
> I think the fix belongs in sparse, making it respect these two expressions
> as equivalent.

It looks like sparse just doesn't support sizeof(VLA), although this is
valid C.

(It makes the same complaint about some places in sfc where we've used
min_t() in array dimensions, as that makes them VLAs even though the
size can actually be determined at compile time.)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[Stephen Hemminger]

On Wed, 08 Jan 2014 00:56:56 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Ying Xue <ying.xue@windriver.com>
> Date: Wed, 8 Jan 2014 13:55:09 +0800
> 
> > 3. Just drop the patch
> 
> This is the only suitable thing to do.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

There was some patches floating around to eliminate uses of variable
length array in the kernel since they aren't supported by CLANG.

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[Ben Hutchings]

On Thu, 2014-01-09 at 15:04 -0800, Stephen Hemminger wrote:
> On Wed, 08 Jan 2014 00:56:56 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Ying Xue <ying.xue@windriver.com>
> > Date: Wed, 8 Jan 2014 13:55:09 +0800
> > 
> > > 3. Just drop the patch
> > 
> > This is the only suitable thing to do.
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> There was some patches floating around to eliminate uses of variable
> length array in the kernel since they aren't supported by CLANG.

I think you're mixing up two different things there.  VLAs are part of
C99 so I'm quite sure Clang supports them (in general).  But they seem
to be deprecated in the kernel anyway, maybe because they make it easy
to introduce a stack overflow.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH net-next] net: xfrm6: silence sparse warning

[Stephen Hemminger]

On Fri, 10 Jan 2014 02:53:06 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Thu, 2014-01-09 at 15:04 -0800, Stephen Hemminger wrote:
> > On Wed, 08 Jan 2014 00:56:56 -0500 (EST)
> > David Miller <davem@davemloft.net> wrote:
> > 
> > > From: Ying Xue <ying.xue@windriver.com>
> > > Date: Wed, 8 Jan 2014 13:55:09 +0800
> > > 
> > > > 3. Just drop the patch
> > > 
> > > This is the only suitable thing to do.
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > There was some patches floating around to eliminate uses of variable
> > length array in the kernel since they aren't supported by CLANG.
> 
> I think you're mixing up two different things there.  VLAs are part of
> C99 so I'm quite sure Clang supports them (in general).  But they seem
> to be deprecated in the kernel anyway, maybe because they make it easy
> to introduce a stack overflow.
> 
> Ben.
> 

Reading up on this, the problem in CLANG is VLA in Structs;
as in:
struct foo {
  char a[n];
  int b;
};