[PATCH RESEND net-next 0/4] tipc: do some clean up

[PATCH RESEND net-next 0/4] tipc: do some clean up

[Wang Weidong]

This patch series do some small clean up in tipc. And add reviewed-by
Erik and Jon.

Wang Weidong (4):
  tipc: remove unnecessary variables and conditions
  tipc: kill unnecessary goto's
  tipc: Use <linux/uaccess.h> instead of <asm/uaccess.h>
  tipc: change lock_sock order in connect()

 net/tipc/core.h       |  2 +-
 net/tipc/name_table.c |  3 +--
 net/tipc/port.c       |  9 +++------
 net/tipc/socket.c     | 33 ++++++++++++---------------------
 4 files changed, 17 insertions(+), 30 deletions(-)

-- 
1.7.12

[PATCH net-next 1/4] tipc: remove unnecessary variables and conditions

[Wang Weidong]

We remove a number of unnecessary variables and branches
in TIPC. This patch is cosmetic and does not change the
operation of TIPC in any way.

Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/name_table.c |  3 +--
 net/tipc/port.c       |  9 +++------
 net/tipc/socket.c     | 13 ++++---------
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 09dcd54..92a1533 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -148,8 +148,7 @@ static struct publication *publ_create(u32 type, u32 lower, u32 upper,
  */
 static struct sub_seq *tipc_subseq_alloc(u32 cnt)
 {
-	struct sub_seq *sseq = kcalloc(cnt, sizeof(struct sub_seq), GFP_ATOMIC);
-	return sseq;
+	return kcalloc(cnt, sizeof(struct sub_seq), GFP_ATOMIC);
 }
 
 /**
diff --git a/net/tipc/port.c b/net/tipc/port.c
index c081a76..5fd4c8c 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -832,17 +832,14 @@ exit:
  */
 int __tipc_disconnect(struct tipc_port *tp_ptr)
 {
-	int res;
-
 	if (tp_ptr->connected) {
 		tp_ptr->connected = 0;
 		/* let timer expire on it's own to avoid deadlock! */
 		tipc_nodesub_unsubscribe(&tp_ptr->subscription);
-		res = 0;
-	} else {
-		res = -ENOTCONN;
+		return 0;
 	}
-	return res;
+
+	return -ENOTCONN;
 }
 
 /*
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 3b61851..32037c5 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -239,7 +239,6 @@ static int tipc_sk_create(struct net *net, struct socket *sock, int protocol,
 int tipc_sock_create_local(int type, struct socket **res)
 {
 	int rc;
-	struct sock *sk;
 
 	rc = sock_create_lite(AF_TIPC, type, 0, res);
 	if (rc < 0) {
@@ -248,8 +247,6 @@ int tipc_sock_create_local(int type, struct socket **res)
 	}
 	tipc_sk_create(&init_net, *res, 0, 1);
 
-	sk = (*res)->sk;
-
 	return 0;
 }
 
@@ -1311,14 +1308,12 @@ static u32 filter_connect(struct tipc_sock *tsock, struct sk_buff **buf)
 static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *buf)
 {
 	struct tipc_msg *msg = buf_msg(buf);
-	unsigned int limit;
 
 	if (msg_connected(msg))
-		limit = sysctl_tipc_rmem[2];
-	else
-		limit = sk->sk_rcvbuf >> TIPC_CRITICAL_IMPORTANCE <<
-			msg_importance(msg);
-	return limit;
+		return sysctl_tipc_rmem[2];
+
+	return sk->sk_rcvbuf >> TIPC_CRITICAL_IMPORTANCE <<
+		msg_importance(msg);
 }
 
 /**
-- 
1.7.12

RE: [PATCH net-next 1/4] tipc: remove unnecessary variables and conditions

[David Laight]

> From: Wang Weidong
> Sent: 16 December 2013 12:40
> We remove a number of unnecessary variables and branches
> in TIPC. This patch is cosmetic and does not change the
> operation of TIPC in any way.
> 
...
>  int __tipc_disconnect(struct tipc_port *tp_ptr)
>  {
> -	int res;
> -
>  	if (tp_ptr->connected) {
>  		tp_ptr->connected = 0;
>  		/* let timer expire on it's own to avoid deadlock! */
>  		tipc_nodesub_unsubscribe(&tp_ptr->subscription);
> -		res = 0;
> -	} else {
> -		res = -ENOTCONN;
> +		return 0;
>  	}
> -	return res;
> +
> +	return -ENOTCONN;
>  }

IMHO better coded as:

	if (!tp_ptr->connected)
		return -ENOTCON;

	tp_ptr->connected = 0;
	/* let timer expire on it's own to avoid deadlock! */
	tipc_nodesub_unsubscribe(&tp_ptr->subscription);
	return 0;

	David

	

