2004-06-15 23:16:49

by Chris Wright

[permalink] [raw]
Subject: [PATCH] security_sk_free void return fixup

CHECK net/core/sock.c
include/linux/security.h:2636:39: warning: return expression in void function
CC net/core/sock.o

From: Mika Kukkonen <[email protected]>
Signed-off-by: Chris Wright <[email protected]>

===== include/linux/security.h 1.35 vs edited =====
--- 1.35/include/linux/security.h 2004-06-14 08:56:50 -07:00
+++ edited/include/linux/security.h 2004-06-15 16:14:56 -07:00
@@ -2633,7 +2633,7 @@

static inline void security_sk_free(struct sock *sk)
{
- return security_ops->sk_free_security(sk);
+ security_ops->sk_free_security(sk);
}
#else /* CONFIG_SECURITY_NETWORK */
static inline int security_unix_stream_connect(struct socket * sock,


2004-06-16 03:04:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] security_sk_free void return fixup



On Tue, 15 Jun 2004, Chris Wright wrote:
>
> CHECK net/core/sock.c
> include/linux/security.h:2636:39: warning: return expression in void function
> CC net/core/sock.o

I'm going to remove this warning from sparse. Apparently it is valid C99,
and somebody (I think Richard Henderson) made the excellent point that it
allows for type-independent code where you do something like

mytype myfunc1(xxx);

mytype myfunc2(xxx)
{
...
return myfunc1(...);
}

and it just works regardless of what type it is.

sparse will obviously warn about expressions with non-void types being
returned from a void function, but the case where the expression exists
and has the right type should be ok.

I'm not sure it's wonderful C in general, but I certainly can't claim it
is actively offensive, and since gcc accepts it and we have these things
in the kernel, why complain?

Linus

2004-06-16 03:11:02

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] security_sk_free void return fixup

Linus Torvalds <[email protected]> wrote:
>
>
>
> On Tue, 15 Jun 2004, Chris Wright wrote:
> >
> > CHECK net/core/sock.c
> > include/linux/security.h:2636:39: warning: return expression in void function
> > CC net/core/sock.o
>
> I'm going to remove this warning from sparse. Apparently it is valid C99,

yes, but in every(?) case where it triggered, the kernel code was wrong.
So I'd suggest the warning be retained, perhaps with an option to enable
it.

2004-06-16 03:11:03

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] security_sk_free void return fixup



On Tue, 15 Jun 2004, Andrew Morton wrote:
> >
> > I'm going to remove this warning from sparse. Apparently it is valid C99,
>
> yes, but in every(?) case where it triggered, the kernel code was wrong.

Is that true? Hmm. That would indeed be a strong argument. However, I was
also a bit alarmed by how one of the fixes for this warning was itself
actually buggy by dropping the "return" statement altogether and falling
through.

> So I'd suggest the warning be retained, perhaps with an option to enable
> it.

Hmm. Sparse already has a "verbose" level, although right now it's not
actually used by anything (it used to enable some warnings that were due
to sparse deficiencies, but I fixed that particular problem).

Linus

2004-06-16 12:12:56

by Kees Bakker

[permalink] [raw]
Subject: Re: [PATCH] security_sk_free void return fixup

Linus Torvalds <[email protected]> wrote:
| I'm going to remove this warning from sparse. Apparently it is valid C99,
| and somebody (I think Richard Henderson) made the excellent point that it
| allows for type-independent code where you do something like
|
| mytype myfunc1(xxx);
|
| mytype myfunc2(xxx)
| {
| ...
| return myfunc1(...);
| }
|
| and it just works regardless of what type it is.

It may work in gcc, but it is invalid according to ISO C99. First
sentence from section 6.8.6.4:

A return statement with an expression shall not appear in a
function whose return type is void.

Now, a function call is obviously an expression.

| sparse will obviously warn about expressions with non-void types being
| returned from a void function, but the case where the expression exists
| and has the right type should be ok.
|
| I'm not sure it's wonderful C in general, but I certainly can't claim it
| is actively offensive, and since gcc accepts it and we have these things
| in the kernel, why complain?

Gcc warns about this with the -pedantic option:

$ gcc-3.3 -pedantic -c return.c
return.c: In function `myfunc2':
return.c:5: warning: `return' with a value, in function returning void

--
Dick Streefland //// Altium BV
[email protected] (@ @) http://www.altium.com
--------------------------------oOO--(_)--OOo---------------------------