[PATCH net-next] net: can: Fix compiling warning

[PATCH net-next] net: can: Fix compiling warning

[Mao Wenan]

There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd
and raw_sock_no_ioctlcmd as static.

net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?

Fixes: 473d924d7d46 ("can: fix ioctl function removal")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/can/bcm.c | 2 +-
 net/can/raw.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..b8a32b4ac368 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a01848ff9b12 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,7 +837,7 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
-- 
2.20.1

Re: [PATCH net-next] net: can: Fix compiling warning

[Oliver Hartkopp]

On 02/08/2019 05.36, Mao Wenan wrote:
> There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd
> and raw_sock_no_ioctlcmd as static.
> 
> net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
> net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
> 
> Fixes: 473d924d7d46 ("can: fix ioctl function removal")
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Thanks Mao!

Btw. what kind of compiler/make switches are you using so that I can see 
these warnings myself the next time?

Best regards,
Oliver

> ---
>   net/can/bcm.c | 2 +-
>   net/can/raw.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index bf1d0bbecec8..b8a32b4ac368 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
>   	return size;
>   }
>   
> -int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
> +static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>   			 unsigned long arg)
>   {
>   	/* no ioctls for socket layer -> hand it down to NIC layer */
> diff --git a/net/can/raw.c b/net/can/raw.c
> index da386f1fa815..a01848ff9b12 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -837,7 +837,7 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
>   	return size;
>   }
>   
> -int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
> +static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>   			 unsigned long arg)
>   {
>   	/* no ioctls for socket layer -> hand it down to NIC layer */
> 

Re: [PATCH net-next] net: can: Fix compiling warning

[maowenan]

On 2019/8/2 16:10, Oliver Hartkopp wrote:
> On 02/08/2019 05.36, Mao Wenan wrote:
>> There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd
>> and raw_sock_no_ioctlcmd as static.
>>
>> net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
>> net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
>>
>> Fixes: 473d924d7d46 ("can: fix ioctl function removal")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> Thanks Mao!
> 
> Btw. what kind of compiler/make switches are you using so that I can see these warnings myself the next time?
> 

I use ARCH=mips CROSS_COMPILE=mips-linux-gnu- .

> Best regards,
> Oliver
> 
>> ---
>>   net/can/bcm.c | 2 +-
>>   net/can/raw.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>> index bf1d0bbecec8..b8a32b4ac368 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
>> @@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
>>       return size;
>>   }
>>   -int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>> +static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>>                unsigned long arg)
>>   {
>>       /* no ioctls for socket layer -> hand it down to NIC layer */
>> diff --git a/net/can/raw.c b/net/can/raw.c
>> index da386f1fa815..a01848ff9b12 100644
>> --- a/net/can/raw.c
>> +++ b/net/can/raw.c
>> @@ -837,7 +837,7 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
>>       return size;
>>   }
>>   -int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>> +static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>>                unsigned long arg)
>>   {
>>       /* no ioctls for socket layer -> hand it down to NIC layer */
>>
> 
> .
> 

Re: [PATCH net-next] net: can: Fix compiling warning

[Sergei Shtylyov]

Hello!

On 02.08.2019 6:36, Mao Wenan wrote:

> There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd

    Warnings. :-)

> and raw_sock_no_ioctlcmd as static.
> 
> net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
> net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
> 
> Fixes: 473d924d7d46 ("can: fix ioctl function removal")
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
[...]

MBR, Sergei

Re: [PATCH net-next] net: can: Fix compiling warning

[maowenan]

On 2019/8/2 16:59, Sergei Shtylyov wrote:
> Hello!
> 
> On 02.08.2019 6:36, Mao Wenan wrote:
> 
>> There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd
> 
>    Warnings. :-)

Thanks, I will send v2.

> 
>> and raw_sock_no_ioctlcmd as static.
>>
>> net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
>> net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
>>
>> Fixes: 473d924d7d46 ("can: fix ioctl function removal")
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> [...]
> 
> MBR, Sergei
> 
> 

[PATCH net-next v2] net: can: Fix compiling warning

[Mao Wenan]

