What about this?

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 6ebd809ef207..04f035bcc272 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2446,7 +2446,7 @@ static int tipc_wait_for_connect(struct socket *sock, 
long *timeo_p)
                done = sk_wait_event(sk, timeo_p,
                                     sk->sk_state != TIPC_CONNECTING, &wait);
                remove_wait_queue(sk_sleep(sk), &wait);
-       } while (!done);
+       } while (!tipc_sk_connected(sk));
        return 0;
 }

BR/Tuong

-----Original Message-----
From: Ying Xue <ying....@windriver.com> 
Sent: Thursday, December 26, 2019 2:55 PM
To: Tuong Lien Tong <tuong.t.l...@dektech.com.au>; 
tipc-discussion@lists.sourceforge.net; jon.ma...@ericsson.com; ma...@donjonn.com
Subject: Re: [net] tipc: fix wrong connect() return code

On 12/26/19 3:16 PM, Tuong Lien Tong wrote:
> Sorry, my mistake! I thought it was "while (!done || !sk->sk_err)"...
> But, this is really confusing and one might ask why we continue the loop 
> while the socket has encountered an error (sk-> sk_err! = 0)???

This is totally reasonable because it can make code simpler.
[Tuong]: but a performance hit since it almost looks like the sk_err has to be 
read twice?

> Moreover, it's not necessarily the "sk_err" will be the only case,
> For example:
>       sk->sk_state != TIPC_CONNECTING but sk->sk_err = 0 (somehow) and a 
> pending/interrupt signal

If sk->sk_state is changed from TIPC_CONNECTING to TIPC_ESTABLISHED,
sk->sk_err should be kept 0, but if sk->sk_state is changed to other
states, sk->sk_err must be set with a proper error code in socket
receive path, otherwise, it's a bug.

However, there might be one below race condition:

sk->sk_state is set to TIPC_DISCONNECTING, but before sk->sk_err is set
with an error code, the process who calls connect() is woken up by one
single. Even so, the process is still blocked when it tries to hold sock
lock with lock_sock() in sk_wait_event() because the socket lock is
taken in socket receive path. After socket lock is released, the process
will continue being run. But at that moment, sk->sk_err has been set
with an error code. So, this is no problem for us.

> 
> then we should return '-EINTR'?



_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to