2019-08-02 10:34:44

by Mao Wenan

[permalink] [raw]
Subject: [PATCH net-next] net: can: Fix compiling warning

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 <[email protected]>
---
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


2019-08-02 12:56:38

by Oliver Hartkopp

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

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 <[email protected]>

Acked-by: Oliver Hartkopp <[email protected]>

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 */
>

2019-08-06 15:07:24

by Dan Carpenter

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

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 <[email protected]>
>
> Acked-by: Oliver Hartkopp <[email protected]>
>
> 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

2019-08-06 17:19:36

by Oliver Hartkopp

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

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

2019-08-07 11:02:11

by Dan Carpenter

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

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

2019-08-12 05:49:04

by Mao Wenan

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



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

2019-08-12 08:12:40

by Oliver Hartkopp

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

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

2019-08-12 17:20:46

by Kees Cook

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

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

2019-08-13 12:52:03

by Dan Carpenter

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

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