There are two warnings in net/can, fix them by setting bcm_sock_no_ioctlcmd
and raw_sock_no_ioctlcmd as static.

net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?

Fixes: 473d924d7d46 ("can: fix ioctl function removal")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 v1->v2: change patch description typo error, 'warings' to 'warnings'.
 net/can/bcm.c | 2 +-
 net/can/raw.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..b8a32b4ac368 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a01848ff9b12 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,7 +837,7 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
-- 
2.20.1

Re: [v2] net: can: Fix compiling warning

[Markus Elfring]

>  v1->v2: change patch description typo error, …

Will an other commit subject like “[PATCH v3] net: can: Fix compilation warnings
for two functions” be more appropriate here?

Regards,
Markus

[PATCH net-next v3] net: can: Fix compiling warnings for two functions

[Mao Wenan]

There are two warnings in net/can, fix them by setting bcm_sock_no_ioctlcmd
and raw_sock_no_ioctlcmd as static.

net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?

Fixes: 473d924d7d46 ("can: fix ioctl function removal")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 v1->v2: change patch description typo error, 'warings' to 'warnings'.
 v2->v3: change subject of patch.
 net/can/bcm.c | 2 +-
 net/can/raw.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..b8a32b4ac368 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a01848ff9b12 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,7 +837,7 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
 			 unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
-- 
2.20.1

Re: [PATCH net-next v3] net: can: Fix compiling warnings for two functions

[David Miller]

From: Mao Wenan <maowenan@huawei.com>
Date: Mon, 5 Aug 2019 19:57:44 +0800

> @@ -1680,7 +1680,7 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
>  	return size;
>  }
>  
> -int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
> +static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
>  			 unsigned long arg)

The alignment of the second line here needs to be adjusted, it must start
precisely at the first column after the openning parenthesis of the first
line.

Same for the other change in this patch.

[PATCH net-next v4] net: can: Fix compiling warnings for two functions

[Mao Wenan]

There are two warnings in net/can, fix them by setting
bcm_sock_no_ioctlcmd and raw_sock_no_ioctlcmd as static.

net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?

Fixes: 473d924d7d46 ("can: fix ioctl function removal")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 v1->v2: change patch description typo error, 'warings' to 'warnings'.
 v2->v3: change subject of patch.
 v3->v4: change the alignment of two functions. 
 net/can/bcm.c | 4 ++--
 net/can/raw.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..eb1d28b8c46a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,8 +1680,8 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
-			 unsigned long arg)
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+				unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
 	return -ENOIOCTLCMD;
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a30aaecd9327 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,8 +837,8 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
-			 unsigned long arg)
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+				unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
 	return -ENOIOCTLCMD;
-- 
2.20.1

Re: [v4] net: can: Fix compiling warnings for two functions

[Markus Elfring]

>  v2->v3: change subject of patch.

Will it be nicer to use the word “compilation” in your patch subject?


>  v3->v4: change the alignment of two functions.

I would refer to an adjusted indentation.

How do you think about to omit “arrows” in these version identifications?

Regards,
Markus

Re: [PATCH net-next] net: can: Fix compiling warning

[Dan Carpenter]

On Fri, Aug 02, 2019 at 10:10:20AM +0200, Oliver Hartkopp wrote:
> On 02/08/2019 05.36, Mao Wenan wrote:
> > There are two warings in net/can, fix them by setting bcm_sock_no_ioctlcmd
> > and raw_sock_no_ioctlcmd as static.
> > 
> > net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
> > net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
> > 
> > Fixes: 473d924d7d46 ("can: fix ioctl function removal")
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> 
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> Thanks Mao!
> 
> Btw. what kind of compiler/make switches are you using so that I can see
> these warnings myself the next time?

These are Sparse warnings, not from GCC.

regards,
dan carpenter

[PATCH net-next v5] net: can: Fix sparse warnings for two functions

[Mao Wenan]

There are two warnings in net/can, fix them by setting
bcm_sock_no_ioctlcmd and raw_sock_no_ioctlcmd as static.

net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?