Re: [PATCH net-next 1/4] tipc: remove unnecessary variables and conditions

[Wang Weidong]

On 2013/12/16 21:33, David Laight wrote:
>> From: Wang Weidong
>> Sent: 16 December 2013 12:40
>> We remove a number of unnecessary variables and branches
>> in TIPC. This patch is cosmetic and does not change the
>> operation of TIPC in any way.
>>
> ...
>>   int __tipc_disconnect(struct tipc_port *tp_ptr)
>>   {
>> -	int res;
>> -
>>   	if (tp_ptr->connected) {
>>   		tp_ptr->connected = 0;
>>   		/* let timer expire on it's own to avoid deadlock! */
>>   		tipc_nodesub_unsubscribe(&tp_ptr->subscription);
>> -		res = 0;
>> -	} else {
>> -		res = -ENOTCONN;
>> +		return 0;
>>   	}
>> -	return res;
>> +
>> +	return -ENOTCONN;
>>   }
>
> IMHO better coded as:
>
> 	if (!tp_ptr->connected)
> 		return -ENOTCON;
>
> 	tp_ptr->connected = 0;
> 	/* let timer expire on it's own to avoid deadlock! */
> 	tipc_nodesub_unsubscribe(&tp_ptr->subscription);
> 	return 0;
>
> 	David
>
>
Sure, I will fix it in v2.

Thanks.
	
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH net-next 2/4] tipc: kill unnecessary goto's

[Wang Weidong]

