Home » Mailing list archives » linux-kernel » 2010 » May » 4

Re: [patch] sunrpc: add missing return statement

Previous thread: [GIT PULL] amended: first round of vhost-net enhancements for net-next by Michael S. Tsirkin on Tuesday, May 4, 2010 - 4:21 am. (2 messages)

Next thread: [PATCH 10/12] perf: make nr_events an int and add perf_online_mutex to protect it by Tejun Heo on Tuesday, May 4, 2010 - 5:38 am. (1 message)
[view original message]
From: Johannes Weiner
Subject: [patch] sunrpc: add missing return statement
Date: Tuesday, May 4, 2010 - 4:59 am

f300bab "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel"
introduced an error case branch that lacks an actual `return' keyword
before the return value.  Add it.

Signed-off-by: Johannes Weiner <jw@emlix.com>
Cc: Alexandros Batsakis <batsakis@netapp.com>
---
 net/sunrpc/xprtsock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2444,7 +2444,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(
 	struct svc_sock *bc_sock;
 
 	if (!args->bc_xprt)
-		ERR_PTR(-EINVAL);
+		return ERR_PTR(-EINVAL);
 
 	xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
 	if (IS_ERR(xprt))
--


[ message continues ]
[view original message]
From: Trond Myklebust
Subject: Re: [patch] sunrpc: add missing return statement
Date: Tuesday, May 4, 2010 - 5:27 am

No. It should either be a BUG_ON(), or else be removed entirely.
Returning an error value for something that is clearly a programming bug
is not a particularly useful exercise...

Cheers
  Trond


--


[ message continues ]
[view original message]
From: Tetsuo Handa
Subject: Re: [patch] sunrpc: add missing return statement
Date: Tuesday, May 4, 2010 - 6:03 am

Removing NULL check is wrong because it will NULL pointer dereference later.

--


[ message continues ]
[view original message]
From: Trond Myklebust
Subject: Re: [patch] sunrpc: add missing return statement
Date: Tuesday, May 4, 2010 - 6:13 am

Wrong. Removing NULL check is _right_ because calling this function
without setting up a back channel first is a major BUG. Returning an
error value to the user is pointless, since the user has no control over
this. It is entirely under control of the sunrpc developers...

Trond

--


[ message continues ]
[view original message]
From: Tetsuo Handa
Subject: Re: [patch] sunrpc: add missing return statement
Date: Tuesday, May 4, 2010 - 7:02 am

For security people, removing

	if (!args->bc_xprt)
		ERR_PTR(-EINVAL);

is worse and changing to

	BUG_ON(!args->bc_xprt);

is better.
--


[ message continues ]
Previous thread: [GIT PULL] amended: first round of vhost-net enhancements for net-next by Michael S. Tsirkin on Tuesday, May 4, 2010 - 4:21 am. (2 messages)

Next thread: [PATCH 10/12] perf: make nr_events an int and add perf_online_mutex to protect it by Tejun Heo on Tuesday, May 4, 2010 - 5:38 am. (1 message)