Fixes: 473d924d7d46 ("can: fix ioctl function removal")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 v2: change patch description typo error, 'warings' to 'warnings'.
 v3: change subject of patch.
 v4: change the alignment of two functions. 
 v5: change subject of patch.
 net/can/bcm.c | 4 ++--
 net/can/raw.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..eb1d28b8c46a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,8 +1680,8 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
-			 unsigned long arg)
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+				unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
 	return -ENOIOCTLCMD;
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a30aaecd9327 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,8 +837,8 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 	return size;
 }
 
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
-			 unsigned long arg)
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+				unsigned long arg)
 {
 	/* no ioctls for socket layer -> hand it down to NIC layer */
 	return -ENOIOCTLCMD;
-- 
2.20.1

Re: [PATCH net-next] net: can: Fix compiling warning

[Oliver Hartkopp]

Hello Dan,

On 06/08/2019 15.52, Dan Carpenter wrote:
> On Fri, Aug 02, 2019 at 10:10:20AM +0200, Oliver Hartkopp wrote:

>> Btw. what kind of compiler/make switches are you using so that I can see
>> these warnings myself the next time?
> 
> These are Sparse warnings, not from GCC.

I compiled the code (the original version), but I do not get that 
"Should it be static?" warning:

user@box:~/net-next$ make C=1
   CALL    scripts/checksyscalls.sh
   CALL    scripts/atomic/check-atomics.sh
   DESCEND  objtool
   CHK     include/generated/compile.h
   CHECK   net/can/af_can.c
./include/linux/sched.h:609:43: error: bad integer constant expression
./include/linux/sched.h:609:73: error: invalid named zero-width bitfield 
`value'
./include/linux/sched.h:610:43: error: bad integer constant expression
./include/linux/sched.h:610:67: error: invalid named zero-width bitfield 
`bucket_id'
   CC [M]  net/can/af_can.o
   CHECK   net/can/proc.c
./include/linux/sched.h:609:43: error: bad integer constant expression
./include/linux/sched.h:609:73: error: invalid named zero-width bitfield 
`value'
./include/linux/sched.h:610:43: error: bad integer constant expression
./include/linux/sched.h:610:67: error: invalid named zero-width bitfield 
`bucket_id'
   CC [M]  net/can/proc.o
   LD [M]  net/can/can.o
   CHECK   net/can/raw.c
./include/linux/sched.h:609:43: error: bad integer constant expression
./include/linux/sched.h:609:73: error: invalid named zero-width bitfield 
`value'
./include/linux/sched.h:610:43: error: bad integer constant expression
./include/linux/sched.h:610:67: error: invalid named zero-width bitfield 
`bucket_id'
   CC [M]  net/can/raw.o
   LD [M]  net/can/can-raw.o
   CHECK   net/can/bcm.c
./include/linux/sched.h:609:43: error: bad integer constant expression
./include/linux/sched.h:609:73: error: invalid named zero-width bitfield 
`value'
./include/linux/sched.h:610:43: error: bad integer constant expression
./include/linux/sched.h:610:67: error: invalid named zero-width bitfield 
`bucket_id'
   CC [M]  net/can/bcm.o
   LD [M]  net/can/can-bcm.o
   CHECK   net/can/gw.c
./include/linux/sched.h:609:43: error: bad integer constant expression
./include/linux/sched.h:609:73: error: invalid named zero-width bitfield 
`value'
./include/linux/sched.h:610:43: error: bad integer constant expression
./include/linux/sched.h:610:67: error: invalid named zero-width bitfield 
`bucket_id'
   CC [M]  net/can/gw.o
   LD [M]  net/can/can-gw.o
Kernel: arch/x86/boot/bzImage is ready  (#2)

I've seen that warning at different other code - but not in bcm.c & raw.c

I downloaded & compiled the latest sparse source. But still no "static" 
warning. Any idea?

Best regards,
Oliver

Re: [PATCH net-next] net: can: Fix compiling warning

[Dan Carpenter]

On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote:
> I compiled the code (the original version), but I do not get that "Should it
> be static?" warning:
> 
> user@box:~/net-next$ make C=1
>   CALL    scripts/checksyscalls.sh
>   CALL    scripts/atomic/check-atomics.sh
>   DESCEND  objtool
>   CHK     include/generated/compile.h
>   CHECK   net/can/af_can.c
> ./include/linux/sched.h:609:43: error: bad integer constant expression
> ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield
> `value'
> ./include/linux/sched.h:610:43: error: bad integer constant expression
> ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield
> `bucket_id'
>   CC [M]  net/can/af_can.o

The sched.h errors suppress Sparse warnings so it's broken/useless now.
The code looks like this:

include/linux/sched.h
   613  struct uclamp_se {
   614          unsigned int value              : bits_per(SCHED_CAPACITY_SCALE);
   615          unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);
   616          unsigned int active             : 1;
   617          unsigned int user_defined       : 1;
   618  };

bits_per() is zero and Sparse doesn't like zero sized bitfields.

regards,
dan carpenter

Re: [PATCH net-next] net: can: Fix compiling warning

[maowenan]

On 2019/8/7 0:41, Oliver Hartkopp wrote:
> Hello Dan,
> 
> On 06/08/2019 15.52, Dan Carpenter wrote:
>> On Fri, Aug 02, 2019 at 10:10:20AM +0200, Oliver Hartkopp wrote:
> 
>>> Btw. what kind of compiler/make switches are you using so that I can see
>>> these warnings myself the next time?
>>
>> These are Sparse warnings, not from GCC.
> 
> I compiled the code (the original version), but I do not get that "Should it be static?" warning:

Hello Oliver,

here are my steps for net/can/bcm.c,
make allmodconfig ARCH=mips CROSS_COMPILE=mips-linux-gnu-
make C=2 net/can/bcm.o ARCH=mips CROSS_COMPILE=mips-linux-gnu-

  CHECK   scripts/mod/empty.c
  CALL    scripts/checksyscalls.sh
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
  CALL    scripts/atomic/check-atomics.sh
  CHECK   net/can/bcm.c
./include/linux/slab.h:672:13: error: undefined identifier '__builtin_mul_overflow'
./include/linux/slab.h:672:13: error: not a function <noident>
./include/linux/slab.h:672:13: error: not a function <noident>
net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
./include/linux/slab.h:672:13: warning: call with no type!
  CC [M]  net/can/bcm.o

for net/can/raw.c,
make allmodconfig ARCH=mips CROSS_COMPILE=mips-linux-gnu-
make C=2 net/can/raw.o ARCH=mips CROSS_COMPILE=mips-linux-gnu-

  CHECK   scripts/mod/empty.c
  CALL    scripts/checksyscalls.sh
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
  CALL    scripts/atomic/check-atomics.sh
  CHECK   net/can/raw.c
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
  CC [M]  net/can/raw.o

Re: [PATCH net-next] net: can: Fix compiling warning

[Oliver Hartkopp]

On 12/08/2019 07.48, maowenan wrote:
> On 2019/8/7 0:41, Oliver Hartkopp wrote:

>> I compiled the code (the original version), but I do not get that "Should it be static?" warning:

> here are my steps for net/can/bcm.c,
> make allmodconfig ARCH=mips CROSS_COMPILE=mips-linux-gnu-
> make C=2 net/can/bcm.o ARCH=mips CROSS_COMPILE=mips-linux-gnu-

There were some sparse _errors_ in my setup that hide that "static" 
warning. I will use sparse by default now.

Many thanks,
Oliver

Re: [PATCH net-next] net: can: Fix compiling warning

[Kees Cook]

On Wed, Aug 07, 2019 at 01:50:42PM +0300, Dan Carpenter wrote:
> On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote:
> > I compiled the code (the original version), but I do not get that "Should it
> > be static?" warning:
> > 
> > user@box:~/net-next$ make C=1
> >   CALL    scripts/checksyscalls.sh
> >   CALL    scripts/atomic/check-atomics.sh
> >   DESCEND  objtool
> >   CHK     include/generated/compile.h
> >   CHECK   net/can/af_can.c
> > ./include/linux/sched.h:609:43: error: bad integer constant expression
> > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield
> > `value'
> > ./include/linux/sched.h:610:43: error: bad integer constant expression
> > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield
> > `bucket_id'
> >   CC [M]  net/can/af_can.o
> 
> The sched.h errors suppress Sparse warnings so it's broken/useless now.
> The code looks like this:
> 
> include/linux/sched.h
>    613  struct uclamp_se {
>    614          unsigned int value              : bits_per(SCHED_CAPACITY_SCALE);
>    615          unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);
>    616          unsigned int active             : 1;
>    617          unsigned int user_defined       : 1;
>    618  };
> 
> bits_per() is zero and Sparse doesn't like zero sized bitfields.