Remove a number of needless 'goto exit' in send_stream
when the socket is in an unconnected state.
This patch is cosmetic and does not alter the operation of
TIPC in any way.

Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/socket.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 32037c5..844bf34 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -751,16 +751,14 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
 
 	/* Handle special cases where there is no connection */
 	if (unlikely(sock->state != SS_CONNECTED)) {
-		if (sock->state == SS_UNCONNECTED) {
+		res = -ENOTCONN;
+
+		if (sock->state == SS_UNCONNECTED)
 			res = send_packet(NULL, sock, m, total_len);
-			goto exit;
-		} else if (sock->state == SS_DISCONNECTING) {
+		else if (sock->state == SS_DISCONNECTING)
 			res = -EPIPE;
-			goto exit;
-		} else {
-			res = -ENOTCONN;
-			goto exit;
-		}
+
+		goto exit;
 	}
 
 	if (unlikely(m->msg_name)) {
-- 
1.7.12

RE: [PATCH net-next 2/4] tipc: kill unnecessary goto's

[David Laight]

> From: Wang Weidong
> Sent: 16 December 2013 12:40
> Remove a number of needless 'goto exit' in send_stream
> when the socket is in an unconnected state.
> This patch is cosmetic and does not alter the operation of
> TIPC in any way.
> 
> Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
> Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
>  net/tipc/socket.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 32037c5..844bf34 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -751,16 +751,14 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
> 
>  	/* Handle special cases where there is no connection */
>  	if (unlikely(sock->state != SS_CONNECTED)) {
> -		if (sock->state == SS_UNCONNECTED) {
> +		res = -ENOTCONN;
> +
> +		if (sock->state == SS_UNCONNECTED)
>  			res = send_packet(NULL, sock, m, total_len);
> -			goto exit;
> -		} else if () {
> +		else if (sock->state == SS_DISCONNECTING)
>  			res = -EPIPE;
> -			goto exit;
> -		} else {
> -			res = -ENOTCONN;
> -			goto exit;
> -		}
> +
> +		goto exit;

I'm not sure that 'removing needless gotos' is a good description of the change.
Possibly the code is easier to read with only one goto, ymmv.
You could remove the 'else' after the 'goto'.
Maybe the easiest to read is:
	if (sock->state == SS_UNCONNECTED)
		res = send_packet(NULL, sock, m, total_len);
	else
		res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
	goto exit;

	David

Re: [PATCH net-next 2/4] tipc: kill unnecessary goto's

[Wang Weidong]

From: Wang weidong <wangweidong1@huawei.com>

On 2013/12/16 21:30, David Laight wrote:
>> From: Wang Weidong
>> Sent: 16 December 2013 12:40
>> Remove a number of needless 'goto exit' in send_stream
>> when the socket is in an unconnected state.
>> This patch is cosmetic and does not alter the operation of
>> TIPC in any way.
>>
>> Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
>> Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/tipc/socket.c | 14 ++++++--------
>>   1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
>> index 32037c5..844bf34 100644
>> --- a/net/tipc/socket.c
>> +++ b/net/tipc/socket.c
>> @@ -751,16 +751,14 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
>>
>>   	/* Handle special cases where there is no connection */
>>   	if (unlikely(sock->state != SS_CONNECTED)) {
>> -		if (sock->state == SS_UNCONNECTED) {
>> +		res = -ENOTCONN;
>> +
>> +		if (sock->state == SS_UNCONNECTED)
>>   			res = send_packet(NULL, sock, m, total_len);
>> -			goto exit;
>> -		} else if () {
>> +		else if (sock->state == SS_DISCONNECTING)
>>   			res = -EPIPE;
>> -			goto exit;
>> -		} else {
>> -			res = -ENOTCONN;
>> -			goto exit;
>> -		}
>> +
>> +		goto exit;
>
> I'm not sure that 'removing needless gotos' is a good description of the change.
> Possibly the code is easier to read with only one goto, ymmv.

I thinks so.

> You could remove the 'else' after the 'goto'.
> Maybe the easiest to read is:
> 	if (sock->state == SS_UNCONNECTED)
> 		res = send_packet(NULL, sock, m, total_len);
> 	else
> 		res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
> 	goto exit;
>
> 	David
>
Thanks for your suggestion. I will fix it too.

Regards.
Wang
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH net-next 2/4] tipc: kill unnecessary goto's

[David Miller]

From: Wang Weidong <weidong1991.wang@gmail.com>
Date: Mon, 16 Dec 2013 22:19:47 +0800

> Thanks for your suggestion. I will fix it too.

You are going to have to send me relative fixups as I already
applied your original series from last week.

Re: [PATCH net-next 2/4] tipc: kill unnecessary goto's

[Wang Weidong]

On 2013/12/17 2:01, David Miller wrote:
> From: Wang Weidong <weidong1991.wang@gmail.com>
> Date: Mon, 16 Dec 2013 22:19:47 +0800
> 
>> Thanks for your suggestion. I will fix it too.
> 
> You are going to have to send me relative fixups as I already
> applied your original series from last week.

Ok, I will send them soon.

Regards.
Wang

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

[PATCH net-next 3/4] tipc: Use <linux/uaccess.h> instead of <asm/uaccess.h>

[Wang Weidong]

As warned by checkpatch.pl, use #include <linux/uaccess.h>
instead of <asm/uaccess.h>

Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tipc/core.h b/net/tipc/core.h
index 94895d4..1ff477b 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -47,7 +47,7 @@
 #include <linux/mm.h>
 #include <linux/timer.h>
 #include <linux/string.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <linux/interrupt.h>
 #include <linux/atomic.h>
 #include <asm/hardirq.h>
-- 
1.7.12

[PATCH net-next 4/4] tipc: change lock_sock order in connect()

[Wang Weidong]

Instead of reaquiring the socket lock and taking the normal exit
path when a connection times out, we bail out early with a
return -ETIMEDOUT.

Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/socket.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 844bf34..83f466e 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1507,14 +1507,12 @@ static int connect(struct socket *sock, struct sockaddr *dest, int destlen,
 				sock->state != SS_CONNECTING,
 				timeout ? (long)msecs_to_jiffies(timeout)
 					: MAX_SCHEDULE_TIMEOUT);
-		lock_sock(sk);
 		if (res <= 0) {
 			if (res == 0)
 				res = -ETIMEDOUT;
-			else
-				; /* leave "res" unchanged */
-			goto exit;
+			return res;
 		}
+		lock_sock(sk);
 	}
 
 	if (unlikely(sock->state == SS_DISCONNECTING))
-- 
1.7.12

Re: [PATCH RESEND net-next 0/4] tipc: do some clean up

[David Miller]

From: Wang Weidong <wangweidong1@huawei.com>
Date: Mon, 16 Dec 2013 20:40:27 +0800

> This patch series do some small clean up in tipc. And add reviewed-by
> Erik and Jon.

There is absolutely no reason for you to send this again, your
original patches were queued up in patchwork and I was going to get to
it shortly.

In fact I applied them just now.

Re: [PATCH RESEND net-next 0/4] tipc: do some clean up

[Wang Weidong]

On 2013/12/17 1:58, David Miller wrote:
> From: Wang Weidong <wangweidong1@huawei.com>
> Date: Mon, 16 Dec 2013 20:40:27 +0800
> 
>> This patch series do some small clean up in tipc. And add reviewed-by
>> Erik and Jon.
> 
> There is absolutely no reason for you to send this again, your
> original patches were queued up in patchwork and I was going to get to
> it shortly.
> 
> In fact I applied them just now.
> 
> 

Ok. Sorry for resending these. Next time I should look up the patchwork
before resending patches.

Regards.
Wang