[PATCH] security_sk_free void return fixup

[PATCH] security_sk_free void return fixup

[Chris Wright]

  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 <mika@osdl.org>
Signed-off-by: Chris Wright <chrisw@osdl.org>

===== 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,

Re: [PATCH] security_sk_free void return fixup

[Linus Torvalds]

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

Re: [PATCH] security_sk_free void return fixup

[Andrew Morton]

Linus Torvalds <torvalds@osdl.org> 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.

Re: [PATCH] security_sk_free void return fixup

[Linus Torvalds]

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

Re: [PATCH] security_sk_free void return fixup

[Dick Streefland]

Linus Torvalds <torvalds@osdl.org> 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
dick.streefland@altium.nl           (@ @)          http://www.altium.com
--------------------------------oOO--(_)--OOo---------------------------