I just noticed these sparse warnings too -- what's happening here? Are
they _supposed_ to be 0-width fields? It doesn't look like it to me:

CONFIG_UCLAMP_BUCKETS_COUNT=5
...
#define UCLAMP_BUCKETS CONFIG_UCLAMP_BUCKETS_COUNT
...
        unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);

I would expect this to be 3 bits wide. ... Looks like gcc agrees:

struct uclamp_se {
    unsigned int               value:11;             /*     0: 0  4 */
    unsigned int               bucket_id:3;          /*     0:11  4 */
...

So this is a sparse issue?

-- 
Kees Cook

Re: [PATCH net-next] net: can: Fix compiling warning

[Dan Carpenter]

On Mon, Aug 12, 2019 at 10:19:27AM -0700, Kees Cook wrote:
> On Wed, Aug 07, 2019 at 01:50:42PM +0300, Dan Carpenter wrote:
> > On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote:
> > > I compiled the code (the original version), but I do not get that "Should it
> > > be static?" warning:
> > > 
> > > user@box:~/net-next$ make C=1
> > >   CALL    scripts/checksyscalls.sh
> > >   CALL    scripts/atomic/check-atomics.sh
> > >   DESCEND  objtool
> > >   CHK     include/generated/compile.h
> > >   CHECK   net/can/af_can.c
> > > ./include/linux/sched.h:609:43: error: bad integer constant expression
> > > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield
> > > `value'
> > > ./include/linux/sched.h:610:43: error: bad integer constant expression
> > > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield
> > > `bucket_id'
> > >   CC [M]  net/can/af_can.o
> > 
> > The sched.h errors suppress Sparse warnings so it's broken/useless now.
> > The code looks like this:
> > 
> > include/linux/sched.h
> >    613  struct uclamp_se {
> >    614          unsigned int value              : bits_per(SCHED_CAPACITY_SCALE);
> >    615          unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);
> >    616          unsigned int active             : 1;
> >    617          unsigned int user_defined       : 1;
> >    618  };
> > 
> > bits_per() is zero and Sparse doesn't like zero sized bitfields.
> 
> I just noticed these sparse warnings too -- what's happening here? Are
> they _supposed_ to be 0-width fields? It doesn't look like it to me:

I'm sorr, I don't even know what code I was looking at before.  I think
my cscope database was stale?  You're right.  Sparse doesn't think it's
zero, it knows that it is 11 and 3.

What's happening is that it's failing the test in in
bad_integer_constant_expression():

	if (!(expr->flags & CEF_ICE))

The ICE in CEF_ICE stands for Integer Constant Expression.  The rule
here is that enums are not constant expressions in c99.  See the
explanation in commit 274c154704db ("constexpr: introduce additional
expression constness tracking flags").

I don't think the CEF_ICE is set properly in evaluate_conditional_expression().
If conditional is constant and it's true and the ->cond_true expression
is constant then the result should be constant as well.  It shouldn't
matter if the cond_false is constant.  But instead it is ANDing all
three sub expressions:

	expr->flags = (expr->conditional->flags & (*true)->flags &
			expr->cond_false->flags & ~CEF_CONST_MASK);

Or actually in this case it's doing:

	if (expr->conditional->flags & (CEF_ACE | CEF_ADDR))
		expr->flags = (*true)->flags & expr->cond_false->flags & ~CEF_CONST_MASK;

But it's the same problem because it's should ignore cond_false.

regards,
dan carpenter