[PATCH net-next 0/2] tipc: do some fixups

[PATCH net-next 0/2] tipc: do some fixups

[Wang Weidong]

in commit 0cee6bbe06f and 3b8401fe9dba not make the
code best. so fix them, as suggested by David Laight.

Wang Weidong (2):
  tipc: make the code look more better
  tipc: make the code look more readability

 net/tipc/port.c   | 13 ++++++-------
 net/tipc/socket.c |  7 ++-----
 2 files changed, 8 insertions(+), 12 deletions(-)

-- 
1.7.12

[PATCH net-next 1/2] tipc: make the code look more better

[Wang Weidong]

make the code look more better than commit 0cee6bbe06f does.
This patch is cosmetic and does not change the operation of
TIPC in any way.

Suggested-by: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/port.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/tipc/port.c b/net/tipc/port.c
index 5fd4c8c..ee81be0 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -832,14 +832,13 @@ exit:
  */
 int __tipc_disconnect(struct tipc_port *tp_ptr)
 {
-	if (tp_ptr->connected) {
-		tp_ptr->connected = 0;
-		/* let timer expire on it's own to avoid deadlock! */
-		tipc_nodesub_unsubscribe(&tp_ptr->subscription);
-		return 0;
-	}
+	if (!tp_ptr->connected)
+		return -ENOTCONN;
 
-	return -ENOTCONN;
+	tp_ptr->connected = 0;
+	/* let timer expire on it's own to avoid deadlock! */
+	tipc_nodesub_unsubscribe(&tp_ptr->subscription);
+	return 0;
 }
 
 /*
-- 
1.7.12

[PATCH net-next 2/2] tipc: make the code look more readability

[Wang Weidong]

make the code look more readability than commit 3b8401fe9dba does.
This patch is cosmetic and does not change the operation of TIPC
in any way.

Suggested-by: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/socket.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 83f466e..5efdeef 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -751,13 +751,10 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
 
 	/* Handle special cases where there is no connection */
 	if (unlikely(sock->state != SS_CONNECTED)) {
-		res = -ENOTCONN;
-
 		if (sock->state == SS_UNCONNECTED)
 			res = send_packet(NULL, sock, m, total_len);
-		else if (sock->state == SS_DISCONNECTING)
-			res = -EPIPE;
-
+		else
+			res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
 		goto exit;
 	}
 
-- 
1.7.12

[PATCH net-next v3 2/2] tipc: make the code look more readable

[Wang Weidong]

In commit 3b8401fe9d ("tipc: kill unnecessary goto's") didn't make
the code look most readable, so fix it. This patch is cosmetic
and does not change the operation of TIPC in any way.

v3: fix the spelling error which pointed out by Sergei.

v2: fix the problem "Referring commit purely by
    SHA1 ID is ambiguous" which pointed out by David.

Suggested-by: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/tipc/socket.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 83f466e..5efdeef 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -751,13 +751,10 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
 
 	/* Handle special cases where there is no connection */
 	if (unlikely(sock->state != SS_CONNECTED)) {
-		res = -ENOTCONN;
-
 		if (sock->state == SS_UNCONNECTED)
 			res = send_packet(NULL, sock, m, total_len);
-		else if (sock->state == SS_DISCONNECTING)
-			res = -EPIPE;
-
+		else
+			res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
 		goto exit;
 	}
 
-- 
1.7.12

Re: [PATCH net-next v3 2/2] tipc: make the code look more readable

[Ying Xue]

On 12/27/2013 09:45 AM, Wang Weidong wrote:
> In commit 3b8401fe9d ("tipc: kill unnecessary goto's") didn't make
> the code look most readable, so fix it. This patch is cosmetic
> and does not change the operation of TIPC in any way.
> 
> v3: fix the spelling error which pointed out by Sergei.
> 
> v2: fix the problem "Referring commit purely by
>     SHA1 ID is ambiguous" which pointed out by David.
> 

Please don't add review version history into patch head comments.


> Suggested-by: David Laight <David.Laight@ACULAB.COM>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
>  net/tipc/socket.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 83f466e..5efdeef 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -751,13 +751,10 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
>  
>  	/* Handle special cases where there is no connection */
>  	if (unlikely(sock->state != SS_CONNECTED)) {
> -		res = -ENOTCONN;
> -
>  		if (sock->state == SS_UNCONNECTED)
>  			res = send_packet(NULL, sock, m, total_len);
> -		else if (sock->state == SS_DISCONNECTING)
> -			res = -EPIPE;
> -
> +		else
> +			res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
>  		goto exit;
>  	}
>  
> 

Re: [PATCH net-next v3 2/2] tipc: make the code look more readable

[Wang Weidong]

On 2013/12/27 9:51, Ying Xue wrote:
> On 12/27/2013 09:45 AM, Wang Weidong wrote:
>> In commit 3b8401fe9d ("tipc: kill unnecessary goto's") didn't make
>> the code look most readable, so fix it. This patch is cosmetic
>> and does not change the operation of TIPC in any way.
>>
>> v3: fix the spelling error which pointed out by Sergei.
>>
>> v2: fix the problem "Referring commit purely by
>>     SHA1 ID is ambiguous" which pointed out by David.
>>
> 
> Please don't add review version history into patch head comments.
> 
Ok, Got it.

Thanks,
Wang

> 
>> Suggested-by: David Laight <David.Laight@ACULAB.COM>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>  net/tipc/socket.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
>> index 83f466e..5efdeef 100644
>> --- a/net/tipc/socket.c
>> +++ b/net/tipc/socket.c
>> @@ -751,13 +751,10 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
>>  
>>  	/* Handle special cases where there is no connection */
>>  	if (unlikely(sock->state != SS_CONNECTED)) {
>> -		res = -ENOTCONN;
>> -
>>  		if (sock->state == SS_UNCONNECTED)
>>  			res = send_packet(NULL, sock, m, total_len);
>> -		else if (sock->state == SS_DISCONNECTING)
>> -			res = -EPIPE;
>> -
>> +		else
>> +			res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
>>  		goto exit;
>>  	}
>>  
>>
> 
> 
> .
> 

Re: [PATCH net-next v3 2/2] tipc: make the code look more readable

[David Miller]

From: Ying Xue <ying.xue@windriver.com>
Date: Fri, 27 Dec 2013 09:51:08 +0800

> On 12/27/2013 09:45 AM, Wang Weidong wrote:
>> In commit 3b8401fe9d ("tipc: kill unnecessary goto's") didn't make
>> the code look most readable, so fix it. This patch is cosmetic
>> and does not change the operation of TIPC in any way.
>> 
>> v3: fix the spelling error which pointed out by Sergei.
>> 
>> v2: fix the problem "Referring commit purely by
>>     SHA1 ID is ambiguous" which pointed out by David.
>> 
> 
> Please don't add review version history into patch head comments.

And please _always_ resubmit the entire series, not just the patches
you are changing.

[PATCH net-next RESEND v3] tipc: make the code look more readable

[Wang Weidong]

In commit 3b8401fe9d ("tipc: kill unnecessary goto's") didn't make
the code look most readable, so fix it. This patch is cosmetic
and does not change the operation of TIPC in any way.

Suggested-by: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
Change note: 

v3: fix the spelling error which pointed out by Sergei.

v2: fix the problem "Referring commit purely by
    SHA1 ID is ambiguous" which pointed out by David.

---
 net/tipc/socket.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 83f466e..5efdeef 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -751,13 +751,10 @@ static int send_stream(struct kiocb *iocb, struct socket *sock,
 
 	/* Handle special cases where there is no connection */
 	if (unlikely(sock->state != SS_CONNECTED)) {
-		res = -ENOTCONN;
-
 		if (sock->state == SS_UNCONNECTED)
 			res = send_packet(NULL, sock, m, total_len);
-		else if (sock->state == SS_DISCONNECTING)
-			res = -EPIPE;
-
+		else
+			res = sock->state == SS_DISCONNECTING ? -EPIPE : -ENOTCONN;
 		goto exit;
 	}
 
-- 
1.7.12

Re: [PATCH net-next RESEND v3] tipc: make the code look more readable

[David Miller]

You need to resubmit patch #1 along with this one, not just this
one by itself.

Thanks.

Re: [PATCH net-next RESEND v3] tipc: make the code look more readable

[Wang Weidong]

On 2013/12/27 12:04, David Miller wrote:
> 
> You need to resubmit patch #1 along with this one, not just this
> one by itself.
> 
> Thanks.
> 
> 
As Ying pointed out that the patch #1 ("tipc: make the code look more better")
does not provide any value. something like below:

Actually the original logic seems better understandable for us because
it immediately lets us know how to do if "tp_ptr->connected" is true.
Instead the patch has a little negative effect on performance because
"tp_ptr->connected" is true in most time.

So I drop the patch #1. Now only this patch.

Regards,
Wang

Re: [PATCH net-next 0/2] tipc: do some fixups

[David Miller]

Referring to commit purely by SHA1 ID is ambiguous.

If the change is placed in other trees, such as the -stable
branches, they will have different SHA1 IDs.

This is why you must always refer to commits as the
SHA1 ID followed by the commit header line text of
the commit in parenthesis and double quotes, like
this:

Commit 0cee6bbe06f ("tipc: remove unnecessary variables and conditions")

Please fix this up in this entire patch series and resubmit.

Re: [PATCH net-next 0/2] tipc: do some fixups

[Wang Weidong]

On 2013/12/25 6:45, David Miller wrote:
> 
> Referring to commit purely by SHA1 ID is ambiguous.
> 
> If the change is placed in other trees, such as the -stable
> branches, they will have different SHA1 IDs.
> 
> This is why you must always refer to commits as the
> SHA1 ID followed by the commit header line text of
> the commit in parenthesis and double quotes, like
> this:
> 
> Commit 0cee6bbe06f ("tipc: remove unnecessary variables and conditions")
> 
> Please fix this up in this entire patch series and resubmit.
> 
Ok, I will fix them.

Regards.
Wang

>